date:20200225

When combined with heavy plugins we occasionally hit the timeouts.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200225124710.14152-17-alex.ben...@linaro.org>

diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 3c7421a356e..b3cff3cad1a 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -79,7 +79,7 @@ QEMU_OPTS=
 
 # If TCG debugging is enabled things are a lot slower
 ifeq ($(CONFIG_DEBUG_TCG),y)
-TIMEOUT=45
+TIMEOUT=60
 else
 TIMEOUT=15
 endif
@@ -137,7 +137,7 @@ PLUGINS=$(notdir $(wildcard $(PLUGIN_DIR)/*.so))
 $(foreach p,$(PLUGINS), \
$(foreach t,$(TESTS),\
$(eval run-plugin-$(t)-with-$(p): $t $p) \
-   $(eval run-plugin-$(t)-with-$(p): TIMEOUT=30) \
+   $(eval run-plugin-$(t)-with-$(p): TIMEOUT=60) \
$(eval RUN_TESTS+=run-plugin-$(t)-with-$(p
 endif
 
-- 
2.20.1

[PULL 15/19] tests/plugins: make howvec clean-up after itself.

TCG plugins are responsible for their own memory usage and although
the plugin_exit is tied to the end of execution in this case it is
still poor practice. Ensure we delete the hash table and related data
when we are done to be a good plugin citizen.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Reviewed-by: Richard Henderson 

Message-Id: <20200225124710.14152-16-alex.ben...@linaro.org>

diff --git a/tests/plugin/howvec.c b/tests/plugin/howvec.c
index 4ca555e1239..3b9a6939f23 100644
--- a/tests/plugin/howvec.c
+++ b/tests/plugin/howvec.c
@@ -163,6 +163,13 @@ static gint cmp_exec_count(gconstpointer a, gconstpointer 
b)
 return ea->count > eb->count ? -1 : 1;
 }
 
+static void free_record(gpointer data)
+{
+InsnExecCount *rec = (InsnExecCount *) data;
+g_free(rec->insn);
+g_free(rec);
+}
+
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
 g_autoptr(GString) report = g_string_new("Instruction Classes:\n");
@@ -195,30 +202,31 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
 
 counts = g_hash_table_get_values(insns);
 if (counts && g_list_next(counts)) {
-GList *it;
-
 g_string_append_printf(report,"Individual Instructions:\n");
+counts = g_list_sort(counts, cmp_exec_count);
 
-it = g_list_sort(counts, cmp_exec_count);
-
-for (i = 0; i < limit && it->next; i++, it = it->next) {
-InsnExecCount *rec = (InsnExecCount *) it->data;
-g_string_append_printf(report, "Instr: %-24s\t(%ld 
hits)\t(op=%#08x/%s)\n",
+for (i = 0; i < limit && g_list_next(counts);
+ i++, counts = g_list_next(counts)) {
+InsnExecCount *rec = (InsnExecCount *) counts->data;
+g_string_append_printf(report,
+   "Instr: %-24s\t(%ld hits)\t(op=%#08x/%s)\n",
rec->insn,
rec->count,
rec->opcode,
rec->class ?
rec->class->class : "un-categorised");
 }
-g_list_free(it);
+g_list_free(counts);
 }
 
+g_hash_table_destroy(insns);
+
 qemu_plugin_outs(report->str);
 }
 
 static void plugin_init(void)
 {
-insns = g_hash_table_new(NULL, g_direct_equal);
+insns = g_hash_table_new_full(NULL, g_direct_equal, NULL, &free_record);
 }
 
 static void vcpu_insn_exec_before(unsigned int cpu_index, void *udata)
-- 
2.20.1

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

2020-02-25 Thread Markus Armbruster

Peter Maydell  writes:

> On Tue, 25 Feb 2020 at 17:48, Paolo Bonzini  wrote:
>>
>> On 25/02/20 18:11, Peter Maydell wrote:
>> >> I assume these are not meant to be applied now, except patch 2?
>> > No, I intended them to be reviewable and applied now. Why
>> > do you think we should wait?
>>
>> Because they remove information from qemu-doc.texi.  I think it's
>> feasible to do a mass conversion quite soon, within a single pull
>> request, the only important part that is missing is the hxtool conversion.
>
> My assumption was that we would attack this by:
>  * converting chunks of the documentation which are in qemu-doc.texi
>but which aren't in the qemu.1 manpage (basically in the way this
>series is doing)
>  * get the qapidoc generation conversion reviewed and into
>master (since at the moment it outputs into files included
>from qemu-doc)

Not true.  QAPI doc comments go into *separate* manuals, the "QEMU QMP
reference" (docs/interop/qemu-qmp-ref.*), and the "QEMU Guest Agent
protocol reference" (docs/interop/qemu-ga-ref.*).

In more detail: scripts/qapi-gen.py generates
docs/interop/qemu-qmp-qapi.texi from qapi/qapi-schema.json, and
docs/interop/qemu-ga-qapi.texi from qga/qapi-schema.json.  These are
included into docs/interop/qemu-qmp-ref.texi and
docs/interop/qemu-ga-ref.texi, respectively.

>  * convert the manpage parts; we have the machinery for dealing
>with the hxtool files, it just needs a little more work
[...]

[PULL 17/19] tcg: save vaddr temp for plugin usage

While do_gen_mem_cb does copy (via extu_tl_i64) vaddr into a new temp
this won't help if the vaddr temp gets clobbered by the actual
load/store op. To avoid this clobbering we explicitly copy vaddr
before the op to ensure it is live my the time we do the
instrumentation.

Suggested-by: Richard Henderson 
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Emilio G. Cota 
Cc: qemu-sta...@nongnu.org
Message-Id: <20200225124710.14152-18-alex.ben...@linaro.org>

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 7d782002e3f..e2e25ebf7db 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -2794,13 +2794,26 @@ static void tcg_gen_req_mo(TCGBar type)
 }
 }
 
+static inline TCGv plugin_prep_mem_callbacks(TCGv vaddr)
+{
+#ifdef CONFIG_PLUGIN
+if (tcg_ctx->plugin_insn != NULL) {
+/* Save a copy of the vaddr for use after a load.  */
+TCGv temp = tcg_temp_new();
+tcg_gen_mov_tl(temp, vaddr);
+return temp;
+}
+#endif
+return vaddr;
+}
+
 static inline void plugin_gen_mem_callbacks(TCGv vaddr, uint16_t info)
 {
 #ifdef CONFIG_PLUGIN
-if (tcg_ctx->plugin_insn == NULL) {
-return;
+if (tcg_ctx->plugin_insn != NULL) {
+plugin_gen_empty_mem_callback(vaddr, info);
+tcg_temp_free(vaddr);
 }
-plugin_gen_empty_mem_callback(vaddr, info);
 #endif
 }
 
@@ -2822,6 +2835,7 @@ void tcg_gen_qemu_ld_i32(TCGv_i32 val, TCGv addr, TCGArg 
idx, MemOp memop)
 }
 }
 
+addr = plugin_prep_mem_callbacks(addr);
 gen_ldst_i32(INDEX_op_qemu_ld_i32, val, addr, memop, idx);
 plugin_gen_mem_callbacks(addr, info);
 
@@ -2868,6 +2882,7 @@ void tcg_gen_qemu_st_i32(TCGv_i32 val, TCGv addr, TCGArg 
idx, MemOp memop)
 memop &= ~MO_BSWAP;
 }
 
+addr = plugin_prep_mem_callbacks(addr);
 gen_ldst_i32(INDEX_op_qemu_st_i32, val, addr, memop, idx);
 plugin_gen_mem_callbacks(addr, info);
 
@@ -2905,6 +2920,7 @@ void tcg_gen_qemu_ld_i64(TCGv_i64 val, TCGv addr, TCGArg 
idx, MemOp memop)
 }
 }
 
+addr = plugin_prep_mem_callbacks(addr);
 gen_ldst_i64(INDEX_op_qemu_ld_i64, val, addr, memop, idx);
 plugin_gen_mem_callbacks(addr, info);
 
@@ -2967,6 +2983,7 @@ void tcg_gen_qemu_st_i64(TCGv_i64 val, TCGv addr, TCGArg 
idx, MemOp memop)
 memop &= ~MO_BSWAP;
 }
 
+addr = plugin_prep_mem_callbacks(addr);
 gen_ldst_i64(INDEX_op_qemu_st_i64, val, addr, memop, idx);
 plugin_gen_mem_callbacks(addr, info);
 
-- 
2.20.1

[PULL 19/19] tests/tcg: take into account expected clashes pauth-4

Pointer authentication isn't perfect so measure the percentage of
failed checks. As we want to vary the pointer we work through a bunch
of different addresses.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Reviewed-by: Richard Henderson 
Message-Id: <20200225124710.14152-20-alex.ben...@linaro.org>

diff --git a/tests/tcg/aarch64/pauth-4.c b/tests/tcg/aarch64/pauth-4.c
index 1040e92aec3..24a639e36ca 100644
--- a/tests/tcg/aarch64/pauth-4.c
+++ b/tests/tcg/aarch64/pauth-4.c
@@ -1,25 +1,45 @@
 #include 
 #include 
+#include 
+#include 
+
+#define TESTS 1000
 
 int main()
 {
-  uintptr_t x, y;
+int i, count = 0;
+float perc;
+void *base = malloc(TESTS);
+
+for (i = 0; i < TESTS; i++) {
+uintptr_t in, x, y;
+
+in = i + (uintptr_t) base;
+
+asm("mov %0, %[in]\n\t"
+"pacia %0, sp\n\t"/* sigill if pauth not supported */
+"eor %0, %0, #4\n\t"  /* corrupt single bit */
+"mov %1, %0\n\t"
+"autia %1, sp\n\t"/* validate corrupted pointer */
+"xpaci %0\n\t"/* strip pac from corrupted pointer */
+: /* out */ "=r"(x), "=r"(y)
+: /* in */ [in] "r" (in)
+: /* clobbers */);
 
-  asm("mov %0, lr\n\t"
-  "pacia %0, sp\n\t"/* sigill if pauth not supported */
-  "eor %0, %0, #4\n\t"  /* corrupt single bit */
-  "mov %1, %0\n\t"
-  "autia %1, sp\n\t"/* validate corrupted pointer */
-  "xpaci %0\n\t"/* strip pac from corrupted pointer */
-  : "=r"(x), "=r"(y));
+/*
+ * Once stripped, the corrupted pointer is of the form 0x...wxyz.
+ * We expect the autia to indicate failure, producing a pointer of the
+ * form 0x000ewxyz.  Use xpaci and != for the test, rather than
+ * extracting explicit bits from the top, because the location of the
+ * error code "e" depends on the configuration of virtual memory.
+ */
+if (x != y) {
+count++;
+}
 
-  /*
-   * Once stripped, the corrupted pointer is of the form 0x...wxyz.
-   * We expect the autia to indicate failure, producing a pointer of the
-   * form 0x000ewxyz.  Use xpaci and != for the test, rather than
-   * extracting explicit bits from the top, because the location of the
-   * error code "e" depends on the configuration of virtual memory.
-   */
-  assert(x != y);
-  return 0;
+}
+perc = (float) count / (float) TESTS;
+printf("Checks Passed: %0.2f%%", perc * 100.0);
+assert(perc > 0.95);
+return 0;
 }
-- 
2.20.1

[PULL 13/19] qemu/bitops.h: Add extract8 and extract16

From: Yoshinori Sato 

Signed-off-by: Yoshinori Sato 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Signed-off-by: Alex Bennée 
Message-Id: <20200212130311.127515-3-ys...@users.sourceforge.jp>
Message-Id: <20200225124710.14152-14-alex.ben...@linaro.org>

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 02c1ce6a5d4..f55ce8b320b 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -301,6 +301,44 @@ static inline uint32_t extract32(uint32_t value, int 
start, int length)
 return (value >> start) & (~0U >> (32 - length));
 }
 
+/**
+ * extract8:
+ * @value: the value to extract the bit field from
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ *
+ * Extract from the 8 bit input @value the bit field specified by the
+ * @start and @length parameters, and return it. The bit field must
+ * lie entirely within the 8 bit word. It is valid to request that
+ * all 8 bits are returned (ie @length 8 and @start 0).
+ *
+ * Returns: the value of the bit field extracted from the input value.
+ */
+static inline uint8_t extract8(uint8_t value, int start, int length)
+{
+assert(start >= 0 && length > 0 && length <= 8 - start);
+return extract32(value, start, length);
+}
+
+/**
+ * extract16:
+ * @value: the value to extract the bit field from
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ *
+ * Extract from the 16 bit input @value the bit field specified by the
+ * @start and @length parameters, and return it. The bit field must
+ * lie entirely within the 16 bit word. It is valid to request that
+ * all 16 bits are returned (ie @length 16 and @start 0).
+ *
+ * Returns: the value of the bit field extracted from the input value.
+ */
+static inline uint16_t extract16(uint16_t value, int start, int length)
+{
+assert(start >= 0 && length > 0 && length <= 16 - start);
+return extract32(value, start, length);
+}
+
 /**
  * extract64:
  * @value: the value to extract the bit field from
-- 
2.20.1

[PULL 18/19] tests/tcg: fix typo in configure.sh test for v8.3

Although most people use the docker images this can trip up on
developer systems with actual valid cross-compilers!

Fixes: bb516dfc5b3
Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20200225124710.14152-19-alex.ben...@linaro.org>

diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index 9eb6ba3b7ea..eaaaff6233a 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -228,7 +228,7 @@ for target in $target_list; do
 echo "CROSS_CC_HAS_SVE=y" >> $config_target_mak
 fi
 if do_compiler "$target_compiler" $target_compiler_cflags \
-   -march=-march=armv8.3-a -o $TMPE $TMPC; then
+   -march=armv8.3-a -o $TMPE $TMPC; then
 echo "CROSS_CC_HAS_ARMV8_3=y" >> $config_target_mak
 fi
 ;;
-- 
2.20.1

[PULL 14/19] target/riscv: progressively load the instruction during decode

The plugin system would throw up a harmless warning when it detected
that a disassembly of an instruction didn't use all it's bytes. Fix
the riscv decoder to only load the instruction bytes it needs as it
needs them.

This drops opcode from the ctx in favour if passing the appropriately
sized opcode down a few levels of the decode.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
Reviewed-by: Robert Foley 

Message-Id: <20200225124710.14152-15-alex.ben...@linaro.org>

diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h
index f8ad7d60fd5..40b6d2b64de 100644
--- a/target/riscv/instmap.h
+++ b/target/riscv/instmap.h
@@ -344,8 +344,8 @@ enum {
 #define GET_C_LW_IMM(inst)  ((extract32(inst, 6, 1) << 2) \
 | (extract32(inst, 10, 3) << 3) \
 | (extract32(inst, 5, 1) << 6))
-#define GET_C_LD_IMM(inst)  ((extract32(inst, 10, 3) << 3) \
-| (extract32(inst, 5, 2) << 6))
+#define GET_C_LD_IMM(inst)  ((extract16(inst, 10, 3) << 3) \
+| (extract16(inst, 5, 2) << 6))
 #define GET_C_J_IMM(inst)   ((extract32(inst, 3, 3) << 1) \
 | (extract32(inst, 11, 1) << 4) \
 | (extract32(inst, 2, 1) << 5) \
@@ -363,7 +363,7 @@ enum {
 #define GET_C_RD(inst)  GET_RD(inst)
 #define GET_C_RS1(inst) GET_RD(inst)
 #define GET_C_RS2(inst) extract32(inst, 2, 5)
-#define GET_C_RS1S(inst)(8 + extract32(inst, 7, 3))
-#define GET_C_RS2S(inst)(8 + extract32(inst, 2, 3))
+#define GET_C_RS1S(inst)(8 + extract16(inst, 7, 3))
+#define GET_C_RS2S(inst)(8 + extract16(inst, 2, 3))
 
 #endif
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 14dc71156be..d5de7f468a7 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -44,7 +44,6 @@ typedef struct DisasContext {
 /* pc_succ_insn points to the instruction following base.pc_next */
 target_ulong pc_succ_insn;
 target_ulong priv_ver;
-uint32_t opcode;
 uint32_t mstatus_fs;
 uint32_t misa;
 uint32_t mem_idx;
@@ -492,45 +491,45 @@ static void gen_set_rm(DisasContext *ctx, int rm)
 tcg_temp_free_i32(t0);
 }
 
-static void decode_RV32_64C0(DisasContext *ctx)
+static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
 {
-uint8_t funct3 = extract32(ctx->opcode, 13, 3);
-uint8_t rd_rs2 = GET_C_RS2S(ctx->opcode);
-uint8_t rs1s = GET_C_RS1S(ctx->opcode);
+uint8_t funct3 = extract16(opcode, 13, 3);
+uint8_t rd_rs2 = GET_C_RS2S(opcode);
+uint8_t rs1s = GET_C_RS1S(opcode);
 
 switch (funct3) {
 case 3:
 #if defined(TARGET_RISCV64)
 /* C.LD(RV64/128) -> ld rd', offset[7:3](rs1')*/
 gen_load_c(ctx, OPC_RISC_LD, rd_rs2, rs1s,
- GET_C_LD_IMM(ctx->opcode));
+ GET_C_LD_IMM(opcode));
 #else
 /* C.FLW (RV32) -> flw rd', offset[6:2](rs1')*/
 gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
-GET_C_LW_IMM(ctx->opcode));
+GET_C_LW_IMM(opcode));
 #endif
 break;
 case 7:
 #if defined(TARGET_RISCV64)
 /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
 gen_store_c(ctx, OPC_RISC_SD, rs1s, rd_rs2,
-  GET_C_LD_IMM(ctx->opcode));
+  GET_C_LD_IMM(opcode));
 #else
 /* C.FSW (RV32) -> fsw rs2', offset[6:2](rs1')*/
 gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
- GET_C_LW_IMM(ctx->opcode));
+ GET_C_LW_IMM(opcode));
 #endif
 break;
 }
 }
 
-static void decode_RV32_64C(DisasContext *ctx)
+static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
 {
-uint8_t op = extract32(ctx->opcode, 0, 2);
+uint8_t op = extract16(opcode, 0, 2);
 
 switch (op) {
 case 0:
-decode_RV32_64C0(ctx);
+decode_RV32_64C0(ctx, opcode);
 break;
 }
 }
@@ -709,22 +708,25 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
 /* Include the auto-generated decoder for 16 bit insn */
 #include "decode_insn16.inc.c"
 
-static void decode_opc(DisasContext *ctx)
+static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
 {
 /* check for compressed insn */
-if (extract32(ctx->opcode, 0, 2) != 3) {
+if (extract16(opcode, 0, 2) != 3) {
 if (!has_ext(ctx, RVC)) {
 gen_exception_illegal(ctx);
 } else {
 ctx->pc_succ_insn = ctx->base.pc_next + 2;
-if (!decode_insn16(ctx, ctx->opcode)) {
+if (!decode_insn16(ctx, opcode)) {
 /* fall back to old decoder */
-decode_RV32_64C(ctx);
+decode_RV32_64C(ctx, opcode);
 }
 }
 } else {
+uint32_t opcode32 = opcode;
+

[PULL 10/19] docs/devel: document query handle lifetimes

I forgot to document the lifetime of handles in the developer
documentation. Do so now.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Reviewed-by: Robert Foley 
Message-Id: <20200225124710.14152-11-alex.ben...@linaro.org>

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 718eef00f22..a05990906cc 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -51,8 +51,17 @@ about how QEMU's translation works to the plugins. While 
there are
 conceptions such as translation time and translation blocks the
 details are opaque to plugins. The plugin is able to query select
 details of instructions and system configuration only through the
-exported *qemu_plugin* functions. The types used to describe
-instructions and events are opaque to the plugins themselves.
+exported *qemu_plugin* functions.
+
+Query Handle Lifetime
+-
+
+Each callback provides an opaque anonymous information handle which
+can usually be further queried to find out information about a
+translation, instruction or operation. The handles themselves are only
+valid during the lifetime of the callback so it is important that any
+information that is needed is extracted during the callback and saved
+by the plugin.
 
 Usage
 =
-- 
2.20.1

[PULL 11/19] plugins/core: add missing break in cb_to_tcg_flags

From: "Emilio G. Cota" 

Fixes: 54cb65d8588
Reported-by: Robert Henry 
Signed-off-by: Emilio G. Cota 
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200105072940.32204-1-c...@braap.org>
Cc: qemu-sta...@nongnu.org
Message-Id: <20200225124710.14152-12-alex.ben...@linaro.org>

diff --git a/plugins/core.c b/plugins/core.c
index 9e1b9e7a915..ed863011baf 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -286,6 +286,7 @@ static inline uint32_t cb_to_tcg_flags(enum 
qemu_plugin_cb_flags flags)
 switch (flags) {
 case QEMU_PLUGIN_CB_RW_REGS:
 ret = 0;
+break;
 case QEMU_PLUGIN_CB_R_REGS:
 ret = TCG_CALL_NO_WG;
 break;
-- 
2.20.1

Re: [PATCH v7 03/11] scripts: add coccinelle script to use auto propagated errp

2020-02-25 Thread Markus Armbruster

Vladimir Sementsov-Ogievskiy  writes:

> 25.02.2020 15:52, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy  writes:
>>
>>> 23.02.2020 11:55, Markus Armbruster wrote:
 Vladimir Sementsov-Ogievskiy  writes:

> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
> does corresponding changes in code (look for details in
> include/qapi/error.h)
>
> Usage example:
> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>
> CC: Eric Blake 
> CC: Kevin Wolf 
> CC: Max Reitz 
> CC: Greg Kurz 
> CC: Stefano Stabellini 
> CC: Anthony Perard 
> CC: Paul Durrant 
> CC: Stefan Hajnoczi 
> CC: "Philippe Mathieu-Daudé" 
> CC: Laszlo Ersek 
> CC: Gerd Hoffmann 
> CC: Stefan Berger 
> CC: Markus Armbruster 
> CC: Michael Roth 
> CC: qemu-bl...@nongnu.org
> CC: xen-de...@lists.xenproject.org
>
>include/qapi/error.h  |   3 +
>scripts/coccinelle/auto-propagated-errp.cocci | 158 ++
>2 files changed, 161 insertions(+)
>create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index b9452d4806..79f8e95214 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -141,6 +141,9 @@
> * ...
> * }
> *
> + * For mass conversion use script
> + *   scripts/coccinelle/auto-propagated-errp.cocci
> + *
> *
> * Receive and accumulate multiple errors (first one wins):
> * Error *err = NULL, *local_err = NULL;

 Extra blank line.

> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
> b/scripts/coccinelle/auto-propagated-errp.cocci
> new file mode 100644
> index 00..fb03c871cb
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
> @@ -0,0 +1,158 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.
> +//
> +// This program is free software; you can redistribute it and/or modify
> +// it under the terms of the GNU General Public License as published by
> +// the Free Software Foundation; either version 2 of the License, or
> +// (at your option) any later version.
> +//
> +// This program is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License
> +// along with this program.  If not, see .
> +//
> +// Usage example:
> +// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> +//  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
> +//  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]
> +
> +@rule0@
> +// Add invocation to errp-functions where necessary
> +// We should skip functions with "Error *const *errp"
> +// parameter, but how to do it with coccinelle?
> +// I don't know, so, I skip them by function name regex.
> +// It's safe: if we did not skip some functions with
> +// "Error *const *errp", ERRP_AUTO_PROPAGATE invocation
> +// will fail to compile, because of const violation.

 Not skipping a function we should skip fails to compile.

 What about skipping a function we should not skip?
>>>
>>> Then it will not be updated.. Not good but I don't have better solution.
>>> Still, I hope, function called *error_append_*_hint will not return error
>>> through errp pointer.
>>
>> Seems likely.  I just dislike inferring behavior from name patterns.
>>
>> Ideally, we recognize the true exceptional pattern instead, i.e. the
>> presence of const.  But figuring out how to make Coccinelle do that for
>> us may be more trouble than it's worth.
>>
>> Hmm...  Coccinelle matches the parameter even with const due to what it
>> calls "isomorphism".  Can I disable it?  *Tinker* *tinker*
>>
>> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci 
>> b/scripts/coccinelle/auto-propagated-errp.cocci
>> index fb03c871cb..0c4414bff3 100644
>> --- a/scripts/coccinelle/auto-propagated-errp.cocci
>> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
>> @@ -20,15 +20,11 @@
>>   //  --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
>>   //  blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]
>>   -@rule0@
>> +@r

[PULL 12/19] tests/plugin: prevent uninitialized warning

From: Chen Qun 

According to the glibc function requirements, we need initialise
 the variable. Otherwise there will be compilation warnings:

glib-autocleanups.h:28:3: warning: ‘out’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
   g_free (*pp);
   ^~~~

Reported-by: Euler Robot 
Signed-off-by: Chen Qun 
Reviewed-by: Thomas Huth 
Message-Id: <20200206093238.203984-1-kuhn.chen...@huawei.com>
[AJB: uses Thomas's single line allocation]
Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Message-Id: <20200225124710.14152-13-alex.ben...@linaro.org>

diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index f30bea08dcc..df19fd359df 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -22,9 +22,9 @@ static bool do_inline;
 
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
-g_autofree gchar *out;
-out = g_strdup_printf("bb's: %" PRIu64", insns: %" PRIu64 "\n",
-  bb_count, insn_count);
+g_autofree gchar *out = g_strdup_printf(
+"bb's: %" PRIu64", insns: %" PRIu64 "\n",
+bb_count, insn_count);
 qemu_plugin_outs(out);
 }
 
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index 0a8f5ae..a9a6e412373 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -44,8 +44,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
qemu_plugin_tb *tb)
 
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
-g_autofree gchar *out;
-out = g_strdup_printf("insns: %" PRIu64 "\n", insn_count);
+g_autofree gchar *out = g_strdup_printf("insns: %" PRIu64 "\n", 
insn_count);
 qemu_plugin_outs(out);
 }
 
-- 
2.20.1

[PULL 08/19] tests/iotests: be a little more forgiving on the size test

At least on ZFS this was failing as 512 was less than or equal to 512.
I suspect the reason is additional compression done by ZFS and however
qemu-img gets the actual size.

Loosen the criteria to make sure after is not bigger than before and
also dump the values in the report.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Reviewed-by: Stefan Berger 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200225124710.14152-9-alex.ben...@linaro.org>

diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 3500e0c47a2..af677d90b86 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -125,9 +125,9 @@ $QEMU_IO -c "write -P 0xcc $offset $data_size" "json:{\
 sizeB=$($QEMU_IMG info --output=json "$TEST_IMG" |
 sed -n '/"actual-size":/ s/[^0-9]//gp')
 
-if [ $sizeA -le $sizeB ]
+if [ $sizeA -lt $sizeB ]
 then
-echo "Compression ERROR"
+echo "Compression ERROR ($sizeA < $sizeB)"
 fi
 
 $QEMU_IMG check --output=json "$TEST_IMG" |
-- 
2.20.1

[PULL 07/19] travis.yml: single-thread build-tcg stages

This still seems to be a problem for Travis.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200225124710.14152-8-alex.ben...@linaro.org>

diff --git a/.travis.yml b/.travis.yml
index 0612998958b..f4020dcc6c8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -400,7 +400,7 @@ jobs:
 - name: "GCC check-tcg (some-softmmu)"
   env:
 - CONFIG="--enable-debug-tcg 
--target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-- TEST_BUILD_CMD="make -j${JOBS} build-tcg"
+- TEST_BUILD_CMD="make build-tcg"
 - TEST_CMD="make check-tcg"
 - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
 
@@ -409,7 +409,7 @@ jobs:
 - name: "GCC plugins check-tcg (some-softmmu)"
   env:
 - CONFIG="--enable-plugins --enable-debug-tcg 
--target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-- TEST_BUILD_CMD="make -j${JOBS} build-tcg"
+- TEST_BUILD_CMD="make build-tcg"
 - TEST_CMD="make check-tcg"
 - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
 
-- 
2.20.1

[PULL 06/19] travis.yml: Fix Travis YAML configuration warnings

From: Wainer dos Santos Moschetta 

This fixes the following warnings Travis has detected on the
YAML configuration:

- 'on root: missing os, using the default "linux"'
- 'on root: the key matrix is an alias for jobs, using jobs'
- 'on jobs.include.python: unexpected sequence, using the first value (3.5)'
- 'on jobs.include.python: unexpected sequence, using the first value (3.6)'

Signed-off-by: Wainer dos Santos Moschetta 
Signed-off-by: Alex Bennée 
Message-Id: <20200207210124.141119-2-waine...@redhat.com>
Message-Id: <20200225124710.14152-7-alex.ben...@linaro.org>

diff --git a/.travis.yml b/.travis.yml
index ea13e071795..0612998958b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,7 @@
 # The current Travis default is a VM based 16.04 Xenial on GCE
 # Additional builds with specific requirements for a full VM need to
 # be added as additional matrix: entries later on
+os: linux
 dist: xenial
 language: c
 compiler:
@@ -113,7 +114,7 @@ after_script:
   - if command -v ccache ; then ccache --show-stats ; fi
 
 
-matrix:
+jobs:
   include:
 - name: "GCC static (user)"
   env:
@@ -297,8 +298,7 @@ matrix:
 - CONFIG="--target-list=x86_64-softmmu"
 - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-default"
   language: python
-  python:
-- "3.5"
+  python: 3.5
 
 
 - name: "GCC Python 3.6 (x86_64-softmmu)"
@@ -306,8 +306,7 @@ matrix:
 - CONFIG="--target-list=x86_64-softmmu"
 - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-default"
   language: python
-  python:
-- "3.6"
+  python: 3.6
 
 
 # Acceptance (Functional) tests
-- 
2.20.1

[PULL 05/19] travis.yml: Test the s390-ccw build, too

From: Thomas Huth 

Since we can now use a s390x host on Travis, we can also build and
test the s390-ccw bios images there. For this we have to make sure
that roms/SLOF is checked out, too, and then move the generated *.img
files to the right location before running the tests.

Signed-off-by: Thomas Huth 
Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Acked-by: Cornelia Huck 
Message-Id: <20200206202543.7085-1-th...@redhat.com>
Message-Id: <20200225124710.14152-6-alex.ben...@linaro.org>

diff --git a/.travis.yml b/.travis.yml
index 58870559515..ea13e071795 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -509,6 +509,16 @@ matrix:
   env:
 - TEST_CMD="make check check-tcg V=1"
 - CONFIG="--disable-containers 
--target-list=${MAIN_SOFTMMU_TARGETS},s390x-linux-user"
+  script:
+- ( cd ${SRC_DIR} ; git submodule update --init roms/SLOF )
+- BUILD_RC=0 && make -j${JOBS} || BUILD_RC=$?
+- |
+  if [ "$BUILD_RC" -eq 0 ] ; then
+  mv pc-bios/s390-ccw/*.img pc-bios/ ;
+  ${TEST_CMD} ;
+  else
+  $(exit $BUILD_RC);
+  fi
 
 # Release builds
 # The make-release script expect a QEMU version, so our tag must start 
with a 'v'.
-- 
2.20.1

[PULL 00/19] testing and plugin updates

The following changes since commit db736e0437aa6fd7c1b7e4599c17f9619ab6b837:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2020-02-25 13:31:16 +)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-testing-and-plugins-250220-1

for you to fetch changes up to bc97f9f64f8a4a84d0d06949749e9dbec143b9f5:

  tests/tcg: take into account expected clashes pauth-4 (2020-02-25 20:20:23 
+)


Testing and plugin updates:

 - fix pauth TCG tests
 - tweak away rcutorture failures
 - various Travis updates
 - relax iotest size check a little
 - fix for -trace/-D clash
 - fix cross compile detection for tcg tests
 - document plugin query lifetime
 - fix missing break in plugin core
 - fix some plugin warnings
 - better progressive instruction decode
 - avoid trampling vaddr in plugins


Alex Bennée (14):
  tests/tcg: include a skip runner for pauth3 with plugins
  tests/rcutorture: update usage hint
  tests/rcutorture: better document locking of stats
  tests/rcutorture: mild documenting refactor of update thread
  travis.yml: single-thread build-tcg stages
  tests/iotests: be a little more forgiving on the size test
  tracing: only allow -trace to override -D if set
  docs/devel: document query handle lifetimes
  target/riscv: progressively load the instruction during decode
  tests/plugins: make howvec clean-up after itself.
  tests/tcg: give debug builds a little bit longer
  tcg: save vaddr temp for plugin usage
  tests/tcg: fix typo in configure.sh test for v8.3
  tests/tcg: take into account expected clashes pauth-4

Chen Qun (1):
  tests/plugin: prevent uninitialized warning

Emilio G. Cota (1):
  plugins/core: add missing break in cb_to_tcg_flags

Thomas Huth (1):
  travis.yml: Test the s390-ccw build, too

Wainer dos Santos Moschetta (1):
  travis.yml: Fix Travis YAML configuration warnings

Yoshinori Sato (1):
  qemu/bitops.h: Add extract8 and extract16

 docs/devel/tcg-plugins.rst| 13 +-
 include/qemu/bitops.h | 38 
 target/riscv/instmap.h|  8 ++--
 plugins/core.c|  1 +
 target/riscv/translate.c  | 40 +
 tcg/tcg-op.c  | 23 --
 tests/plugin/bb.c |  6 +--
 tests/plugin/howvec.c | 26 +++
 tests/plugin/insn.c   |  3 +-
 tests/rcutorture.c| 74 +--
 tests/tcg/aarch64/pauth-4.c   | 54 +++---
 trace/control.c   | 11 +++--
 .travis.yml   | 23 +++---
 tests/qemu-iotests/214|  4 +-
 tests/tcg/Makefile.target |  4 +-
 tests/tcg/aarch64/Makefile.softmmu-target |  2 +
 tests/tcg/configure.sh|  2 +-
 17 files changed, 235 insertions(+), 97 deletions(-)

-- 
2.20.1

[PULL 09/19] tracing: only allow -trace to override -D if set

Otherwise any -D settings the user may have made get ignored.

Signed-off-by: Alex Bennée 
Tested-by: Laurent Vivier 
Reviewed-by: Robert Foley 
Message-Id: <20200225124710.14152-10-alex.ben...@linaro.org>

diff --git a/trace/control.c b/trace/control.c
index 6c775e68eba..2ffe0008184 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -226,10 +226,15 @@ void trace_init_file(const char *file)
 #ifdef CONFIG_TRACE_SIMPLE
 st_set_trace_file(file);
 #elif defined CONFIG_TRACE_LOG
-/* If both the simple and the log backends are enabled, "--trace file"
- * only applies to the simple backend; use "-D" for the log backend.
+/*
+ * If both the simple and the log backends are enabled, "--trace file"
+ * only applies to the simple backend; use "-D" for the log
+ * backend. However we should only override -D if we actually have
+ * something to override it with.
  */
-qemu_set_log_filename(file, &error_fatal);
+if (file) {
+qemu_set_log_filename(file, &error_fatal);
+}
 #else
 if (file) {
 fprintf(stderr, "error: --trace file=...: "
-- 
2.20.1

[PULL 04/19] tests/rcutorture: mild documenting refactor of update thread

This is mainly to help with reasoning what the test is trying to do.
We can move rcu_stress_idx to a local variable as there is only ever
one updater thread. I've also added an assert to catch the case where
we end up updating the current structure to itself which is the only
way I can see the mberror cases we are seeing on Travis.

We shall see if the rcutorture test failures go away now.

Signed-off-by: Alex Bennée 
Reviewed-by: Paolo Bonzini 

Message-Id: <20200225124710.14152-5-alex.ben...@linaro.org>

diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index 256d24ed5ba..732f03abdaa 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -230,13 +230,12 @@ static void uperftest(int nupdaters, int duration)
 #define RCU_STRESS_PIPE_LEN 10
 
 struct rcu_stress {
-int pipe_count;
+int age;  /* how many update cycles while not rcu_stress_current */
 int mbtest;
 };
 
 struct rcu_stress rcu_stress_array[RCU_STRESS_PIPE_LEN] = { { 0 } };
 struct rcu_stress *rcu_stress_current;
-int rcu_stress_idx;
 int n_mberror;
 
 /* Updates protected by counts_mutex */
@@ -261,7 +260,7 @@ static void *rcu_read_stress_test(void *arg)
 while (goflag == GOFLAG_RUN) {
 rcu_read_lock();
 p = atomic_rcu_read(&rcu_stress_current);
-if (p->mbtest == 0) {
+if (atomic_read(&p->mbtest) == 0) {
 n_mberror++;
 }
 rcu_read_lock();
@@ -269,7 +268,7 @@ static void *rcu_read_stress_test(void *arg)
 garbage++;
 }
 rcu_read_unlock();
-pc = p->pipe_count;
+pc = atomic_read(&p->age);
 rcu_read_unlock();
 if ((pc > RCU_STRESS_PIPE_LEN) || (pc < 0)) {
 pc = RCU_STRESS_PIPE_LEN;
@@ -288,32 +287,52 @@ static void *rcu_read_stress_test(void *arg)
 return NULL;
 }
 
+/*
+ * Stress Test Updater
+ *
+ * The updater cycles around updating rcu_stress_current to point at
+ * one of the rcu_stress_array_entries and resets it's age. It
+ * then increments the age of all the other entries. The age
+ * will be read under an rcu_read_lock() and distribution of values
+ * calculated. The final result gives an indication of how many
+ * previously current rcu_stress entries are in flight until the RCU
+ * cycle complete.
+ */
 static void *rcu_update_stress_test(void *arg)
 {
-int i;
-struct rcu_stress *p;
+int i, rcu_stress_idx = 0;
+struct rcu_stress *cp = atomic_read(&rcu_stress_current);
 
 rcu_register_thread();
-
 *(struct rcu_reader_data **)arg = &rcu_reader;
+
 while (goflag == GOFLAG_INIT) {
 g_usleep(1000);
 }
+
 while (goflag == GOFLAG_RUN) {
-i = rcu_stress_idx + 1;
-if (i >= RCU_STRESS_PIPE_LEN) {
-i = 0;
+struct rcu_stress *p;
+rcu_stress_idx++;
+if (rcu_stress_idx >= RCU_STRESS_PIPE_LEN) {
+rcu_stress_idx = 0;
 }
-p = &rcu_stress_array[i];
-p->mbtest = 0;
+p = &rcu_stress_array[rcu_stress_idx];
+/* catching up with ourselves would be a bug */
+assert(p != cp);
+atomic_set(&p->mbtest, 0);
 smp_mb();
-p->pipe_count = 0;
-p->mbtest = 1;
+atomic_set(&p->age, 0);
+atomic_set(&p->mbtest, 1);
 atomic_rcu_set(&rcu_stress_current, p);
-rcu_stress_idx = i;
+cp = p;
+/*
+ * New RCU structure is now live, update pipe counts on old
+ * ones.
+ */
 for (i = 0; i < RCU_STRESS_PIPE_LEN; i++) {
 if (i != rcu_stress_idx) {
-rcu_stress_array[i].pipe_count++;
+atomic_set(&rcu_stress_array[i].age,
+   rcu_stress_array[i].age + 1);
 }
 }
 synchronize_rcu();
@@ -346,7 +365,7 @@ static void stresstest(int nreaders, int duration)
 int i;
 
 rcu_stress_current = &rcu_stress_array[0];
-rcu_stress_current->pipe_count = 0;
+rcu_stress_current->age = 0;
 rcu_stress_current->mbtest = 1;
 for (i = 0; i < nreaders; i++) {
 create_thread(rcu_read_stress_test);
@@ -376,7 +395,7 @@ static void gtest_stress(int nreaders, int duration)
 int i;
 
 rcu_stress_current = &rcu_stress_array[0];
-rcu_stress_current->pipe_count = 0;
+rcu_stress_current->age = 0;
 rcu_stress_current->mbtest = 1;
 for (i = 0; i < nreaders; i++) {
 create_thread(rcu_read_stress_test);
-- 
2.20.1

[PULL 01/19] tests/tcg: include a skip runner for pauth3 with plugins

If we have plugins enabled we still need to have built the test to be
able to run it.

Signed-off-by: Alex Bennée 
Reviewed-by: Robert Foley 
Message-Id: <20200225124710.14152-2-alex.ben...@linaro.org>

diff --git a/tests/tcg/aarch64/Makefile.softmmu-target 
b/tests/tcg/aarch64/Makefile.softmmu-target
index d2299b98b76..71f72cfbe34 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -70,4 +70,6 @@ pauth-3:
$(call skip-test, "BUILD of $@", "missing compiler support")
 run-pauth-3:
$(call skip-test, "RUN of pauth-3", "not built")
+run-plugin-pauth-3-with-%:
+   $(call skip-test, "RUN of pauth-3 ($*)", "not built")
 endif
-- 
2.20.1

[PULL 03/19] tests/rcutorture: better document locking of stats

This is pure code motion with no functional effect.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200225124710.14152-4-alex.ben...@linaro.org>

diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index e8b2169e7dd..256d24ed5ba 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -65,8 +65,6 @@
 #include "qemu/rcu.h"
 #include "qemu/thread.h"
 
-long long n_reads = 0LL;
-long n_updates = 0L;
 int nthreadsrunning;
 
 #define GOFLAG_INIT 0
@@ -78,11 +76,20 @@ static volatile int goflag = GOFLAG_INIT;
 #define RCU_READ_RUN 1000
 
 #define NR_THREADS 100
-static QemuMutex counts_mutex;
 static QemuThread threads[NR_THREADS];
 static struct rcu_reader_data *data[NR_THREADS];
 static int n_threads;
 
+/*
+ * Statistical counts
+ *
+ * These are the sum of local counters at the end of a run.
+ * Updates are protected by a mutex.
+ */
+static QemuMutex counts_mutex;
+long long n_reads = 0LL;
+long n_updates = 0L;
+
 static void create_thread(void *(*func)(void *))
 {
 if (n_threads >= NR_THREADS) {
@@ -230,8 +237,9 @@ struct rcu_stress {
 struct rcu_stress rcu_stress_array[RCU_STRESS_PIPE_LEN] = { { 0 } };
 struct rcu_stress *rcu_stress_current;
 int rcu_stress_idx;
-
 int n_mberror;
+
+/* Updates protected by counts_mutex */
 long long rcu_stress_count[RCU_STRESS_PIPE_LEN + 1];
 
 
-- 
2.20.1

[PULL 02/19] tests/rcutorture: update usage hint

Although documented in the comments we don't display all the various
invocations we can in the usage.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200225124710.14152-3-alex.ben...@linaro.org>

diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index 49311c82ea4..e8b2169e7dd 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -413,7 +413,8 @@ static void gtest_stress_10_5(void)
 
 static void usage(int argc, char *argv[])
 {
-fprintf(stderr, "Usage: %s [nreaders [ perf | stress ] ]\n", argv[0]);
+fprintf(stderr, "Usage: %s [nreaders [ [r|u]perf | stress [duration]]\n",
+argv[0]);
 exit(-1);
 }
 
-- 
2.20.1

Issue with vl.c: move -m parsing after memory backends has been processed. Commit a1b18df9a4848fc8a906e40c275063bfe9ca2047

2020-02-25 Thread Howard Spoelstra

Hi all,

Commit a1b18df9a4848fc8a906e40c275063bfe9ca2047 on the ppc-for-50 branch
makes qemu-system-ppc running Mac OS 9 extremely slow. I bisected to the
result below.

Command line used:
./qemu-system-ppc -L pc-bios -M mac99,via=pmu -m 512 -boot c \
-hda 9.2.img \
-serial stdio -sdl

Best,
Howard

a1b18df9a4848fc8a906e40c275063bfe9ca2047 is the first bad commit
commit a1b18df9a4848fc8a906e40c275063bfe9ca2047
Author: Igor Mammedov 
Date:   Wed Feb 19 11:08:40 2020 -0500

vl.c: move -m parsing after memory backends has been processed

It will be possible for main RAM to come from memory-backend
and we should check that size specified in -m matches the size
of the backend and [MachineState::]ram_size also matches
backend's size.

However -m parsing (set_memory_options()) happens before backends
are intialized (object_create_delayed()) which complicates it.
Consolidate set_memory_options() and assigning parsed results to
current_machine after backends are initialized, so it would be
possible access the initialized backend instance to compare
sizes.

This patch only consolidates scattered places touching ram_size
within vl.c. And follow up patch will integrate backend handling
to set_memory_options().

Signed-off-by: Igor Mammedov 
Message-Id: <20200219160953.13771-7-imamm...@redhat.com>

 vl.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

Re: QAPI schema for desired state of LUKS keyslots

2020-02-25 Thread Markus Armbruster

Max Reitz  writes:

> On 25.02.20 17:48, Markus Armbruster wrote:
>> Max Reitz  writes:
>> 
>>> On 15.02.20 15:51, Markus Armbruster wrote:
 Review of this patch led to a lengthy QAPI schema design discussion.
 Let me try to condense it into a concrete proposal.

 This is about the QAPI schema, and therefore about QMP.  The
 human-friendly interface is out of scope.  Not because it's not
 important (it clearly is!), only because we need to *focus* to have a
 chance at success.

 I'm going to include a few design options.  I'll mark them "Option:".

 The proposed "amend" interface takes a specification of desired state,
 and figures out how to get from here to there by itself.  LUKS keyslots
 are one part of desired state.

 We commonly have eight LUKS keyslots.  Each keyslot is either active or
 inactive.  An active keyslot holds a secret.

 Goal: a QAPI type for specifying desired state of LUKS keyslots.

 Proposal:

 { 'enum': 'LUKSKeyslotState',
   'data': [ 'active', 'inactive' ] }

 { 'struct': 'LUKSKeyslotActive',
   'data': { 'secret': 'str',
 '*iter-time': 'int } }

 { 'struct': 'LUKSKeyslotInactive',
   'data': { '*old-secret': 'str' } }

 { 'union': 'LUKSKeyslotAmend',
   'base': { '*keyslot': 'int',
 'state': 'LUKSKeyslotState' }
   'discriminator': 'state',
   'data': { 'active': 'LUKSKeyslotActive',
 'inactive': 'LUKSKeyslotInactive' } }
>>>
>>> Looks OK to me.  The only thing is that @old-secret kind of works as an
>>> address, just like @keyslot,
>> 
>> It does.
>> 
>>>  so it might also make sense to me to put
>>> @keyslot/@old-secret into a union in the base structure.
>> 
>> I'm fine with state-specific extra adressing modes (I better be, I
>> proposed them).
>> 
>> I'd also be fine with a single state-independent addressing mode, as
>> long as we can come up with sane semantics.  Less flexible when adding
>> states, but we almost certainly won't.
>> 
>> Let's see how we could merge my two addressing modes into one.
>> 
>> The two are
>> 
>> * active
>> 
>>   keyslot old-secret  slot(s) selected
>>   absent  N/A one inactive slot if exist, else error
>>   present N/A the slot given by @keyslot
>
> Oh, I thought that maybe we could use old-secret here, too, for
> modifying the iter-time.

Update in place is unsafe.

>   But if old-secret makes no sense for
> to-be-active slots, then there’s little point in putting old-secret in
> the base.
>
> (OTOH, specifying old-secret for to-be-active slots does have a sensible
> meaning; it’s just that we won’t support changing anything about
> already-active slots, except making them inactive.  So that might be an
> argument for not making it a syntactic error, but just a semantic error.)

Matter of taste.  I like to keep simple things syntactic, and thus
visible in introspection.

> [...]
>
>> Note we we don't really care what "inactive, both absent" does.  My
>> proposed semantics are just the most regular I could find.  We can
>> therefore resolve the conflict by picking "active, both absent":
>> 
>>   keyslot old-secret  slot(s) selected
>>   absent  absent  one inactive slot if exist, else error
>>   present absent  the slot given by @keyslot
>>   absent  present all active slots holding @old-secret
>>   present present the slot given by @keyslot, error unless
>>   it's active holding @old-secret
>> 
>> Changes:
>> 
>> * inactive, both absent: changed; we select "one inactive slot" instead of
>>   "all slots".
>> 
>>   "All slots" is a no-op when the current state has no active keyslots,
>>   else error.
>> 
>>   "One inactive slot" is a no-op when the current state has one, else
>>   error.  Thus, we no-op rather than error in some states.
>> 
>> * active, keyslot absent or present, old-secret present: new; selects
>>   active slot(s) holding @old-secret, no-op when old-secret == secret,
>>   else error (no in place update)
>> 
>> Can do.  It's differently irregular, and has a few more combinations
>> that are basically useless, which I find unappealing.  Matter of taste,
>> I guess.
>> 
>> Anyone got strong feelings here?
>
> The only strong feeling I have is that I absolutely don’t have a strong
> feeling about this. :)
>
> As such, I think we should just treat my rambling as such and stick to
> your proposal, since we’ve already gathered support for it.

Thanks!

Re: [PATCH] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM




- Original Message -
> On Wed, Feb 26, 2020 at 03:06:47PM +0800, Jason Wang wrote:
> > We turn on device IOTLB via VIRTIO_F_IOMMU_PLATFORM unconditionally on
> > platform without IOMMU support. This can lead unnecessary IOTLB
> > transactions which will damage the performance.
> > 
> > Fixing this by check whether the device is backed by IOMMU and disable
> > device IOTLB.
> > 
> > Reported-by: Halil Pasic 
> > Fixes: c471ad0e9bd46 ("vhost_net: device IOTLB support")
> > Signed-off-by: Jason Wang 
> > ---
> >  hw/virtio/vhost.c | 12 +++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 9edfadc81d..6e12c3d2de 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -290,7 +290,14 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
> >  {
> >  VirtIODevice *vdev = dev->vdev;
> >  
> > -return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> > +/*
> > + * For vhost, VIRTIO_F_IOMMU_PLATFORM means the backend support
> > + * incremental memory mapping API via IOTLB API. For platform that
> > + * does not have IOMMU, there's no need to enable this feature
> > + * which may cause unnecessary IOTLB miss/update trnasactions.
> > + */
> > +return vdev->dma_as != &address_space_memory &&
> > +   virtio_has_feature(dev->acked_features,
> > VIRTIO_F_IOMMU_PLATFORM);
> >  }
> >  
> >  static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
> 
> Why check acked_features and not host features here?
> I'd worry that if we do it like this, userspace driver
> within guest can clear the feature and make device access
> memory directly.

Right, host_features should be more than enough.

> 
> > @@ -765,6 +772,9 @@ static int vhost_dev_set_features(struct vhost_dev
> > *dev,
> >  if (enable_log) {
> >  features |= 0x1ULL << VHOST_F_LOG_ALL;
> >  }
> > +if (dev->vdev->dma_as == &address_space_memory) {
> > +features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> > +}
> 
> 
> That's a guest visible change. Which seems at best unnecessary.
>

I don't get how this can be visible from guest? It works as F_LOG_ALL.

Thanks

> >  r = dev->vhost_ops->vhost_set_features(dev, features);
> >  if (r < 0) {
> >  VHOST_OPS_DEBUG("vhost_set_features failed");
> > --
> > 2.19.1
> 
>

Re: [PATCH] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM

2020-02-25 Thread Michael S. Tsirkin

On Wed, Feb 26, 2020 at 03:06:47PM +0800, Jason Wang wrote:
> We turn on device IOTLB via VIRTIO_F_IOMMU_PLATFORM unconditionally on
> platform without IOMMU support. This can lead unnecessary IOTLB
> transactions which will damage the performance.
> 
> Fixing this by check whether the device is backed by IOMMU and disable
> device IOTLB.
> 
> Reported-by: Halil Pasic 
> Fixes: c471ad0e9bd46 ("vhost_net: device IOTLB support")
> Signed-off-by: Jason Wang 
> ---
>  hw/virtio/vhost.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9edfadc81d..6e12c3d2de 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -290,7 +290,14 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
>  {
>  VirtIODevice *vdev = dev->vdev;
>  
> -return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> +/*
> + * For vhost, VIRTIO_F_IOMMU_PLATFORM means the backend support
> + * incremental memory mapping API via IOTLB API. For platform that
> + * does not have IOMMU, there's no need to enable this feature
> + * which may cause unnecessary IOTLB miss/update trnasactions.
> + */
> +return vdev->dma_as != &address_space_memory &&
> +   virtio_has_feature(dev->acked_features, VIRTIO_F_IOMMU_PLATFORM);
>  }
>  
>  static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,

Why check acked_features and not host features here?
I'd worry that if we do it like this, userspace driver
within guest can clear the feature and make device access
memory directly.

> @@ -765,6 +772,9 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
>  if (enable_log) {
>  features |= 0x1ULL << VHOST_F_LOG_ALL;
>  }
> +if (dev->vdev->dma_as == &address_space_memory) {
> +features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> +}


That's a guest visible change. Which seems at best unnecessary.

>  r = dev->vhost_ops->vhost_set_features(dev, features);
>  if (r < 0) {
>  VHOST_OPS_DEBUG("vhost_set_features failed");
> -- 
> 2.19.1

[PATCH] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM

We turn on device IOTLB via VIRTIO_F_IOMMU_PLATFORM unconditionally on
platform without IOMMU support. This can lead unnecessary IOTLB
transactions which will damage the performance.

Fixing this by check whether the device is backed by IOMMU and disable
device IOTLB.

Reported-by: Halil Pasic 
Fixes: c471ad0e9bd46 ("vhost_net: device IOTLB support")
Signed-off-by: Jason Wang 
---
 hw/virtio/vhost.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 9edfadc81d..6e12c3d2de 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -290,7 +290,14 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
 {
 VirtIODevice *vdev = dev->vdev;
 
-return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+/*
+ * For vhost, VIRTIO_F_IOMMU_PLATFORM means the backend support
+ * incremental memory mapping API via IOTLB API. For platform that
+ * does not have IOMMU, there's no need to enable this feature
+ * which may cause unnecessary IOTLB miss/update trnasactions.
+ */
+return vdev->dma_as != &address_space_memory &&
+   virtio_has_feature(dev->acked_features, VIRTIO_F_IOMMU_PLATFORM);
 }
 
 static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
@@ -765,6 +772,9 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
 if (enable_log) {
 features |= 0x1ULL << VHOST_F_LOG_ALL;
 }
+if (dev->vdev->dma_as == &address_space_memory) {
+features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
+}
 r = dev->vhost_ops->vhost_set_features(dev, features);
 if (r < 0) {
 VHOST_OPS_DEBUG("vhost_set_features failed");
-- 
2.19.1

RE: [PATCH] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()

2020-02-25 Thread Chenqun (kuhn)


>-Original Message-
>From: Jason Wang [mailto:jasow...@redhat.com]
>Sent: Wednesday, February 26, 2020 11:03 AM
>To: Peter Maydell 
>Cc: Chenqun (kuhn) ; QEMU Developers
>; QEMU Trivial ;
>Zhanghailiang ; Peter Chubb
>; qemu-arm 
>Subject: Re: [PATCH] hw/net/imx_fec: write TGSR and TCSR3 in
>imx_enet_write()
>
>
>On 2020/2/25 下午6:18, Peter Maydell wrote:
>> On Tue, 25 Feb 2020 at 05:41, Jason Wang  wrote:
>>>
>>> On 2020/2/25 上午10:59, Chen Qun wrote:
 The current code causes clang static code analyzer generate warning:
 hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read
   value = value & 0x000f;
   ^   ~~
 hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read
   value = value & 0x00fd;
   ^   ~~

 According to the definition of the function, the two “value” assignments
should be written to registers.

 Reported-by: Euler Robot 
 Signed-off-by: Chen Qun 
 ---
 I'm not sure if this modification is correct, just from the function
definition, it is correct.
 ---
hw/net/imx_fec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index
 6a124a154a..92f6215712 100644
 --- a/hw/net/imx_fec.c
 +++ b/hw/net/imx_fec.c
 @@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s,
>uint32_t index, uint32_t value)
break;
case ENET_TGSR:
/* implement clear timer flag */
 -value = value & 0x000f;
 +s->regs[index] = value & 0x000f;
break;
>> Hi; the datasheet for this SoC says that these bits of the register
>> are write-1-to-clear, so while this is definitely a bug I don't think
>> this is the right fix.
>>
case ENET_TCSR0:
case ENET_TCSR1:
case ENET_TCSR2:
case ENET_TCSR3:
 -value = value & 0x00fd;
 +s->regs[index] = value & 0x00fd;
break;
>> Here bit 7 is write-1-to-clear, though bits 0 and
>> 2..5 are simple write-the-value.
>>
case ENET_TCCR0:
case ENET_TCCR1:
>>>
>>> Applied.
>> Could you drop this from your queue, please?
>>
>> thanks
>> -- PMM
>
>
>Sure, Chen please send V2 to address Peter's comment.
OK,  but I didn't find the datasheet that contains these two registers 
description.
Could someone provide me with a  connection for the datasheet ?

[Bug 1863710] Re: qemu 4.2 does not process discard(trim) commands

2020-02-25 Thread Chris S.

** Attachment removed: "win10-real.xml"
   
https://bugs.launchpad.net/qemu/+bug/1863710/+attachment/5329158/+files/win10-real.xml

** Attachment added: "win10.xml"
   
https://bugs.launchpad.net/qemu/+bug/1863710/+attachment/5331124/+files/win10.xml

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

Title:
  qemu 4.2 does not process discard(trim) commands

Status in QEMU:
  New

Bug description:
  I'm using Arch Linux with qemu 4.2 and blktrace to monitor discard
  commands as they are sent to the hardware.  Blktrace shows nothing as
  the VM is trimming the SSDs.

  I downgraded to qemu 4.1.1 and blktrace shows lots of discard commands
  as the VM is trimming.

  Kernel version is 5.5.4.

  Attached is the libvirt xml.

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

Re: [PATCH] hw/ide: Remove status register read side effect

2020-02-25 Thread jasper.lowell

> Problem with that patch is that it removes this clearing from the
> func 
> that's also used to emulate ISA IDE ioports which according to their
> spec 
> should clear irq on read so that function should be OK but maybe
> should 
> not be called by PCI IDE code?

This might be it.

The patch I provided is definitely incorrect and deviates from the
specification as Mark mentioned earlier. I misunderstood what
ide_ioport_read/write were for and haven't been thinking about legacy
mode. 

The bug that I believe exists is present when the CMD646 is operating
in PCI native mode. Yeah, I think a possible solution might be to avoid
using the ioport_read/write functions from the PCI code if they have
side effects that assume the device is in legacy mode. I'll have to
spend more time reading through the code and documentation.

> Except the legacy IDE spec that does say reading status is clearing
> IRQ 
> but not sure PCI native mode should do the same but it seems to use
> the 
> same function in QEMU so it will clear IRQ as in legacy IDE mode.

According to the CMD646U2 specification:
"When an IDE port is in PCI IDE Legacy Mode, the PCI646U2 is compatible
with standard ISA IDE. The IDE task file registers are mapped to the
standard ISA port addresses, and IDE drive interrupts occur at IRQ14
(primary) or IRQ15 (secondary)."

In legacy mode, IRQ14 and IRQ15 mirror the state of INTRQ on each of
the selected IDE devices. QEMU appears to emulate this correctly.

In PCI native mode, INTRQ is not mirrored or given a single IRQ.
Interrupts are provided by the PCI IDE controller depending on the
controller's logic. For instance, an IDE device can raise an interrupt
but the CMD646 may not propagate that interrupt if MRDMODE has certain
bits set. I'm thinking that maybe the controller does not have logic to
unset the interrupt bits in CFR and ARTTIM23 when the IDE device lowers
INTRQ. This might mean that the controller will continue to assert an
interrupt while bits in CFR and ARTTIM23 remain set, even if the IDE
device lowers INTRQ. This would explain why the CMD646 documentation
instructs developers to lower them explicitly.

> Except the legacy IDE spec that does say reading status is clearing
> IRQ 
> but not sure PCI native mode should do the same but it seems to use
> the 
> same function in QEMU so it will clear IRQ as in legacy IDE mode. But
> this 
> Linux driver says IRQ is cleared on read for PCI as well:
> 
> 
https://github.com/torvalds/linux/blob/master/drivers/ata/libata-sff.c
> 
> as does the CMD646 driver:
> 
> 
https://github.com/torvalds/linux/blob/master/drivers/ata/pata_cmd64x.c
> 
> in cmd64x_sff_irq_check() although for different chip revisions it
> uses 
> cmd648_sff_irq_* functions which does this differently and avoids
> reading 
> status reg and clears irq explicitely. It also has a warning at the 
> beginning that UDMA mode is broken on most of these chips so it won't
> try 
> to use it on anything below CMD646U2 so this suggests maybe there's
> a 
> problem with clearing IRQs on at least some CMD646 chip revisions. I
> think 
> the Sun Ultra 10 used CMD646U but not sure what the Solaris driver
> expects 
> and if it can work with later chip revisions. Maybe we should either 
> emulate the chip bugs or change something to identify as CMD646U2
> which 
> should behave more like stadard PCI IDE controllers? Although if I
> got 
> that correctly Linux thinks revisions over 5 are OK and QEMU has 7.

I'm not sure what it expects. If the Sun Ultra 10 shipped with the
CMD646U, I reason that Solaris 10 either expects it or has support for
it.

The Linux driver code appears to be consistent with the behaviour that
I'm seeing from Solaris 10.

The following appears to be used to initialise the CMD646U.

{   /* CMD 646U with broken UDMA */
.flags = ATA_FLAG_SLAVE_POSS,
.pio_mask = ATA_PIO4,
.mwdma_mask = ATA_MWDMA2,
.port_ops = &cmd646r3_port_ops
},

The port operations it uses are defined as so:

static struct ata_port_operations cmd646r3_port_ops = {
.inherits   = &cmd64x_base_ops,
.sff_irq_check  = cmd648_sff_irq_check,
.sff_irq_clear  = cmd648_sff_irq_clear,
.cable_detect   = ata_cable_40wire,
}

As you mention, cmd648_sff_irq_clear clears interrupts explicitly by
setting bits in MRDMODE - consistent with the CMD646U2 documentation.
This behaviour is very similar to Solaris 10.

> Although if I got 
> that correctly Linux thinks revisions over 5 are OK and QEMU has 7.

I'm not sure how revision numbers work with these chips. Do CMD646 and
CMD646U2 refer to different revisions of the CMD646 chip?

Thanks,
Jasper Lowell.

On Tue, 2020-02-25 at 16:08 +0100, BALATON Zoltan wrote:
> On Tue, 25 Feb 2020, jasper.low...@bt.com wrote:
I don't believe the
> quick interrupt here is the problem. Solaris 10
will spin for a short
> time while waiting for the interrupt bit to be
set before continuing
> with its routine. If it doesn't see the in

Re: [PATCH 1/6] block: add bitmap-populate job

2020-02-25 Thread Vladimir Sementsov-Ogievskiy


25.02.2020 23:41, John Snow wrote:



On 2/25/20 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:

25.02.2020 3:56, John Snow wrote:

This job copies the allocation map into a bitmap. It's a job because
there's no guarantee that allocation interrogation will be quick (or
won't hang), so it cannot be retrofit into block-dirty-bitmap-merge.

It was designed with different possible population patterns in mind,
but only top layer allocation was implemented for now.

Signed-off-by: John Snow 
---
   qapi/block-core.json  |  48 +
   qapi/job.json |   2 +-
   include/block/block_int.h |  21 
   block/bitmap-alloc.c  | 207 ++
   blockjob.c    |   3 +-
   block/Makefile.objs   |   1 +
   6 files changed, 280 insertions(+), 2 deletions(-)
   create mode 100644 block/bitmap-alloc.c

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 85e27bb61f..df1797681a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2245,6 +2245,54 @@
     { 'command': 'block-dirty-bitmap-merge',
   'data': 'BlockDirtyBitmapMerge' }
   +##
+# @BitmapPattern:
+#
+# An enumeration of possible patterns that can be written into a bitmap.
+#
+# @allocation-top: The allocation status of the top layer
+#  of the attached storage node.
+#
+# Since: 5.0
+##
+{ 'enum': 'BitmapPattern',
+  'data': ['allocation-top'] }
+
+##
+# @BlockDirtyBitmapPopulate:
+#
+# @job-id: identifier for the newly-created block job.
+#
+# @pattern: What pattern should be written into the bitmap?
+#
+# @on-error: the action to take if an error is encountered on a bitmap's
+#    attached node, default 'report'.
+#    'stop' and 'enospc' can only be used if the block device
supports
+#    io-status (see BlockInfo).
+#
+# @auto-finalize: When false, this job will wait in a PENDING state
after it has
+# finished its work, waiting for @block-job-finalize
before
+# making any block graph changes.


sounds a bit strange in context of bitmap-population job



Yeah, you're right. Copy-pasted for "consistency".


+# When true, this job will automatically
+# perform its abort or commit actions.
+# Defaults to true.
+#
+# @auto-dismiss: When false, this job will wait in a CONCLUDED state
after it
+#    has completely ceased all work, and awaits
@block-job-dismiss.
+#    When true, this job will automatically disappear
from the query
+#    list without user intervention.
+#    Defaults to true.
+#
+# Since: 5.0
+##
+{ 'struct': 'BlockDirtyBitmapPopulate',
+  'base': 'BlockDirtyBitmap',
+  'data': { 'job-id': 'str',
+    'pattern': 'BitmapPattern',
+    '*on-error': 'BlockdevOnError',
+    '*auto-finalize': 'bool',
+    '*auto-dismiss': 'bool' } }
+
   ##
   # @BlockDirtyBitmapSha256:
   #
diff --git a/qapi/job.json b/qapi/job.json
index 5e658281f5..5f496d4630 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -22,7 +22,7 @@
   # Since: 1.7
   ##
   { 'enum': 'JobType',
-  'data': ['commit', 'stream', 'mirror', 'backup', 'create'] }
+  'data': ['commit', 'stream', 'mirror', 'backup', 'create',
'bitmap-populate'] }
     ##
   # @JobStatus:
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6f9fd5e20e..a5884b597e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1215,6 +1215,27 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
   BlockCompletionFunc *cb, void *opaque,
   JobTxn *txn, Error **errp);
   +/*
+ * bitpop_job_create: Create a new bitmap population job.
+ *
+ * @job_id: The id of the newly-created job.
+ * @bs: Block device associated with the @target_bitmap.
+ * @target_bitmap: The bitmap to populate.
+ * @on_error: What to do if an error on @bs is encountered.
+ * @creation_flags: Flags that control the behavior of the Job lifetime.
+ *  See @BlockJobCreateFlags
+ * @cb: Completion function for the job.
+ * @opaque: Opaque pointer value passed to @cb.
+ * @txn: Transaction that this job is part of (may be NULL).
+ */
+BlockJob *bitpop_job_create(const char *job_id, BlockDriverState *bs,
+    BdrvDirtyBitmap *target_bitmap,
+    BitmapPattern pattern,
+    BlockdevOnError on_error,
+    int creation_flags,
+    BlockCompletionFunc *cb, void *opaque,
+    JobTxn *txn, Error **errp);
+
   void hmp_drive_add_node(Monitor *mon, const char *optstr);
     BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
diff --git a/block/bitmap-alloc.c b/block/bitmap-alloc.c
new file mode 100644
index 00..47d542dc12
--- /dev/null
+++ b/block/bitmap-alloc.c
@@ -0,0 +1,207 @@
+/*
+ * Async Di

Re: [PULL 00/32] virtio, pc: fixes, features

2020-02-25 Thread no-reply

Patchew URL: https://patchew.org/QEMU/20200225151210.647797-1-...@redhat.com/



Hi,

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

Subject: [PULL 00/32] virtio, pc: fixes, features
Message-id: 20200225151210.647797-1-...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag] patchew/20200225151210.647797-1-...@redhat.com -> 
patchew/20200225151210.647797-1-...@redhat.com
Switched to a new branch 'test'
1c37a27 Fixed assert in vhost_user_set_mem_table_postcopy
bed1571 virtiofsd: add it to the tools list
9295d98 tests/vhost-user-bridge: move to contrib/
218b70f vhost-user: only set slave channel for first vq
5351c9b acpi: cpuhp: document CPHP_GET_CPU_ID_CMD command
082b014 libvhost-user: implement in-band notifications
84760e4 docs: vhost-user: add in-band kick/call messages
314ca4f libvhost-user: handle NOFD flag in call/kick/err better
57e2969 libvhost-user-glib: use g_main_context_get_thread_default()
2e9a28a libvhost-user-glib: fix VugDev main fd cleanup
abeaf81 libvhost-user: implement VHOST_USER_PROTOCOL_F_REPLY_ACK
b2dcd9c MAINTAINERS: add virtio-iommu related files
c2b7c49 hw/arm/virt: Add the virtio-iommu device tree mappings
e7d8ad0 virtio-iommu-pci: Add virtio iommu pci support
13a909a virtio-iommu: Support migration
3bc77b7 virtio-iommu: Implement fault reporting
4f6df8c virtio-iommu: Implement translate
eb6ee62 virtio-iommu: Implement map/unmap
c0d89aa virtio-iommu: Implement attach/detach command
30de952 virtio-iommu: Decode the command payload
faa62bf virtio-iommu: Add skeleton
1a2cc03 virtio: gracefully handle invalid region caches
a9d2299 vhost-user-blk: convert to new virtio_delete_queue
f5dba2c vhost-user-blk: delete virtioqueues in unrealize to fix memleaks
b022556 virtio-crypto: do delete ctrl_vq in virtio_crypto_device_unrealize
b7586a0 virtio-pmem: do delete rq_vq in virtio_pmem_unrealize
3d33901 vhost-user-fs: convert to the new virtio_delete_queue function
fc938b8 vhost-user-fs: do delete virtio_queues in unrealize
24dee37 rebuild-expected-aml.sh: remind about the process
908d585 bios-tables-test: default diff command
6047460 bios-tables-test: fix up DIFF generation
98eabca bios-tables-test: tell people how to update

=== OUTPUT BEGIN ===
1/32 Checking commit 98eabca21985 (bios-tables-test: tell people how to update)
2/32 Checking commit 60474600983e (bios-tables-test: fix up DIFF generation)
3/32 Checking commit 908d58520019 (bios-tables-test: default diff command)
WARNING: line over 80 characters
#30: FILE: tests/qtest/bios-tables-test.c:471:
+ exp_sdt->asl_file, 
sdt->asl_file);

total: 0 errors, 1 warnings, 36 lines checked

Patch 3/32 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/32 Checking commit 24dee376fa13 (rebuild-expected-aml.sh: remind about the 
process)
ERROR: line over 90 characters
#22: FILE: tests/data/acpi/rebuild-expected-aml.sh:34:
+old_allowed_dif=`grep -v -e 'List of comma-separated changed AML files to 
ignore' ${SRC_PATH}/tests/qtest/bios-tables-test-allowed-diff.h`

ERROR: line over 90 characters
#30: FILE: tests/data/acpi/rebuild-expected-aml.sh:42:
+echo "Note! Please follow the process documented in 
${SRC_PATH}/tests/qtest/bios-tables-test.c"

total: 2 errors, 0 warnings, 13 lines checked

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

5/32 Checking commit fc938b8bf16f (vhost-user-fs: do delete virtio_queues in 
unrealize)
6/32 Checking commit 3d3390160abf (vhost-user-fs: convert to the new 
virtio_delete_queue function)
WARNING: line over 80 characters
#26: FILE: hw/virtio/vhost-user-fs.c:212:
+fs->hiprio_vq = virtio_add_queue(vdev, fs->conf.queue_size, 
vuf_handle_output);

WARNING: line over 80 characters
#32: FILE: hw/virtio/vhost-user-fs.c:217:
+fs->req_vqs[i] = virtio_add_queue(vdev, fs->conf.queue_size, 
vuf_handle_output);

total: 0 errors, 2 warnings, 48 lines checked

Patch 6/32 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/32 Checking commit b7586a03bd40 (virtio-pmem: do delete rq_vq in 
virtio_pmem_unrealize)
8/32 Checking commit b02255628ccc (virtio-crypto: do delete ctrl_vq in 
virtio_crypto_device_unrealize)
9/32 Checking commit f5dba2cc1755 (vhost-user-blk: delete virtioqueues in 
unrealize to fix memleaks)
10/32 Checking commit a9d229943363 (vhost-user-blk: convert to new 
virtio_delete_queue)
11/32 Checking commit 1a2cc0393dac (virtio:

[PATCH] MAINTAINERS: Add entry for Guest X86 HAXM CPUs

2020-02-25 Thread Colin Xu

HAXM covers below files:
include/sysemu/hax.h
target/i386/hax-*

Cc: Wenchao Wang 
Cc: Hang Yuan 
Signed-off-by: Colin Xu 
---
 MAINTAINERS | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 36d94c17a654..27727e2fac13 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -435,6 +435,16 @@ F: include/hw/block/dataplane/xen*
 F: include/hw/xen/
 F: include/sysemu/xen-mapcache.h
 
+Guest CPU Cores (HAXM)
+-
+X86 HAXM CPUs
+M: Wenchao Wang 
+M: Colin Xu 
+L: haxm-t...@intel.com
+S: Maintained
+F: include/sysemu/hax.h
+F: target/i386/hax-*
+
 Hosts
 -
 LINUX
-- 
2.25.1

Re: [PATCH 1/2] aspeed/smc: Add some tracing

2020-02-25 Thread Joel Stanley

n

On Thu, 6 Feb 2020 at 11:27, Cédric Le Goater  wrote:
>
> Signed-off-by: Cédric Le Goater 

Reviewed-by: Joel Stanley 


> ---
>  Makefile.objs   |  1 +
>  hw/ssi/aspeed_smc.c | 17 +
>  hw/ssi/trace-events |  9 +
>  3 files changed, 27 insertions(+)
>  create mode 100644 hw/ssi/trace-events
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 26b9cff95436..9e4ba95794e9 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -168,6 +168,7 @@ trace-events-subdirs += hw/scsi
>  trace-events-subdirs += hw/sd
>  trace-events-subdirs += hw/sparc
>  trace-events-subdirs += hw/sparc64
> +trace-events-subdirs += hw/ssi
>  trace-events-subdirs += hw/timer
>  trace-events-subdirs += hw/tpm
>  trace-events-subdirs += hw/usb
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 23c8d2f06245..e5621bf728ca 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -31,6 +31,7 @@
>  #include "qapi/error.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/units.h"
> +#include "trace.h"
>
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
> @@ -513,6 +514,8 @@ static void aspeed_smc_flash_set_segment(AspeedSMCState 
> *s, int cs,
>
>  s->ctrl->reg_to_segment(s, new, &seg);
>
> +trace_aspeed_smc_flash_set_segment(cs, new, seg.addr, seg.addr + 
> seg.size);
> +
>  /* The start address of CS0 is read-only */
>  if (cs == 0 && seg.addr != s->ctrl->flash_window_base) {
>  qemu_log_mask(LOG_GUEST_ERROR,
> @@ -753,6 +756,8 @@ static uint64_t aspeed_smc_flash_read(void *opaque, 
> hwaddr addr, unsigned size)
>__func__, aspeed_smc_flash_mode(fl));
>  }
>
> +trace_aspeed_smc_flash_read(fl->id, addr, size, ret,
> +aspeed_smc_flash_mode(fl));
>  return ret;
>  }
>
> @@ -808,6 +813,9 @@ static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl,  
> uint64_t data,
>  AspeedSMCState *s = fl->controller;
>  uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3;
>
> +trace_aspeed_smc_do_snoop(fl->id, s->snoop_index, s->snoop_dummies,
> +  (uint8_t) data & 0xff);
> +
>  if (s->snoop_index == SNOOP_OFF) {
>  return false; /* Do nothing */
>
> @@ -858,6 +866,9 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr 
> addr, uint64_t data,
>  AspeedSMCState *s = fl->controller;
>  int i;
>
> +trace_aspeed_smc_flash_write(fl->id, addr, size, data,
> + aspeed_smc_flash_mode(fl));
> +
>  if (!aspeed_smc_is_writable(fl)) {
>  qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%"
>HWADDR_PRIx "\n", __func__, addr);
> @@ -972,6 +983,9 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr 
> addr, unsigned int size)
>  (s->ctrl->has_dma && addr == R_DMA_CHECKSUM) ||
>  (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) ||
>  (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->ctrl->max_slaves)) {
> +
> +trace_aspeed_smc_read(addr, size, s->regs[addr]);
> +
>  return s->regs[addr];
>  } else {
>  qemu_log_mask(LOG_UNIMP, "%s: not implemented: 0x%" HWADDR_PRIx "\n",
> @@ -1091,6 +1105,7 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
>__func__, s->regs[R_DMA_FLASH_ADDR]);
>  return;
>  }
> +trace_aspeed_smc_dma_checksum(s->regs[R_DMA_FLASH_ADDR], data);
>
>  /*
>   * When the DMA is on-going, the DMA registers are updated
> @@ -1225,6 +1240,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, 
> uint64_t data,
>
>  addr >>= 2;
>
> +trace_aspeed_smc_write(addr, size, data);
> +
>  if (addr == s->r_conf ||
>  (addr >= s->r_timings &&
>   addr < s->r_timings + s->ctrl->nregs_timings) ||
> diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events
> new file mode 100644
> index ..ffe531a500aa
> --- /dev/null
> +++ b/hw/ssi/trace-events
> @@ -0,0 +1,9 @@
> +# aspeed_smc.c
> +
> +aspeed_smc_flash_set_segment(int cs, uint64_t reg, uint64_t start, uint64_t 
> end) "CS%d segreg=0x%"PRIx64" [ 0x%"PRIx64" - 0x%"PRIx64" ]"
> +aspeed_smc_flash_read(int cs, uint64_t addr,  uint32_t size, uint64_t data, 
> int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d"
> +aspeed_smc_do_snoop(int cs, int index, int dummies, int data) "CS%d 
> index:0x%x dummies:%d data:0x%x"
> +aspeed_smc_flash_write(int cs, uint64_t addr,  uint32_t size, uint64_t data, 
> int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d"
> +aspeed_smc_read(uint64_t addr,  uint32_t size, uint64_t data) "@0x%" PRIx64 
> " size %u: 0x%" PRIx64
> +aspeed_smc_dma_checksum(uint32_t addr, uint32_t data) "0x%08x: 0x%08x"
> +aspeed_smc_write(uint64_t addr,  uint32_t size, uint64_t data) "@0x%" PRIx64 
> " size %u: 0x%" PRIx64
> --
> 2.21.1
>

[PATCH v2 2/2] util: add util function buffer_zero_avx512()

2020-02-25 Thread Robert Hoo

Intialize buffer_accel with this buffer_zero_avx512(), when Intel AVX512F is
available on host.

This function utilizes Intel AVX512 fundamental instructions which
is faster than its implementation with AVX2 (in my unit test, with
4K buffer, on CascadeLake SP, ~36% faster, buffer_zero_avx512() V.S.
buffer_zero_avx2()).

Signed-off-by: Robert Hoo 
---
 include/qemu/cpuid.h |  3 +++
 util/bufferiszero.c  | 64 
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/include/qemu/cpuid.h b/include/qemu/cpuid.h
index 6930170..09fc245 100644
--- a/include/qemu/cpuid.h
+++ b/include/qemu/cpuid.h
@@ -45,6 +45,9 @@
 #ifndef bit_AVX2
 #define bit_AVX2(1 << 5)
 #endif
+#ifndef bit_AVX512F
+#define bit_AVX512F(1 << 16)
+#endif
 #ifndef bit_BMI2
 #define bit_BMI2(1 << 8)
 #endif
diff --git a/util/bufferiszero.c b/util/bufferiszero.c
index bfb2605..2161628 100644
--- a/util/bufferiszero.c
+++ b/util/bufferiszero.c
@@ -63,11 +63,11 @@ buffer_zero_int(const void *buf, size_t len)
 }
 }
 
-#if defined(CONFIG_AVX2_OPT) || defined(__SSE2__)
+#if defined(CONFIG_AVX512F_OPT) || defined(CONFIG_AVX2_OPT) || 
defined(__SSE2__)
 /* Do not use push_options pragmas unnecessarily, because clang
  * does not support them.
  */
-#ifdef CONFIG_AVX2_OPT
+#if defined(CONFIG_AVX512F_OPT) || defined(CONFIG_AVX2_OPT)
 #pragma GCC push_options
 #pragma GCC target("sse2")
 #endif
@@ -104,7 +104,7 @@ buffer_zero_sse2(const void *buf, size_t len)
 
 return _mm_movemask_epi8(_mm_cmpeq_epi8(t, zero)) == 0x;
 }
-#ifdef CONFIG_AVX2_OPT
+#if defined(CONFIG_AVX512F_OPT) || defined(CONFIG_AVX2_OPT)
 #pragma GCC pop_options
 #endif
 
@@ -187,18 +187,54 @@ buffer_zero_avx2(const void *buf, size_t len)
 #pragma GCC pop_options
 #endif /* CONFIG_AVX2_OPT */
 
+#ifdef CONFIG_AVX512F_OPT
+#pragma GCC push_options
+#pragma GCC target("avx512f")
+#include 
+
+static bool
+buffer_zero_avx512(const void *buf, size_t len)
+{
+/* Begin with an unaligned head of 64 bytes.  */
+__m512i t = _mm512_loadu_si512(buf);
+__m512i *p = (__m512i *)(((uintptr_t)buf + 5 * 64) & -64);
+__m512i *e = (__m512i *)(((uintptr_t)buf + len) & -64);
+
+/* Loop over 64-byte aligned blocks of 256.  */
+while (p <= e) {
+__builtin_prefetch(p);
+if (unlikely(_mm512_test_epi64_mask(t, t))) {
+return false;
+}
+t = p[-4] | p[-3] | p[-2] | p[-1];
+p += 4;
+}
+
+t |= _mm512_loadu_si512(buf + len - 4 * 64);
+t |= _mm512_loadu_si512(buf + len - 3 * 64);
+t |= _mm512_loadu_si512(buf + len - 2 * 64);
+t |= _mm512_loadu_si512(buf + len - 1 * 64);
+
+return !_mm512_test_epi64_mask(t, t);
+
+}
+#pragma GCC pop_options
+#endif
+
+
 /* Note that for test_buffer_is_zero_next_accel, the most preferred
  * ISA must have the least significant bit.
  */
-#define CACHE_AVX21
-#define CACHE_SSE42
-#define CACHE_SSE24
+#define CACHE_AVX512F 1
+#define CACHE_AVX22
+#define CACHE_SSE44
+#define CACHE_SSE28
 
 /* Make sure that these variables are appropriately initialized when
  * SSE2 is enabled on the compiler command-line, but the compiler is
  * too old to support CONFIG_AVX2_OPT.
  */
-#ifdef CONFIG_AVX2_OPT
+#if defined(CONFIG_AVX512F_OPT) || defined(CONFIG_AVX2_OPT)
 # define INIT_CACHE 0
 # define INIT_ACCEL buffer_zero_int
 #else
@@ -211,6 +247,7 @@ buffer_zero_avx2(const void *buf, size_t len)
 
 static unsigned cpuid_cache = INIT_CACHE;
 static bool (*buffer_accel)(const void *, size_t) = INIT_ACCEL;
+static int length_to_accel = 64;
 
 static void init_accel(unsigned cache)
 {
@@ -226,10 +263,16 @@ static void init_accel(unsigned cache)
 fn = buffer_zero_avx2;
 }
 #endif
+#ifdef CONFIG_AVX512F_OPT
+if (cache & CACHE_AVX512F) {
+fn = buffer_zero_avx512;
+length_to_accel = 256;
+}
+#endif
 buffer_accel = fn;
 }
 
-#ifdef CONFIG_AVX2_OPT
+#if defined(CONFIG_AVX512F_OPT) || defined(CONFIG_AVX2_OPT)
 #include "qemu/cpuid.h"
 
 static void __attribute__((constructor)) init_cpuid_cache(void)
@@ -255,6 +298,9 @@ static void __attribute__((constructor)) 
init_cpuid_cache(void)
 if ((bv & 6) == 6 && (b & bit_AVX2)) {
 cache |= CACHE_AVX2;
 }
+if ((bv & 6) == 6 && (b & bit_AVX512F)) {
+cache |= CACHE_AVX512F;
+}
 }
 }
 cpuid_cache = cache;
@@ -277,7 +323,7 @@ bool test_buffer_is_zero_next_accel(void)
 
 static bool select_accel_fn(const void *buf, size_t len)
 {
-if (likely(len >= 64)) {
+if (likely(len >= length_to_accel)) {
 return buffer_accel(buf, len);
 }
 return buffer_zero_int(buf, len);
-- 
1.8.3.1

[PATCH v2 1/2] configure: introduce configure option avx512f

2020-02-25 Thread Robert Hoo

Introduce {enable,disable}-avx512f configure option. It is by default disabled.
Only when user explicitly enable-avx512f and compiling environment supports
AVX512F, CONFIG_AVX512F_OPT will be defined.

AVX512F instruction set is available since Intel Skylake.
More info:
https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf

Signed-off-by: Robert Hoo 
---
 configure | 41 +
 1 file changed, 41 insertions(+)

diff --git a/configure b/configure
index d57261e..a0b41ce 100755
--- a/configure
+++ b/configure
@@ -1395,6 +1395,11 @@ for opt do
   ;;
   --enable-avx2) avx2_opt="yes"
   ;;
+  --disable-avx512f) avx512f_opt="no"
+  ;;
+  --enable-avx512f) avx512f_opt="yes"
+  ;;
+
   --enable-glusterfs) glusterfs="yes"
   ;;
   --disable-virtio-blk-data-plane|--enable-virtio-blk-data-plane)
@@ -1825,6 +1830,7 @@ disabled with --disable-FEATURE, default is enabled if 
available:
   tcmalloctcmalloc support
   jemallocjemalloc support
   avx2AVX2 optimization support
+  avx512f AVX512F optimization support
   replication replication support
   opengl  opengl support
   virglrenderer   virgl rendering support
@@ -5518,6 +5524,36 @@ EOF
   fi
 fi
 
+##
+# avx512f optimization requirement check
+#
+# There is no point enabling this if cpuid.h is not usable,
+# since we won't be able to select the new routines.
+# by default, it is turned off.
+# if user explicitly want to enable it, check environment
+
+if test "$cpuid_h" = "yes" && test "$avx512f_opt" = "yes"; then
+  cat > $TMPC << EOF
+#pragma GCC push_options
+#pragma GCC target("avx512f")
+#include 
+#include 
+static int bar(void *a) {
+__m512i x = *(__m512i *)a;
+return _mm512_test_epi64_mask(x, x);
+}
+int main(int argc, char *argv[])
+{
+   return bar(argv[0]);
+}
+EOF
+  if ! compile_object "" ; then
+avx512f_opt="no"
+  fi
+else
+  avx512f_opt="no"
+fi
+
 
 # check if __[u]int128_t is usable.
 
@@ -6650,6 +6686,7 @@ echo "libxml2   $libxml2"
 echo "tcmalloc support  $tcmalloc"
 echo "jemalloc support  $jemalloc"
 echo "avx2 optimization $avx2_opt"
+echo "avx512f optimization $avx512f_opt"
 echo "replication support $replication"
 echo "VxHS block device $vxhs"
 echo "bochs support $bochs"
@@ -7200,6 +7237,10 @@ if test "$avx2_opt" = "yes" ; then
   echo "CONFIG_AVX2_OPT=y" >> $config_host_mak
 fi
 
+if test "$avx512f_opt" = "yes" ; then
+  echo "CONFIG_AVX512F_OPT=y" >> $config_host_mak
+fi
+
 if test "$lzo" = "yes" ; then
   echo "CONFIG_LZO=y" >> $config_host_mak
 fi
-- 
1.8.3.1

[PATCH v2 0/2] Add AVX512F optimization option and buffer_zero_avx512()

2020-02-25 Thread Robert Hoo

1) Introduce {enable,disable}-avx512f configure option

2) Implement new buffer_zero_avx512() with AVX512F instructions

Changes in v2:
1. Fixes wrong definition of CACHE_SSE2 in v1.
2. Fixes not handle <256 length case in buffer_zero_avx512() implementaion.
(Follow Richard's suggestion: handle the case in select_accel_fn(), and have a
global variable alongside buffer_accel)
3. Changes avx512f configuration option's default status to disabled.
4. Ran 'make check-unit' on this patch, on both a Ivybridge machine and a
CascadeLake machine.

Robert Hoo (2):
  configure: add configure option avx512f_opt
  util: add util function buffer_zero_avx512()

 configure| 41 +
 include/qemu/cpuid.h |  3 +++
 util/bufferiszero.c  | 64 
 3 files changed, 99 insertions(+), 9 deletions(-)

-- 
1.8.3.1

Re: [PATCH 1/1] hw/net/can: Introduce Xlnx ZynqMP CAN controller for QEMU




On 2020/2/25 下午2:22, Vikram Garhwal wrote:

Hi Jason,
Apologies for the delayed response. I tried plugging NetClientState in the CAN 
which is required if we use qemu_send_packet but this will change the 
underlying architecture of can-core, can-socketcan a lot. This means changes 
the way CAN bus is created/works and socket CAN works. CAN Socket(CAN Raw 
socket) is much different from Ethernet so plugging/using NetClient state is 
not working here.



I get you.




I apologize for still being a little confused about the filters but when 
looking into the code, I can only find them being used with ethernet frames. 
Since no other can controller uses NetClientState it makes me wonder if this 
model perhaps was thought of being an ethernet NIC?



Nope NetclientState is not necessarily a NIC, it can be a peer of the 
NIC (e.g network backend like tap, hubport etc).




Or has the code in net/can/ which I referenced been obsoleted?



No :)




Sharing this link for SocketCAN(in case you want to have a look): 
https://www.kernel.org/doc/Documentation/networking/can.txt. Section 4 talks on 
how CAN Socket is intended to work. Equivalent file is located as 
net/can-socketcan.c.



Thanks for the pointer.

I agree that there's no need to change that part. But we may consider to 
unify the CanBusClientState and NetClientState in the future.



  
Regards,

Vikram


-Original Message-
From: Jason Wang 
Sent: Monday, February 10, 2020 7:09 PM
To: Vikram Garhwal ; qemu-devel@nongnu.org
Subject: Re: [PATCH 1/1] hw/net/can: Introduce Xlnx ZynqMP CAN controller
for QEMU


On 2020/2/11 上午5:45, Vikram Garhwal wrote:

+}
+} else {
+/* Normal mode Tx. */
+generate_frame(&frame, data);
+
+can_bus_client_send(&s->bus_client, &frame, 1);

I had a quick glance at can_bus_client_send():

It did:

       QTAILQ_FOREACH(peer, &bus->clients, next) {
       if (peer->info->can_receive(peer)) {
       if (peer == client) {
       /* No loopback support for now */
       continue;
       }
       if (peer->info->receive(peer, frames, frames_cnt) > 0) {
       ret = 1;
       }
       }
       }

which looks not correct. We need to use qemu_send_packet() instead of
calling peer->info->receive() directly which bypasses filters completely.

[Vikram Garhwal] Can you please elaborate it bit more on why do we need

to filter outgoing message? So, I can either add a filter before sending the
packets. I am unable to understand the use case for it. For any message which
is incoming, we are filtering it for sure before storing in update_rx_fifo().


I might be not clear, I meant the netfilters supported by qemu which allows
you to attach a filter to a specific NetClientState, see
qemu_send_packet_async_with_flags. It doesn't mean the filter implemented
in your own NIC model.

Thanks



Also, I can see existing CAN models like CAN sja1000 and CAN Kavser are

using it same can_bus_client_send() function. However, this doesn't mean
that it is the correct way to send & receive packets.

[PATCH 1/2] block/qcow2: do free crypto_opts in qcow2_close()

2020-02-25 Thread Pan Nengyuan

'crypto_opts' forgot to free in qcow2_close(), this patch fix the bellow leak 
stack:

Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f0edd81f970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f0edc6d149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x55d7eaede63d in qobject_input_start_struct 
/mnt/sdb/qemu-new/qemu_test/qemu/qapi/qobject-input-visitor.c:295
#3 0x55d7eaed78b8 in visit_start_struct 
/mnt/sdb/qemu-new/qemu_test/qemu/qapi/qapi-visit-core.c:49
#4 0x55d7eaf5140b in visit_type_QCryptoBlockOpenOptions 
qapi/qapi-visit-crypto.c:290
#5 0x55d7eae43af3 in block_crypto_open_opts_init 
/mnt/sdb/qemu-new/qemu_test/qemu/block/crypto.c:163
#6 0x55d7eacd2924 in qcow2_update_options_prepare 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1148
#7 0x55d7eacd33f7 in qcow2_update_options 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1232
#8 0x55d7eacd9680 in qcow2_do_open 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1512
#9 0x55d7eacdc55e in qcow2_open_entry 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1792
#10 0x55d7eacdc8fe in qcow2_open 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1819
#11 0x55d7eac3742d in bdrv_open_driver 
/mnt/sdb/qemu-new/qemu_test/qemu/block.c:1317
#12 0x55d7eac3e990 in bdrv_open_common 
/mnt/sdb/qemu-new/qemu_test/qemu/block.c:1575
#13 0x55d7eac4442c in bdrv_open_inherit 
/mnt/sdb/qemu-new/qemu_test/qemu/block.c:3126
#14 0x55d7eac45c3f in bdrv_open 
/mnt/sdb/qemu-new/qemu_test/qemu/block.c:3219
#15 0x55d7ead8e8a4 in blk_new_open 
/mnt/sdb/qemu-new/qemu_test/qemu/block/block-backend.c:397
#16 0x55d7eacde74c in qcow2_co_create 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3534
#17 0x55d7eacdfa6d in qcow2_co_create_opts 
/mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3668
#18 0x55d7eac1c678 in bdrv_create_co_entry 
/mnt/sdb/qemu-new/qemu_test/qemu/block.c:485
#19 0x55d7eb0024d2 in coroutine_trampoline 
/mnt/sdb/qemu-new/qemu_test/qemu/util/coroutine-ucontext.c:115

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 block/qcow2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 8dcee5efec..ac231b688e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2603,6 +2603,7 @@ static void qcow2_close(BlockDriverState *bs)
 
 qcrypto_block_free(s->crypto);
 s->crypto = NULL;
+qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
 
 g_free(s->unknown_header_fields);
 cleanup_unknown_header_ext(bs);
-- 
2.18.2

[PATCH 0/2] fix two small memleaks

2020-02-25 Thread Pan Nengyuan

This series fix two small memleaks.
1. 'crypto_opts' forgot to free in qcow2_close(), do this cleanup in 
qcow2_close();
2. Do free filename/format in collect_image_check() when we re-allocate it.  

Pan Nengyuan (2):
  block/qcow2: do free crypto_opts in qcow2_close()
  qemu-img: free memory before re-assign

 block/qcow2.c | 1 +
 qemu-img.c| 2 ++
 2 files changed, 3 insertions(+)

-- 
2.18.2

[PATCH 2/2] qemu-img: free memory before re-assign

2020-02-25 Thread Pan Nengyuan

collect_image_check() is called twice in img_check(), the filename/format will 
be alloced without free the original memory.
It is not a big deal since the process will exit anyway, but seems like a clean 
code and it will remove the warning spotted by asan.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 qemu-img.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 2b4562b9d9..bcbca6c9a2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -638,6 +638,8 @@ static int collect_image_check(BlockDriverState *bs,
 return ret;
 }
 
+g_free(check->filename);
+g_free(check->format);
 check->filename = g_strdup(filename);
 check->format   = g_strdup(bdrv_get_format_name(bs));
 check->check_errors = result.check_errors;
-- 
2.18.2

Re: [PATCH] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()




On 2020/2/25 下午6:18, Peter Maydell wrote:

On Tue, 25 Feb 2020 at 05:41, Jason Wang  wrote:


On 2020/2/25 上午10:59, Chen Qun wrote:

The current code causes clang static code analyzer generate warning:
hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read
  value = value & 0x000f;
  ^   ~~
hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read
  value = value & 0x00fd;
  ^   ~~

According to the definition of the function, the two “value” assignments
   should be written to registers.

Reported-by: Euler Robot 
Signed-off-by: Chen Qun 
---
I'm not sure if this modification is correct, just from the function
   definition, it is correct.
---
   hw/net/imx_fec.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 6a124a154a..92f6215712 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s, uint32_t 
index, uint32_t value)
   break;
   case ENET_TGSR:
   /* implement clear timer flag */
-value = value & 0x000f;
+s->regs[index] = value & 0x000f;
   break;

Hi; the datasheet for this SoC says that these bits
of the register are write-1-to-clear, so while this
is definitely a bug I don't think this is the right fix.


   case ENET_TCSR0:
   case ENET_TCSR1:
   case ENET_TCSR2:
   case ENET_TCSR3:
-value = value & 0x00fd;
+s->regs[index] = value & 0x00fd;
   break;

Here bit 7 is write-1-to-clear, though bits 0 and
2..5 are simple write-the-value.


   case ENET_TCCR0:
   case ENET_TCCR1:


Applied.

Could you drop this from your queue, please?

thanks
-- PMM



Sure, Chen please send V2 to address Peter's comment.

Thanks

[PATCH v3 1/1] target/riscv: add vector integer operations

2020-02-25 Thread LIU Zhiwei

Signed-off-by: LIU Zhiwei 
---
 target/riscv/helper.h   |  395 +++
 target/riscv/insn32.decode  |  127 +++
 target/riscv/insn_trans/trans_rvv.inc.c |  671 +++-
 target/riscv/vector_helper.c| 1308 ++-
 4 files changed, 2462 insertions(+), 39 deletions(-)

diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index cbe0d107c0..dee21b4128 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -296,3 +296,398 @@ DEF_HELPER_6(vamominw_v_w,  void, ptr, ptr, tl, ptr, env, 
i32)
 DEF_HELPER_6(vamomaxw_v_w,  void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamominuw_v_w, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamomaxuw_v_w, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_6(vadd_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vadd_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vadd_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vadd_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsub_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsub_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsub_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsub_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vand_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vand_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vand_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vand_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vor_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vor_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vor_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vor_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vxor_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vxor_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vxor_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vxor_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsll_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsll_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsll_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsll_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsrl_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsrl_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsrl_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsrl_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsra_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsra_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsra_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vsra_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vminu_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vminu_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vminu_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vminu_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmin_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmin_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmin_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmin_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmaxu_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmaxu_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmaxu_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmaxu_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmax_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmax_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmax_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmax_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmul_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmul_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmul_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmul_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulh_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulh_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulh_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulh_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhu_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhu_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhu_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhu_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhsu_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhsu_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhsu_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vmulhsu_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vdivu_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vdivu_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vdivu_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vdivu_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vdiv_vv_b, void, ptr, ptr, ptr, ptr, env, i32)

Re: [PATCH v6 17/18] spapr: Clean up RMA size calculation

2020-02-25 Thread David Gibson

On Tue, Feb 25, 2020 at 12:07:29PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/25/20 12:37 AM, David Gibson wrote:
> > Move the calculation of the Real Mode Area (RMA) size into a helper
> > function.  While we're there clean it up and correct it in a few ways:
> >* Add comments making it clearer where the various constraints come from
> >* Remove a pointless check that the RMA fits within Node 0 (we've just
> >  clamped it so that it does)
> > 
> > Signed-off-by: David Gibson 
> > ---
> >   hw/ppc/spapr.c | 59 ++
> >   1 file changed, 35 insertions(+), 24 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 6e9f15f64d..f0354b699d 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2648,6 +2648,40 @@ static PCIHostState *spapr_create_default_phb(void)
> >   return PCI_HOST_BRIDGE(dev);
> >   }
> > +static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp)
> > +{
> > +MachineState *machine = MACHINE(spapr);
> > +hwaddr rma_size = machine->ram_size;
> > +hwaddr node0_size = spapr_node0_size(machine);
> > +
> > +/* RMA has to fit in the first NUMA node */
> > +rma_size = MIN(rma_size, node0_size);
> > +
> > +/*
> > + * VRMA access is via a special 1TiB SLB mapping, so the RMA can
> > + * never exceed that
> > + */
> > +rma_size = MIN(rma_size, TiB);
> 
> Can you use '1 * TiB'? It makes review obvious.

Done.

> > +
> > +/*
> > + * Clamp the RMA size based on machine type.  This is for
> > + * migration compatibility with older qemu versions, which limited
> > + * the RMA size for complicated and mostly bad reasons.
> > + */
> > +if (smc->rma_limit) {
> > +spapr->rma_size = MIN(spapr->rma_size, smc->rma_limit);
> > +}
> > +
> > +if (rma_size < (MIN_RMA_SLOF * MiB)) {
> 
> This looks old copy/paste before the change "spapr: Don't use weird units
> for MIN_RMA_SLOF".
> 
> > +error_setg(errp,
> > +"pSeries SLOF firmware requires >= %ldMiB guest RMA (Real Mode Area)",
> > +   MIN_RMA_SLOF);
> 
> Similarly, "MIN_RMA_SLOF / MiB"?

Ah, good catch.  I re-ordered the series at some point and forgot to
fix this up.

> > +return -1;
> 
> Maybe return 0 in case this function is called with errp !=
> &error_fatal.

Good idea.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v6 09/18] target/ppc: Streamline calculation of RMA limit from LPCR[RMLS]

2020-02-25 Thread David Gibson

On Tue, Feb 25, 2020 at 11:47:25PM +0100, Greg Kurz wrote:
> On Tue, 25 Feb 2020 18:05:31 +0100
> Greg Kurz  wrote:
> 
> > On Tue, 25 Feb 2020 10:37:15 +1100
> > David Gibson  wrote:
> > 
> > > Currently we use a big switch statement in ppc_hash64_update_rmls() to 
> > > work
> > > out what the right RMA limit is based on the LPCR[RMLS] field.  There's no
> > > formula for this - it's just an arbitrary mapping defined by the existing
> > > CPU implementations - but we can make it a bit more readable by using a
> > > lookup table rather than a switch.  In addition we can use the MiB/GiB
> > > symbols to make it a bit clearer.
> > > 
> > > While there we add a bit of clarity and rationale to the comment about
> > > what happens if the LPCR[RMLS] doesn't contain a valid value.
> > > 
> > > Signed-off-by: David Gibson 
> > > Reviewed-by: Cédric Le Goater 
> > > ---
> > >  target/ppc/mmu-hash64.c | 71 -
> > >  1 file changed, 35 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > > index 0ef330a614..4f082d775d 100644
> > > --- a/target/ppc/mmu-hash64.c
> > > +++ b/target/ppc/mmu-hash64.c
> > > @@ -18,6 +18,7 @@
> > >   * License along with this library; if not, see 
> > > .
> > >   */
> > >  #include "qemu/osdep.h"
> > > +#include "qemu/units.h"
> > >  #include "cpu.h"
> > >  #include "exec/exec-all.h"
> > >  #include "exec/helper-proto.h"This tool was originally developed to fix 
> > > Linux CPU throttling issues affecting Lenovo T480 / T480s / X1C6 as 
> > > described here.
> > > @@ -757,6 +758,39 @@ static void ppc_hash64_set_c(PowerPCCPU *cpu, hwaddr 
> > > ptex, uint64_t pte1)
> > >  stb_phys(CPU(cpu)->as, base + offset, (pte1 & 0xff) | 0x80);
> > >  }
> > >  
> > > +static target_ulong rmls_limit(PowerPCCPU *cpu)
> > > +{
> > > +CPUPPCState *env = &cpu->env;
> > > +/*
> > > + * This is the full 4 bits encoding of POWER8. Previous
> > > + * CPUs only support a subset of these but the filtering
> > > + * is done when writing LPCR
> > > + */
> > > +const target_ulong rma_sizes[] = {
> > > +[0] = 0,
> > > +[1] = 16 * GiB,
> > > +[2] = 1 * GiB,
> > > +[3] = 64 * MiB,
> > > +[4] = 256 * MiB,
> > > +[5] = 0,
> > > +[6] = 0,
> > > +[7] = 128 * MiB,
> > > +[8] = 32 * MiB,
> > > +};
> > > +target_ulong rmls = (env->spr[SPR_LPCR] & LPCR_RMLS) >> 
> > > LPCR_RMLS_SHIFT;
> > > +
> > > +if (rmls < ARRAY_SIZE(rma_sizes)) {
> > 
> > This condition is always true since the RMLS field is 4-bit long... 
> 
> Oops my mistake, I was already thinking about the suggestion I have
> for something that was puzzling me. See below.
> 
> > I guess you want to check that RMLS encodes a valid RMA size instead.
> > 
> > if (rma_sizes[rmls]) {
> > 
> > > +return rma_sizes[rmls];
> > > +} else {
> > > +/*
> > > + * Bad value, so the OS has shot itself in the foot.  Return a
> > > + * 0-sized RMA which we expect to trigger an immediate DSI or
> > > + * ISI
> > > + */
> 
> It seems a bit weird to differentiate the case where the value is bad
> because it happens to be bigger than the highest supported one, compared
> to values that are declared bad in rma_sizes[], like 0, 5 or 6. They're
> all basically the same case of values not used to encode a valid
> size...

Right, but the result is the same either way - the function returns
0.  This is basically just a small space optimization.

> 
> What about :
> 
> static const target_ulong rma_sizes[16] = {
> [1] = 16 * GiB,
> [2] = 1 * GiB,
> [3] = 64 * MiB,
> [4] = 256 * MiB,
> [7] = 128 * MiB,
> [8] = 32 * MiB,
> };

Eh, I guess?  I don't see much to pick between them.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v6 06/18] spapr, ppc: Remove VPM0/RMLS hacks for POWER9

2020-02-25 Thread David Gibson

On Tue, Feb 25, 2020 at 04:58:01PM +0100, Greg Kurz wrote:
> On Tue, 25 Feb 2020 12:29:00 +0100
> Greg Kurz  wrote:
> 
> > On Tue, 25 Feb 2020 10:37:12 +1100
> > David Gibson  wrote:
> > 
> > > For the "pseries" machine, we use "virtual hypervisor" mode where we
> > > only model the CPU in non-hypervisor privileged mode.  This means that
> > > we need guest physical addresses within the modelled cpu to be treated
> > > as absolute physical addresses.
> > > 
> > > We used to do that by clearing LPCR[VPM0] and setting LPCR[RMLS] to a high
> > > limit so that the old offset based translation for guest mode applied,
> > > which does what we need.  However, POWER9 has removed support for that
> > > translation mode, which meant we had some ugly hacks to keep it working.
> > > 
> > > We now explicitly handle this sort of translation for virtual hypervisor
> > > mode, so the hacks aren't necessary.  We don't need to set VPM0 and RMLS
> > > from the machine type code - they're now ignored in vhyp mode.  On the cpu
> > > side we don't need to allow LPCR[RMLS] to be set on POWER9 in vhyp mode -
> > > that was only there to allow the hack on the machine side.
> > > 
> > > Signed-off-by: David Gibson 
> > > Reviewed-by: Cédric Le Goater 
> > > ---
> > 
> > Reviewed-by: Greg Kurz 
> > 
> 
> Ah wait...
> 
> > >  hw/ppc/spapr_cpu_core.c | 6 +-
> > >  target/ppc/mmu-hash64.c | 8 
> > >  2 files changed, 1 insertion(+), 13 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index d09125d9af..ea5e11f1d9 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -58,14 +58,10 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
> > >   * we don't get spurious wakups before an RTAS start-cpu call.
> > >   * For the same reason, set PSSCR_EC.
> > >   */
> > > -lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | 
> > > pcc->lpcr_pm);
> > > +lpcr &= ~(LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
> 
> ... a few lines above, we have a comment that should be dropped as well.
> 
>  * Clearing VPM0 will also cause us to use RMOR in mmu-hash64.c for
>  * real mode accesses, which thankfully defaults to 0 and isn't
>  * accessible in guest mode.

Removed, thanks.

> 
> My R-b tag stands anyway.
> 
> > >  lpcr |= LPCR_LPES0 | LPCR_LPES1;
> > >  env->spr[SPR_PSSCR] |= PSSCR_EC;
> > >  
> > > -/* Set RMLS to the max (ie, 16G) */
> > > -lpcr &= ~LPCR_RMLS;
> > > -lpcr |= 1ull << LPCR_RMLS_SHIFT;
> > > -
> > >  ppc_store_lpcr(cpu, lpcr);
> > >  
> > >  /* Set a full AMOR so guest can use the AMR as it sees fit */
> > > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > > index e372c42add..caf47ad6fc 100644
> > > --- a/target/ppc/mmu-hash64.c
> > > +++ b/target/ppc/mmu-hash64.c
> > > @@ -1126,14 +1126,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong 
> > > val)
> > >(LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | 
> > > LPCR_EEE |
> > >LPCR_DEE | LPCR_OEE)) | LPCR_MER | LPCR_GTSE | 
> > > LPCR_TC |
> > >LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE);
> > > -/*
> > > - * If we have a virtual hypervisor, we need to bring back RMLS. 
> > > It
> > > - * doesn't exist on an actual P9 but that's all we know how to
> > > - * configure with softmmu at the moment
> > > - */
> > > -if (cpu->vhyp) {
> > > -lpcr |= (val & LPCR_RMLS);
> > > -}
> > >  break;
> > >  default:
> > >  g_assert_not_reached();
> > 
> > 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v2 1/2] linux-user: Protect more syscalls

2020-02-25 Thread Alistair Francis

On Tue, Feb 25, 2020 at 3:59 AM Laurent Vivier  wrote:
>
> Le 25/02/2020 à 00:21, Alistair Francis a écrit :
> > New y2038 safe 32-bit architectures (like RISC-V) don't support old
> > syscalls with a 32-bit time_t. The kernel defines new *_time64 versions
> > of these syscalls. Add some more #ifdefs to syscall.c in linux-user to
> > allow us to compile without these old syscalls.
> >
> > Signed-off-by: Alistair Francis 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > ---
> >  linux-user/strace.c  |  2 ++
> >  linux-user/syscall.c | 20 
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/linux-user/strace.c b/linux-user/strace.c
> > index 4f7130b2ff..6420ccd97b 100644
> > --- a/linux-user/strace.c
> > +++ b/linux-user/strace.c
> > @@ -775,6 +775,7 @@ print_syscall_ret_newselect(const struct syscallname 
> > *name, abi_long ret)
> >  #define TARGET_TIME_OOP  3   /* leap second in progress */
> >  #define TARGET_TIME_WAIT 4   /* leap second has occurred */
> >  #define TARGET_TIME_ERROR5   /* clock not synchronized */
> > +#ifdef TARGET_NR_adjtimex
> >  static void
> >  print_syscall_ret_adjtimex(const struct syscallname *name, abi_long ret)
> >  {
> > @@ -813,6 +814,7 @@ print_syscall_ret_adjtimex(const struct syscallname 
> > *name, abi_long ret)
> >
> >  qemu_log("\n");
> >  }
> > +#endif
> >
> >  UNUSED static struct flags access_flags[] = {
> >  FLAG_GENERIC(F_OK),
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 8d27d10807..5a2156f95a 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -742,8 +742,10 @@ safe_syscall3(ssize_t, read, int, fd, void *, buff, 
> > size_t, count)
> >  safe_syscall3(ssize_t, write, int, fd, const void *, buff, size_t, count)
> >  safe_syscall4(int, openat, int, dirfd, const char *, pathname, \
> >int, flags, mode_t, mode)
> > +#if defined(TARGET_NR_wait4)
> >  safe_syscall4(pid_t, wait4, pid_t, pid, int *, status, int, options, \
> >struct rusage *, rusage)
>
> safe_wait4 is also used in TARGET_NR_waitpid

Fixed!

Alistair

>
> Thanks,
> Laurent

[PATCH 1/2] iotests: add JobRunner class

The idea is that instead of increasing the arguments to job_run all the
time, create a more general-purpose job runner that can be subclassed to
do interesting things with.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/255|   9 +-
 tests/qemu-iotests/257|  12 ++-
 tests/qemu-iotests/287|  19 +++-
 tests/qemu-iotests/iotests.py | 176 --
 4 files changed, 158 insertions(+), 58 deletions(-)

diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255
index 4a4818bafb..513e9ebb58 100755
--- a/tests/qemu-iotests/255
+++ b/tests/qemu-iotests/255
@@ -71,8 +71,13 @@ with iotests.FilePath('t.qcow2') as disk_path, \
 result = vm.qmp_log('block-commit', job_id='job0', auto_finalize=False,
 device='overlay', top_node='mid')
 
-vm.run_job('job0', auto_finalize=False, pre_finalize=start_requests,
-auto_dismiss=True)
+class TestJobRunner(iotests.JobRunner):
+def on_pending(self, event):
+start_requests()
+super().on_pending(event)
+
+runner = TestJobRunner(vm, 'job0', auto_finalize=False, auto_dismiss=True)
+runner.run()
 
 vm.shutdown()
 
diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index 2a81f9e30c..e73b0c20b3 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -265,9 +265,15 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 ebitmap.clear()
 ebitmap.dirty_group(2)
 
-vm.run_job(job, auto_dismiss=True, auto_finalize=False,
-   pre_finalize=_callback,
-   cancel=(failure == 'simulated'))
+class TestJobRunner(iotests.JobRunner):
+def on_pending(self, event):
+_callback()
+super().on_pending(event)
+
+runner = TestJobRunner(vm, job, cancel=(failure == 'simulated'),
+   auto_finalize=False, auto_dismiss=True)
+runner.run()
+
 bitmaps = vm.query_bitmaps()
 log({'bitmaps': bitmaps}, indent=2)
 log('')
diff --git a/tests/qemu-iotests/287 b/tests/qemu-iotests/287
index 0ab58dc011..f06e6ff084 100755
--- a/tests/qemu-iotests/287
+++ b/tests/qemu-iotests/287
@@ -165,13 +165,22 @@ def test_bitmap_populate(config):
 if not config.disabled:
 ebitmap.dirty_group(2)
 
+
+class TestJobRunner(iotests.JobRunner):
+def on_pending(self, event):
+if config.mid_writes:
+perform_writes(drive0, 2)
+if not config.disabled:
+ebitmap.dirty_group(2)
+super().on_pending(event)
+
 job = populate(drive0, 'target', 'bitpop0')
 assert job['return'] == {'return': {}}
-vm.run_job(job['id'],
-   auto_dismiss=job['auto-dismiss'],
-   auto_finalize=job['auto-finalize'],
-   pre_finalize=pre_finalize,
-   cancel=config.cancel)
+job_runner = TestJobRunner(vm, job['id'],
+   auto_dismiss=job['auto-dismiss'],
+   auto_finalize=job['auto-finalize'],
+   cancel=config.cancel)
+job_runner.run()
 log('')
 
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3390fab021..37a8b4d649 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -460,6 +460,130 @@ def remote_filename(path):
 else:
 raise Exception("Protocol %s not supported" % (imgproto))
 
+
+class JobRunner:
+def __init__(self, vm, job,
+ use_log=True,
+ cancel=False,
+ auto_finalize=True,
+ auto_dismiss=False):
+self._vm = vm
+self._id = job
+self.logging = use_log
+self.cancel = cancel
+
+self._auto_finalize = auto_finalize
+self._auto_dismiss = auto_dismiss
+self._exited = False
+self._error = None
+
+match_device = {'data': {'device': job}}
+match_id = {'data': {'id': job}}
+self._events = {
+'BLOCK_JOB_COMPLETED': match_device,
+'BLOCK_JOB_CANCELLED': match_device,
+'BLOCK_JOB_ERROR': match_device,
+'BLOCK_JOB_READY': match_device,
+'BLOCK_JOB_PENDING': match_id,
+'JOB_STATUS_CHANGE': match_id
+}
+
+self._dispatch = {
+'created': self.on_create,
+'running': self.on_run,
+'paused': self.on_pause,
+'ready': self.on_ready,
+'standby': self.on_standby,
+'waiting': self.on_waiting,
+'pending': self.on_pending,
+'aborting': self.on_abort,
+'concluded': self.on_conclude,
+'null': self.on_null,
+}
+
+# Job events -- state chang

[PATCH 2/2] iotests: modify test 040 to use JobRunner

Instead of having somewhat reproduced it for itself.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/040 | 51 +-
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 90b59081ff..579dafc797 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -483,34 +483,33 @@ class TestErrorHandling(iotests.QMPTestCase):
   file=('top-dbg' if top_debug else 'top-file'),
   backing='mid-fmt')
 
+
+class TestJobRunner(iotests.JobRunner):
+expected_events = ('BLOCK_JOB_COMPLETED',
+   'BLOCK_JOB_ERROR',
+   'BLOCK_JOB_READY')
+
+def __init__(self, *args, test, **kwargs):
+super().__init__(*args, **kwargs)
+self.log = []
+self.test = test
+
+def on_pause(self, event):
+result = self._vm.qmp('block-job-resume', device=self._id)
+self.test.assert_qmp(result, 'return', {})
+super().on_pause(event)
+
+def on_block_job_event(self, event):
+if event['event'] not in self.expected_events:
+self.test.fail("Unexpected event: %s" % event)
+super().on_block_job_event(event)
+self.log.append(iotests.filter_qmp_event(event))
+
 def run_job(self, expected_events, error_pauses_job=False):
-match_device = {'data': {'device': 'job0'}}
-events = {
-'BLOCK_JOB_COMPLETED': match_device,
-'BLOCK_JOB_CANCELLED': match_device,
-'BLOCK_JOB_ERROR': match_device,
-'BLOCK_JOB_READY': match_device,
-}
-
-completed = False
-log = []
-while not completed:
-ev = self.vm.events_wait(events, timeout=5.0)
-if ev['event'] == 'BLOCK_JOB_COMPLETED':
-completed = True
-elif ev['event'] == 'BLOCK_JOB_ERROR':
-if error_pauses_job:
-result = self.vm.qmp('block-job-resume', device='job0')
-self.assert_qmp(result, 'return', {})
-elif ev['event'] == 'BLOCK_JOB_READY':
-result = self.vm.qmp('block-job-complete', device='job0')
-self.assert_qmp(result, 'return', {})
-else:
-self.fail("Unexpected event: %s" % ev)
-log.append(iotests.filter_qmp_event(ev))
-
+job = self.TestJobRunner(self.vm, 'job0', use_log=False, test=self)
+job.run()
 self.maxDiff = None
-self.assertEqual(expected_events, log)
+self.assertEqual(expected_events, job.log)
 
 def event_error(self, op, action):
 return {
-- 
2.21.1

Re: [PATCH 4/4] docs: Convert qemu-deprecated.texi to rST

2020-02-25 Thread Alistair Francis

On Tue, Feb 25, 2020 at 7:41 AM Peter Maydell  wrote:
>
> Convert the documentation of deprecated features to rST.
>
> We put the whole of this document into the system manual, though
> technically a few parts of it apply to qemu-img or qemu-nbd which are
> otherwise documented in tools/.
>
> We only make formatting fixes, except for one use of 'appendix' which
> we change to 'section' because this isn't an appendix in the Sphinx
> manual.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  Makefile   |   2 +-
>  MAINTAINERS|   2 +-
>  docs/system/deprecated.rst | 446 +
>  docs/system/index.rst  |   1 +
>  qemu-deprecated.texi   | 386 
>  qemu-doc.texi  |   4 -
>  6 files changed, 449 insertions(+), 392 deletions(-)
>  create mode 100644 docs/system/deprecated.rst
>  delete mode 100644 qemu-deprecated.texi
>
> diff --git a/Makefile b/Makefile
> index 28749d20401..ec4a4be8355 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1115,7 +1115,7 @@ txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt 
> docs/interop/qemu-ga-ref.txt
>  qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
> qemu-options.texi \
> qemu-option-trace.texi \
> -   qemu-deprecated.texi qemu-monitor.texi \
> +   qemu-monitor.texi \
> qemu-monitor-info.texi \
> docs/qemu-cpu-models.texi
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 195dd58cac1..546f2b83017 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2787,7 +2787,7 @@ F: contrib/gitdm/*
>
>  Incompatible changes
>  R: libvir-l...@redhat.com
> -F: qemu-deprecated.texi
> +F: docs/system/deprecated.rst
>
>  Build System
>  
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> new file mode 100644
> index 000..1eaa559079b
> --- /dev/null
> +++ b/docs/system/deprecated.rst
> @@ -0,0 +1,446 @@
> +Deprecated features
> +===
> +
> +In general features are intended to be supported indefinitely once
> +introduced into QEMU. In the event that a feature needs to be removed,
> +it will be listed in this section. The feature will remain functional
> +for 2 releases prior to actual removal. Deprecated features may also
> +generate warnings on the console when QEMU starts up, or if activated
> +via a monitor command, however, this is not a mandatory requirement.
> +
> +Prior to the 2.10.0 release there was no official policy on how
> +long features would be deprecated prior to their removal, nor
> +any documented list of which features were deprecated. Thus
> +any features deprecated prior to 2.10.0 will be treated as if
> +they were first deprecated in the 2.10.0 release.
> +
> +What follows is a list of all features currently marked as
> +deprecated.
> +
> +System emulator command line arguments
> +--
> +
> +``-machine enforce-config-section=on|off`` (since 3.1)
> +''
> +
> +The ``enforce-config-section`` parameter is replaced by the
> +``-global migration.send-configuration={on|off}`` option.
> +
> +``-no-kvm`` (since 1.3.0)
> +'
> +
> +The ``-no-kvm`` argument is now a synonym for setting ``-accel tcg``.
> +
> +``-usbdevice`` (since 2.10.0)
> +'
> +
> +The ``-usbdevice DEV`` argument is now a synonym for setting
> +the ``-device usb-DEV`` argument instead. The deprecated syntax
> +would automatically enable USB support on the machine type.
> +If using the new syntax, USB support must be explicitly
> +enabled via the ``-machine usb=on`` argument.
> +
> +``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
> +'''
> +
> +The 'file' driver for drives is no longer appropriate for character or host
> +devices and will only accept regular files (S_IFREG). The correct driver
> +for these file types is 'host_cdrom' or 'host_device' as appropriate.
> +
> +``-net ...,name=``\ *name* (since 3.1)
> +''
> +
> +The ``name`` parameter of the ``-net`` option is a synonym
> +for the ``id`` parameter, which should now be used instead.
> +
> +``-smp`` (invalid topologies) (since 3.1)
> +'
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs.
> +
> +However, historically it was possible to start QEMU with an incorrect 
> topology
> +where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies will be removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> +*sockets* * *cores* * *threads* = *maxcpus*.
> +
> +``-vnc acl`` (since 4.0.0)
> +''
> +
> +The ``acl`` option to the ``-vnc`` argument has

Re: [PATCH qemu v7 0/5] spapr: Kill SLOF

2020-02-25 Thread Alexey Kardashevskiy




On 21/02/2020 19:27, Paolo Bonzini wrote:
> On 21/02/20 01:18, Alexey Kardashevskiy wrote:
>> I am not quite sure I understood the request.  Write my own small
>> firmware and replace GRUB with it? The firmware from 5/5 reads first 2
>> sectors and the entire PReP, I could add there stuff if that helps (I
>> have "work in progress" patch for the firmware with printk/etc borrowed
>> from SLOF).
> 
> Okay, that's great!  I'll take a look next week.


Just to make sure I understood - you'll take a look on this series, you
do not expect other patches on top, right?



ps. while I have your attention, what was the practical reason for
including capstone to QEMU? Thanks,



> 
> Thanks,
> 
> Paolo
> 
>>>  (Also, I lost the pointer to your super-minimal
>>> pSeries firmware).
>>
>> It is incorporated into these patches under /pc-bios/vof - 4/5 has the
>> minimum (may be even too much), 5/5 has MBR+GPT+ELF.
> 
> 

-- 
Alexey

[Bug 1864704] Re: No compatible -machine option in qemu-system-ppc64 for e6500 core

2020-02-25 Thread Laurent Vivier

Try "-M ppce500 -cpu e6500"

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

Title:
  No compatible -machine option in qemu-system-ppc64 for e6500 core

Status in QEMU:
  New

Bug description:
  Hi,

  I'm trying to use qemu-system-ppc64 for emulating a QorIQ T2080 (with e6500 
cores). 
  However, I couldn't find any -machine option that matches -cpu e6500 option, 
which are listed below:

  C:\Program Files\qemu>qemu-system-ppc64 -machine help
  Supported machines are:
  40p  IBM RS/6000 7020 (40p)
  bamboo   bamboo
  g3beige  Heathrow based PowerMAC
  mac99Mac99 based PowerMAC
  mpc8544dsmpc8544ds
  none empty machine
  powernv8 IBM PowerNV (Non-Virtualized) POWER8
  powernv  IBM PowerNV (Non-Virtualized) POWER9 (alias of powernv9)
  powernv9 IBM PowerNV (Non-Virtualized) POWER9
  ppce500  generic paravirt e500 platform
  prep PowerPC PREP platform (deprecated)
  pseries-2.1  pSeries Logical Partition (PAPR compliant)
  pseries-2.10 pSeries Logical Partition (PAPR compliant)
  pseries-2.11 pSeries Logical Partition (PAPR compliant)
  pseries-2.12 pSeries Logical Partition (PAPR compliant)
  pseries-2.12-sxxmpSeries Logical Partition (PAPR compliant)
  pseries-2.2  pSeries Logical Partition (PAPR compliant)
  pseries-2.3  pSeries Logical Partition (PAPR compliant)
  pseries-2.4  pSeries Logical Partition (PAPR compliant)
  pseries-2.5  pSeries Logical Partition (PAPR compliant)
  pseries-2.6  pSeries Logical Partition (PAPR compliant)
  pseries-2.7  pSeries Logical Partition (PAPR compliant)
  pseries-2.8  pSeries Logical Partition (PAPR compliant)
  pseries-2.9  pSeries Logical Partition (PAPR compliant)
  pseries-3.0  pSeries Logical Partition (PAPR compliant)
  pseries-3.1  pSeries Logical Partition (PAPR compliant)
  pseries-4.0  pSeries Logical Partition (PAPR compliant)
  pseries-4.1  pSeries Logical Partition (PAPR compliant)
  pseries  pSeries Logical Partition (PAPR compliant) (alias of 
pseries-4.2)
  pseries-4.2  pSeries Logical Partition (PAPR compliant) (default)
  ref405ep ref405ep
  sam460ex aCube Sam460ex
  taihutaihu
  virtex-ml507 Xilinx Virtex ML507 reference design

  I am wondering if anyone knows that is if any of them can be selected
  for such emulation? Thank you!

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

Re: [PATCH] memory: Fix start offset for bitmap log_clear hook

2020-02-25 Thread Matt Borgerson

[ ping ]

Hi Paolo, would you mind taking a quick look at this patch for
memory.c to consider
it for merge? This resolves an issue with dirty bits not being cleared
as expected.

Here's the Patchwork link: http://patchwork.ozlabs.org/patch/1240121/

Thanks for your time!

Matt

On Tue, Feb 18, 2020 at 9:53 AM Peter Xu  wrote:
>
> On Tue, Feb 18, 2020 at 03:19:10AM -0700, Matt Borgerson wrote:
> > Currently only the final page offset is being passed to the `log_clear`
> > hook via `memory_region_clear_dirty_bitmap` after it is used as an
> > iterator in `cpu_physical_memory_test_and_clear_dirty`. This patch
> > corrects the start address and size of the region.
> >
> > Signed-off-by: Matt Borgerson 
>
> Looks correct, thanks!
>
> Reviewed-by: Peter Xu 
>
> --
> Peter Xu
>

[PATCH] Arithmetic error fixed in EDID generation

2020-02-25 Thread Anton V. Boyarshinov

To compute screen size in centimeters we should calculate:
pixels/dpi*2.54
but not
pixels*dpi/2540

Using wrong formula we actually get 65 DPI and very small fonts.

Signed-off-by: Anton V. Boyarshinov 
---
 hw/display/edid-generate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index 75c945a948..537e063662 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -360,8 +360,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
 edid[20] = 0xa5;
 
 /* screen size: undefined */
-edid[21] = info->prefx * info->dpi / 2540;
-edid[22] = info->prefy * info->dpi / 2540;
+edid[21] = (uint8_t) ((float) info->prefx / info->dpi * 2.54);
+edid[22] = (uint8_t) ((float) info->prefy / info->dpi * 2.54);
 
 /* display gamma: 2.2 */
 edid[23] = 220 - 100;
-- 
2.21.0

Sudden slowdown of ARM emulation in master

2020-02-25 Thread Niek Linnenbank

Hello Igor and Paolo,

Just now I was working on some small fixes for the cubieboard machine and
rebasing my Allwinner H3 branches.
While doing some testing, I noticed that suddenly the machines were much
slower than before.
I only see this happening when I rebase to this commit:
   ca6155c0f2bd39b4b4162533be401c98bd960820 ("Merge tag 'patchew/
20200219160953.13771-1-imamm...@redhat.com' of
https://github.com/patchew-project/qemu into HEAD")

Also the avocado tests I'm running started to timeout:

+ AVOCADO_ALLOW_LARGE_STORAGE=yes avocado --show=app,console run -t
machine:cubieboard tests/acceptance/boot_linux_console.py
...
(1/2)
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd:
|console: Uncompressing Linux... done, booting the kernel.
|console: Booting Linux on physical CPU 0x0
console: Linux version 4.20.7-sunxi (r...@armbian.com) (gcc version 7.2.1
20171011 (Linaro GCC 7.2-2017.11)) #5.75 SMP Fri Feb 8 09:02:10 CET 2019
console: CPU: ARMv7 Processor [410fc080] revision 0 (ARMv7), cr=50c5387d
console: CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing
instruction cache
console: OF: fdt: Machine model: Cubietech Cubieboard
...
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\n
Original status: ERROR\n{'name':
'1-tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd',
'logdir': '/home/me/avocado/job-results/job-2020-02-25T23.58-d43884...
(90.41 s)
...
console: random: crng init done
/console: mount: mounting devtmpfs on /dev failed: Device or resource busy
-console: EXT4-fs (sda): re-mounted. Opts:
block_validity,barrier,user_xattr,acl
/console: Starting logging: OK
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'2-tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_sata',
'logdir': '/home/fox/avocado/job-results/job-2020-02-25T23.58-d438849/...
(90.53 s)
RESULTS: PASS 0 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 2 |
CANCEL 0
JOB TIME   : 181.22 s
 

Have you noticed a similar performance change?
Do you have any clue if there may be something changed here that could
cause a slowdown?

Regards,
Niek


-- 
Niek Linnenbank

Re: [PATCH v6 09/18] target/ppc: Streamline calculation of RMA limit from LPCR[RMLS]

2020-02-25 Thread Greg Kurz

On Tue, 25 Feb 2020 18:05:31 +0100
Greg Kurz  wrote:

> On Tue, 25 Feb 2020 10:37:15 +1100
> David Gibson  wrote:
> 
> > Currently we use a big switch statement in ppc_hash64_update_rmls() to work
> > out what the right RMA limit is based on the LPCR[RMLS] field.  There's no
> > formula for this - it's just an arbitrary mapping defined by the existing
> > CPU implementations - but we can make it a bit more readable by using a
> > lookup table rather than a switch.  In addition we can use the MiB/GiB
> > symbols to make it a bit clearer.
> > 
> > While there we add a bit of clarity and rationale to the comment about
> > what happens if the LPCR[RMLS] doesn't contain a valid value.
> > 
> > Signed-off-by: David Gibson 
> > Reviewed-by: Cédric Le Goater 
> > ---
> >  target/ppc/mmu-hash64.c | 71 -
> >  1 file changed, 35 insertions(+), 36 deletions(-)
> > 
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index 0ef330a614..4f082d775d 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -18,6 +18,7 @@
> >   * License along with this library; if not, see 
> > .
> >   */
> >  #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> >  #include "cpu.h"
> >  #include "exec/exec-all.h"
> >  #include "exec/helper-proto.h"This tool was originally developed to fix 
> > Linux CPU throttling issues affecting Lenovo T480 / T480s / X1C6 as 
> > described here.
> > @@ -757,6 +758,39 @@ static void ppc_hash64_set_c(PowerPCCPU *cpu, hwaddr 
> > ptex, uint64_t pte1)
> >  stb_phys(CPU(cpu)->as, base + offset, (pte1 & 0xff) | 0x80);
> >  }
> >  
> > +static target_ulong rmls_limit(PowerPCCPU *cpu)
> > +{
> > +CPUPPCState *env = &cpu->env;
> > +/*
> > + * This is the full 4 bits encoding of POWER8. Previous
> > + * CPUs only support a subset of these but the filtering
> > + * is done when writing LPCR
> > + */
> > +const target_ulong rma_sizes[] = {
> > +[0] = 0,
> > +[1] = 16 * GiB,
> > +[2] = 1 * GiB,
> > +[3] = 64 * MiB,
> > +[4] = 256 * MiB,
> > +[5] = 0,
> > +[6] = 0,
> > +[7] = 128 * MiB,
> > +[8] = 32 * MiB,
> > +};
> > +target_ulong rmls = (env->spr[SPR_LPCR] & LPCR_RMLS) >> 
> > LPCR_RMLS_SHIFT;
> > +
> > +if (rmls < ARRAY_SIZE(rma_sizes)) {
> 
> This condition is always true since the RMLS field is 4-bit long... 

Oops my mistake, I was already thinking about the suggestion I have
for something that was puzzling me. See below.

> I guess you want to check that RMLS encodes a valid RMA size instead.
> 
> if (rma_sizes[rmls]) {
> 
> > +return rma_sizes[rmls];
> > +} else {
> > +/*
> > + * Bad value, so the OS has shot itself in the foot.  Return a
> > + * 0-sized RMA which we expect to trigger an immediate DSI or
> > + * ISI
> > + */

It seems a bit weird to differentiate the case where the value is bad
because it happens to be bigger than the highest supported one, compared
to values that are declared bad in rma_sizes[], like 0, 5 or 6. They're
all basically the same case of values not used to encode a valid size...

What about :

static const target_ulong rma_sizes[16] = {
[1] = 16 * GiB,
[2] = 1 * GiB,
[3] = 64 * MiB,
[4] = 256 * MiB,
[7] = 128 * MiB,
[8] = 32 * MiB,
};

?

> > +return 0;
> > +}
> > +}
> > +
> >  int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> >  int rwx, int mmu_idx)
> >  {
> > @@ -1006,41 +1040,6 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, 
> > target_ulong ptex,
> >  cpu->env.tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH;
> >  }
> >  
> > -static void ppc_hash64_update_rmls(PowerPCCPU *cpu)
> > -{
> > -CPUPPCState *env = &cpu->env;
> > -uint64_t lpcr = env->spr[SPR_LPCR];
> > -
> > -/*
> > - * This is the full 4 bits encoding of POWER8. Previous
> > - * CPUs only support a subset of these but the filtering
> > - * is done when writing LPCR
> > - */
> > -switch ((lpcr & LPCR_RMLS) >> LPCR_RMLS_SHIFT) {
> > -case 0x8: /* 32MB */
> > -env->rmls = 0x200ull;
> > -break;
> > -case 0x3: /* 64MB */
> > -env->rmls = 0x400ull;
> > -break;
> > -case 0x7: /* 128MB */
> > -env->rmls = 0x800ull;
> > -break;
> > -case 0x4: /* 256MB */
> > -env->rmls = 0x1000ull;
> > -break;
> > -case 0x2: /* 1GB */
> > -env->rmls = 0x4000ull;
> > -break;
> > -case 0x1: /* 16GB */
> > -env->rmls = 0x4ull;
> > -break;
> > -default:
> > -/* What to do here ??? */
> > -env->rmls = 0;
> > -}
> > -}
> > -
> >  static void ppc_hash64_update_vrma(PowerPCCPU *cpu)
> >  {
> >  CPUPPCState *env = &cpu->env;
> >

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

On Tue, 25 Feb 2020 at 20:10, Paolo Bonzini  wrote:
> The main issue with this series and Kashyap's is that if we don't manage to 
> get
> everything done in 5.0 we have a mutilated qemu-doc. Then either we keep it
> mutilated or we scramble to undo the work. So I would agree to commit the
> series in this order, but without the removal of the .texi files.

Kashyap's set is in the same ballpark as what we've currently
converted (notably it's pretty much equivalent to the qemu-block-drivers
conversion in that it takes what was part of qemu-doc plus a manpage
and turns it into part of the system manual plus a manpage).
It's also the most awkward to try to keep the texi around for, because
the makefile runes for the texi want to generate the manpage too.
So I think I would argue for taking that as-is, including removal of the
texi files.

I agree that it would be good to avoid a half-converted qemu-doc;
if people think keeping two parallel doc files until we're sure we
can do the conversion is useful insurance I'm happy to go along
with that.

If we ended up with "we managed all the conversion except for
the qapi json doc comments parts" would we be ok with having a
qemu-doc.html that just contained those, and all the actual docs
transitioning to rST for this release? Or would we want to roll
back the rST for the main qemu-doc parts too in that situation?

thanks
-- PMM

RE: Emulating Solaris 10 on SPARC64 sun4u

2020-02-25 Thread BALATON Zoltan


On Mon, 10 Feb 2020, BALATON Zoltan wrote:
This suggests the common IDE bmdma and ide-cd code is likely OK and problem 
is somewhere in irq routing. What's relevant for this thread and sparc64 is 
that then you should also check interrupt controller and routing if an 
interrupt raised by the IDE controller could get to the CPU in your case as 
that could be where the problem is and maybe not in common code as I've 
suspected before.


I can now confirm that my problem was related to IRQ routing as noted 
here:


https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg07225.html

so any similar problem for Solaris is not related to this and common IDE 
and BMDMA code are likely OK so you may want to check IRQ handling in 
board and chipset emulation in case the cause is similar to what I had.


Regards,
BALATON Zoltan

Re: IDE IRQ problem after UDMA enabled (was: Re: Emulating Solaris 10 on SPARC64 sun4u)

2020-02-25 Thread BALATON Zoltan

On Tue, 25 Feb 2020, BALATON Zoltan wrote:

On Mon, 10 Feb 2020, John Snow wrote:

It sounds like the real problem is either in the bmdma controller (or
its unique interaction with hw/ide/core.c -- which is possible) or in
the interrupt routing somewhere else.

If you have any IDE traces from a hang, feel free to throw them up on a
pastebin for me to take a peek at; it might help for me to see the exact
sequence that causes a hang in QEMU's IDE terms to see if I can't
"reverse engineer" what the guest is hoping to have happen. Maybe I can
trace this to a bad register value.

I've got some traces from Linux and MorphOS (both on my work in progress
pegasos2 emulation using via-ide where I can most easily reproduce this) but
I'm not sure what to look for in these. MorphOS starts booting, so firmware
can read ide-cd connected to via-ide as well as MorphOS can before enabling
UDMA 5 mode but stops after that and cannot read the drive any more. Linux
works even after enabling DMA. I've gathered some logs in
https://osdn.net/projects/qmiga/ticket/38949 previously but now I try to list
here the part in more detail where drive is detected, enabling DMA and first
command after that in case you can spot something in these that could explain
why it fails with MorphOS driver.

Never mind, I've found a clue in NetBSD's driver:

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/viaide.c?rev=1.89&content-type=text/x-cvsweb-markup&only_with_tag=MAIN

which has a comment that says:

/*
* At least under certain (mis)configurations (e.g. on the "Pegasos" board)
* the VT8231-IDE's native mode only works with irq 14/15, and cannot be
* programmed to use a single native PCI irq alone. So we install an interrupt
* handler for each channel, as in compatibility mode.
*/

If I change via-ide to use ISA IRQ14 and 15 and ignore what's programmed
in the PCI config reg then MorphOS works with UDMA so it expects that.
This change however breaks Linux which still boots after getting some
errors but maybe it downgrades to PIO mode then. I'll need to find out
more about how is this broken on real hardware and how can we emulate it.

So you don't need to look at the logs unless you want to check why it sees
a non working ATA device after resetting the bus but logs in the ticket
above may be more useful for that as I did not include that part in this
email.

Thank you,
BALATON Zoltan

Re: [PULL 00/32] virtio, pc: fixes, features

2020-02-25 Thread Michael S. Tsirkin

On Tue, Feb 25, 2020 at 04:47:31PM +, Peter Maydell wrote:
> On Tue, 25 Feb 2020 at 15:12, Michael S. Tsirkin  wrote:
> >
> > The following changes since commit 9a8abceb5f01d1066d3a1ac5a33aabcbaeec1860:
> >
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-docs-20200225' 
> > into staging (2020-02-25 11:03:47 +)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to afc37debf27ecf34d6bc1d4b52fa0918d0bd3f3c:
> >
> >   Fixed assert in vhost_user_set_mem_table_postcopy (2020-02-25 08:47:47 
> > -0500)
> >
> > 
> > virtio, pc: fixes, features
> >
> > New virtio iommu.
> > Unrealize memory leaks.
> > In-band kick/call support.
> > Bugfixes, documentation all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin 
> 
> Hi; this fails to build on OSX:
> 
>   CC  contrib/libvhost-user/libvhost-user.o
> /Users/pm215/src/qemu-for-merges/contrib/libvhost-user/libvhost-user.c:27:10:
> fatal error: 'sys/eventfd.h' file not found
> #include 
>  ^~~
> In file included from
> /Users/pm215/src/qemu-for-merges/contrib/vhost-user-bridge/main.c:37:
> /Users/pm215/src/qemu-for-merges/contrib/libvhost-user/libvhost-user.h:21:10:
> fatal error: 'linux/vhost.h' file not found
> #include 
>  ^~~
> 1 error generated.
> 
> thanks
> -- PMM

OK this must be the move to contrib. Peter could you please try
again now? New tag at c3744b57bb37cd1120acb621ce4683b1c8e8a1c6.

Re: [RFC qemu 0/6] mirror: implement incremental and bitmap modes




On 2/18/20 5:07 AM, Fabian Grünbichler wrote:
> picking up on John's in-progress patch series from last summer, this is
> a stab at rebasing and adding test cases for the low-hanging fruits:
> 
> - bitmap mirror mode with always/on-success/never bitmap sync mode
> - incremental mirror mode as sugar for bitmap + on-success
> 
> Fabian Grünbichler (4):
>   mirror: add check for bitmap-mode without bitmap
>   mirror: switch to bdrv_dirty_bitmap_merge_internal
>   iotests: add test for bitmap mirror
>   mirror: move some checks to QMP
> 
> John Snow (2):
>   drive-mirror: add support for sync=bitmap mode=never
>   drive-mirror: add support for conditional and always bitmap sync modes
> 
>  include/block/block_int.h   |4 +-
>  block/mirror.c  |   96 +-
>  blockdev.c  |   71 +-
>  tests/test-block-iothread.c |4 +-
>  qapi/block-core.json|   29 +-
>  tests/qemu-iotests/284  |  547 +++
>  tests/qemu-iotests/284.out  | 2846 +++
>  tests/qemu-iotests/group|1 +
>  8 files changed, 3567 insertions(+), 31 deletions(-)
>  create mode 100755 tests/qemu-iotests/284
>  create mode 100644 tests/qemu-iotests/284.out
> 

Hi Fabian! Thanks for picking this up. I'm a bit behind on my mail, but
this on my list to look at.

(Hint to other maintainers: It might be a while.)

--js

Re: [PATCH v5 0/5] iotests: use python logging

On 2/24/20 6:15 AM, Max Reitz wrote:
> On 12.10.19 01:39, John Snow wrote:
>> Just caught up with the discussion.
>>
>> It looks like Thomas took my 1/5; so I'll respin on top of his "[PATCH
>> 0/5] Enable more iotests during "make check-block" series to catch those
>> improvements as they stand.
> 
> Any updates on this? :)
> 
> Max
> 

Nope.

Well, except that I was working on job_run today and remembered that I
needed to do this. I was waiting for that discussion to die down, and
then forgetting took over.

Will attempt to resuscitate.

Re: [PATCH 01/13] block/stream: Remove redundant statement in stream_run()




On 2/24/20 9:09 PM, kuhn.chen...@huawei.com wrote:
> From: Chen Qun 
> 
> Clang static code analyzer show warning:
>   block/stream.c:186:9: warning: Value stored to 'ret' is never read
> ret = 0;
> ^ ~
> Reported-by: Euler Robot 
> Signed-off-by: Chen Qun 
> ---
> Cc: John Snow 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: qemu-bl...@nongnu.org
> ---
>  block/stream.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/block/stream.c b/block/stream.c
> index 5562ccbf57..d78074ac80 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -183,7 +183,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
>  break;
>  }
>  }
> -ret = 0;
>  
>  /* Publish progress */
>  job_progress_update(&s->common.job, n);
> 

Reviewed-by: John Snow

Re: [edk2-devel] A problem with live migration of UEFI virtual machines

2020-02-25 Thread Andrew Fish via




> On Feb 25, 2020, at 12:40 PM, Laszlo Ersek  wrote:
> 
> Hi Andrew,
> 
> On 02/25/20 19:56, Andrew Fish wrote:
>> Laszlo,
>> 
>> If I understand this correctly is it not more complicated than just size. It 
>> also assumes the memory layout is the same?
> 
> Yes.
> 
>> The legacy BIOS used fixed magic address ranges, but UEFI uses dynamically 
>> allocated memory so addresses are not fixed. While the UEFI firmware does 
>> try to keep S3 and S4 layouts consistent between boots, I'm not aware of any 
>> mechanism to keep the memory map address the same between versions of the 
>> firmware?
> 
> It's not about RAM, but platform MMIO.
> 

Laszlo,

The FLASH offsets changing breaking things makes sense. 

I now realize this is like updating the EFI ROM without rebooting the system.  
Thus changes in how the new EFI code works is not the issue. 

Is this migration event visible to the firmware? Traditionally the NVRAM is a 
region in the FD so if you update the FD you have to skip NVRAM region or save 
and restore it. Is that activity happening in this case? Even if the ROM layout 
does not change how do you not lose the contents of the NVRAM store when the 
live migration happens? Sorry if this is a remedial question but I'm trying to 
learn how this migration works. 

Thanks,

Andrew Fish

> The core of the issue here is that the -D FD_SIZE_4MB and -D FD_SIZE_2MB
> build options (or more directly, the different FD_SIZE_IN_KB macro
> settings) set a bunch of flash-related build-time constant macros, and
> PCDs, differently, in the following files:
> 
> - OvmfPkg/OvmfPkg.fdf.inc
> - OvmfPkg/VarStore.fdf.inc
> - OvmfPkg/OvmfPkg*.dsc
> 
> As a result, the OVMF_CODE.fd firmware binary will have different
> hard-coded references to the variable store pflash addresses.
> (Guest-physical MMIO addresses that point into the pflash range.)
> 
> If someone tries to combine an OVMF_CODE.fd firmware binary from e.g.
> the 4MB build, with a variable store file that was originally
> instantiated from an OVMF_VARS.fd varstore template from the 2MB build,
> then the firmware binary's physical address references and various size
> references will not match the contents / layout of the varstore pflash
> chip, which maps an incompatibly structured varstore file.
> 
> For example, "OvmfPkg/VarStore.fdf.inc" describes two incompatible
> EFI_FIRMWARE_VOLUME_HEADER structures (which "build" generates for the
> OVMF_VARS.fd template) between the 4MB (total size) build, and the
> 1MB/2MB (total size) build.
> 
> The commit message below summarizes the internal layout differences,
> from 1MB/2MB -> 4MB:
> 
> https://github.com/tianocore/edk2/commit/b24fca05751f
> 
> Excerpt (relevant for OVMF_VARS.fd):
> 
>  DescriptionCompression typeSize [KB]
>  -  -  --
>  Non-volatile data storage  open-coded binary128 ->   528 ( +400)
>   data
>Variable store 56 ->   256 ( +200)
>Event log   4 -> 4 (   +0)
>Working block   4 -> 4 (   +0)
>Spare area 64 ->   264 ( +200)
> 
> Thanks
> Laszlo
> 
> 
>>> On Feb 25, 2020, at 9:53 AM, Laszlo Ersek  wrote:
>>> 
>>> On 02/24/20 16:28, Daniel P. Berrangé wrote:
 On Tue, Feb 11, 2020 at 05:39:59PM +, Alex Bennée wrote:
> 
> wuchenye1995  writes:
> 
>> Hi all,
>>  We found a problem with live migration of UEFI virtual machines
>>  due to size of OVMF.fd changes.
>>  Specifically, the size of OVMF.fd in edk with low version such as
>>  edk-2.0-25 is 2MB while the size of it in higher version such as
>>  edk-2.0-30 is 4MB.
>>  When we migrate a UEFI virtual machine from the host with low
>>  version of edk2 to the host with higher one, qemu component will
>>  report an error in function qemu_ram_resize while
>> checking size of ovmf_pcbios: Length mismatch: pc.bios: 0x20 in
>> != 0x40: Invalid argument.
>>  We want to know how to solve this problem after updating the
>>  version of edk2.
> 
> You can only migrate a machine that is identical - so instantiating a
> empty machine with a different EDK image is bound to cause a problem
> because the machines don't match.
 
 I don't believe we are that strict for firmware in general. The
 firmware is loaded when QEMU starts, but that only matters for the
 original source host QEMU. During migration, the memory content of the
 original firmware will be copied during live migration, overwriting
 whatever the target QEMU loaded off disk. This worksprovided the
 memory region is the same size on source & target host, which is where
 the problem arises in this case.
 
 If there's a risk that newer firmware will be larger than old firmware
 there's o

Getting Program Counter

2020-02-25 Thread nikhil bansal

Hi,

I need the memory access traces of an android system running on qemu.
trace_memory_region_ops_read/write in memory.c provide most of the
information I need. However, in addition to the trace information already
provided, I also need the program counter of the current instructions. What
changes should I make to memory.c to output pc along with the information
already printed?

Thanking you in advance.

IDE IRQ problem after UDMA enabled (was: Re: Emulating Solaris 10 on SPARC64 sun4u)

2020-02-25 Thread BALATON Zoltan


Hello,

On Mon, 10 Feb 2020, John Snow wrote:

It sounds like the real problem is either in the bmdma controller (or
its unique interaction with hw/ide/core.c -- which is possible) or in
the interrupt routing somewhere else.

If you have any IDE traces from a hang, feel free to throw them up on a
pastebin for me to take a peek at; it might help for me to see the exact
sequence that causes a hang in QEMU's IDE terms to see if I can't
"reverse engineer" what the guest is hoping to have happen. Maybe I can
trace this to a bad register value.


I've got some traces from Linux and MorphOS (both on my work in progress 
pegasos2 emulation using via-ide where I can most easily reproduce this) 
but I'm not sure what to look for in these. MorphOS starts booting, so 
firmware can read ide-cd connected to via-ide as well as MorphOS can 
before enabling UDMA 5 mode but stops after that and cannot read the drive 
any more. Linux works even after enabling DMA. I've gathered some logs in 
https://osdn.net/projects/qmiga/ticket/38949 previously but now I try to 
list here the part in more detail where drive is detected, enabling DMA 
and first command after that in case you can spot something in these that 
could explain why it fails with MorphOS driver.


First the working Linux case:

pci_cfg_read via-ide 12:1 @0x4 -> 0x87
pci_cfg_read via-ide 12:1 @0x3d -> 0x1
pci_cfg_read via-ide 12:1 @0x4 -> 0x87
pci_cfg_read via-ide 12:1 @0x40 -> 0xb
pci_cfg_read via-ide 12:1 @0x40 -> 0xb
bmdma_read_via bmdma: readb 0x2 : 0x00
bmdma_read_via bmdma: readb 0x2 : 0x00
pci_cfg_read via-ide 12:1 @0x4 -> 0x87
ide_cmd_write IDE PIO wr @ 0x4 (Device Control); val 0x0a; bus 0x56229cb35600
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x00; bus 0x56229cb35600 
IDEState 0x56229cb35a58
bmdma_read_via bmdma: readb 0x2 : 0x00
bmdma_write_via bmdma: writeb 0x2 : 0x00
ide_cmd_write IDE PIO wr @ 0x4 (Device Control); val 0x0a; bus 0x56229cb35ef0
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
bmdma_read_via bmdma: readb 0x2 : 0x00
bmdma_write_via bmdma: writeb 0x2 : 0x00
pci_cfg_read via-ide 12:1 @0x9 -> 0x8f
[2.589547] scsi0 : pata_via
[2.590949] scsi1 : pata_via
[2.591488] ata1: PATA max UDMA/100 cmd 0x1000 ctl 0x100c bmdma 0x1020 irq 9
[2.591652] ata2: PATA max UDMA/100 cmd 0x1010 ctl 0x101c bmdma 0x1028 irq 9

[...]

[2.938174] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
pci_cfg_read via-ide 12:1 @0x4c -> 0xaa
pci_cfg_write via-ide 12:1 @0x4c <- 0xa2
pci_cfg_write via-ide 12:1 @0x4e <- 0x31
pci_cfg_write via-ide 12:1 @0x49 <- 0x31
pci_cfg_read via-ide 12:1 @0x51 -> 0x17
pci_cfg_write via-ide 12:1 @0x51 <- 0x17
pci_cfg_read via-ide 12:1 @0x4c -> 0xa2
pci_cfg_write via-ide 12:1 @0x4c <- 0xa2
pci_cfg_write via-ide 12:1 @0x4e <- 0x31
pci_cfg_write via-ide 12:1 @0x49 <- 0x31
pci_cfg_read via-ide 12:1 @0x51 -> 0x17
pci_cfg_write via-ide 12:1 @0x51 <- 0xf0
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x6 (Device/Head); val 0xa0; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_status_read IDE PIO rd @ 0x4 (Alt Status); val 0x50; bus 0x56229cb35ef0; 
IDEState 0x56229cb35f78
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_cmd_write IDE PIO wr @ 0x4 (Device Control); val 0x0a; bus 0x56229cb35ef0
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x6 (Device/Head); val 0xa0; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x1 (Features); val 0x03; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x2 (Sector Count); val 0x45; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x3 (Sector Number); val 0x00; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x4 (Cylinder Low); val 0x00; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x5 (Cylinder High); val 0x00; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_write IDE PIO wr @ 0x7 (Command); val 0xef; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_exec_cmd IDE exec cmd: bus 0x56229cb35ef0; state 0x56229cb35f78; cmd 0xef
ide_status_read IDE PIO rd @ 0x4 (Alt Status); val 0x50; bus 0x56229cb35ef0; 
IDEState 0x56229cb35f78
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_cmd_write IDE PIO wr @ 0x4 (Device Control); val 0x08; bus 0x56229cb35ef0
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
bmdma_read_via bmdma: readb 0x2 : 0x00
bmdma_write_via bmdma: writeb 0x2 : 0x00
ide_ioport_read IDE PIO rd @ 0x7 (Status); val 0x50; bus 0x56229cb35ef0 
IDEState 0x56229cb35f78
ide_ioport_read IDE PIO rd @ 0x1 (Error);

Re: [PATCH 1/6] block: add bitmap-populate job




On 2/25/20 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
> 25.02.2020 3:56, John Snow wrote:
>> This job copies the allocation map into a bitmap. It's a job because
>> there's no guarantee that allocation interrogation will be quick (or
>> won't hang), so it cannot be retrofit into block-dirty-bitmap-merge.
>>
>> It was designed with different possible population patterns in mind,
>> but only top layer allocation was implemented for now.
>>
>> Signed-off-by: John Snow 
>> ---
>>   qapi/block-core.json  |  48 +
>>   qapi/job.json |   2 +-
>>   include/block/block_int.h |  21 
>>   block/bitmap-alloc.c  | 207 ++
>>   blockjob.c    |   3 +-
>>   block/Makefile.objs   |   1 +
>>   6 files changed, 280 insertions(+), 2 deletions(-)
>>   create mode 100644 block/bitmap-alloc.c
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 85e27bb61f..df1797681a 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2245,6 +2245,54 @@
>>     { 'command': 'block-dirty-bitmap-merge',
>>   'data': 'BlockDirtyBitmapMerge' }
>>   +##
>> +# @BitmapPattern:
>> +#
>> +# An enumeration of possible patterns that can be written into a bitmap.
>> +#
>> +# @allocation-top: The allocation status of the top layer
>> +#  of the attached storage node.
>> +#
>> +# Since: 5.0
>> +##
>> +{ 'enum': 'BitmapPattern',
>> +  'data': ['allocation-top'] }
>> +
>> +##
>> +# @BlockDirtyBitmapPopulate:
>> +#
>> +# @job-id: identifier for the newly-created block job.
>> +#
>> +# @pattern: What pattern should be written into the bitmap?
>> +#
>> +# @on-error: the action to take if an error is encountered on a bitmap's
>> +#    attached node, default 'report'.
>> +#    'stop' and 'enospc' can only be used if the block device
>> supports
>> +#    io-status (see BlockInfo).
>> +#
>> +# @auto-finalize: When false, this job will wait in a PENDING state
>> after it has
>> +# finished its work, waiting for @block-job-finalize
>> before
>> +# making any block graph changes.
> 
> sounds a bit strange in context of bitmap-population job
> 

Yeah, you're right. Copy-pasted for "consistency".

>> +# When true, this job will automatically
>> +# perform its abort or commit actions.
>> +# Defaults to true.
>> +#
>> +# @auto-dismiss: When false, this job will wait in a CONCLUDED state
>> after it
>> +#    has completely ceased all work, and awaits
>> @block-job-dismiss.
>> +#    When true, this job will automatically disappear
>> from the query
>> +#    list without user intervention.
>> +#    Defaults to true.
>> +#
>> +# Since: 5.0
>> +##
>> +{ 'struct': 'BlockDirtyBitmapPopulate',
>> +  'base': 'BlockDirtyBitmap',
>> +  'data': { 'job-id': 'str',
>> +    'pattern': 'BitmapPattern',
>> +    '*on-error': 'BlockdevOnError',
>> +    '*auto-finalize': 'bool',
>> +    '*auto-dismiss': 'bool' } }
>> +
>>   ##
>>   # @BlockDirtyBitmapSha256:
>>   #
>> diff --git a/qapi/job.json b/qapi/job.json
>> index 5e658281f5..5f496d4630 100644
>> --- a/qapi/job.json
>> +++ b/qapi/job.json
>> @@ -22,7 +22,7 @@
>>   # Since: 1.7
>>   ##
>>   { 'enum': 'JobType',
>> -  'data': ['commit', 'stream', 'mirror', 'backup', 'create'] }
>> +  'data': ['commit', 'stream', 'mirror', 'backup', 'create',
>> 'bitmap-populate'] }
>>     ##
>>   # @JobStatus:
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 6f9fd5e20e..a5884b597e 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -1215,6 +1215,27 @@ BlockJob *backup_job_create(const char *job_id,
>> BlockDriverState *bs,
>>   BlockCompletionFunc *cb, void *opaque,
>>   JobTxn *txn, Error **errp);
>>   +/*
>> + * bitpop_job_create: Create a new bitmap population job.
>> + *
>> + * @job_id: The id of the newly-created job.
>> + * @bs: Block device associated with the @target_bitmap.
>> + * @target_bitmap: The bitmap to populate.
>> + * @on_error: What to do if an error on @bs is encountered.
>> + * @creation_flags: Flags that control the behavior of the Job lifetime.
>> + *  See @BlockJobCreateFlags
>> + * @cb: Completion function for the job.
>> + * @opaque: Opaque pointer value passed to @cb.
>> + * @txn: Transaction that this job is part of (may be NULL).
>> + */
>> +BlockJob *bitpop_job_create(const char *job_id, BlockDriverState *bs,
>> +    BdrvDirtyBitmap *target_bitmap,
>> +    BitmapPattern pattern,
>> +    BlockdevOnError on_error,
>> +    int creation_flags,
>> +    BlockCompletionFunc *cb, void *opaque,
>> +    JobTxn *txn, Error **errp);
>>

Re: [PATCH v3 0/5] linux-user: Implement x86_64 vsyscalls

2020-02-25 Thread Laurent Vivier

Le 25/02/2020 à 20:59, Richard Henderson a écrit :
> On 2/12/20 7:22 PM, Richard Henderson wrote:
>> Changes for v3:
>>
>>   * Add TARGET_VSYSCALL_PAGE define.
>>   * Move the sigsegv goto around.
>>
>> v2: https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg03474.html
>> v1: https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg02841.html
> 
> Ping?

Applied to my linux-user branch.
I've removed the trailing whitespace reported by checkpatch.pl

Thanks,
Laurent

Re: [edk2-devel] A problem with live migration of UEFI virtual machines

2020-02-25 Thread Laszlo Ersek

Hi Andrew,

On 02/25/20 19:56, Andrew Fish wrote:
> Laszlo,
> 
> If I understand this correctly is it not more complicated than just size. It 
> also assumes the memory layout is the same?

Yes.

> The legacy BIOS used fixed magic address ranges, but UEFI uses dynamically 
> allocated memory so addresses are not fixed. While the UEFI firmware does try 
> to keep S3 and S4 layouts consistent between boots, I'm not aware of any 
> mechanism to keep the memory map address the same between versions of the 
> firmware?

It's not about RAM, but platform MMIO.

The core of the issue here is that the -D FD_SIZE_4MB and -D FD_SIZE_2MB
build options (or more directly, the different FD_SIZE_IN_KB macro
settings) set a bunch of flash-related build-time constant macros, and
PCDs, differently, in the following files:

- OvmfPkg/OvmfPkg.fdf.inc
- OvmfPkg/VarStore.fdf.inc
- OvmfPkg/OvmfPkg*.dsc

As a result, the OVMF_CODE.fd firmware binary will have different
hard-coded references to the variable store pflash addresses.
(Guest-physical MMIO addresses that point into the pflash range.)

If someone tries to combine an OVMF_CODE.fd firmware binary from e.g.
the 4MB build, with a variable store file that was originally
instantiated from an OVMF_VARS.fd varstore template from the 2MB build,
then the firmware binary's physical address references and various size
references will not match the contents / layout of the varstore pflash
chip, which maps an incompatibly structured varstore file.

For example, "OvmfPkg/VarStore.fdf.inc" describes two incompatible
EFI_FIRMWARE_VOLUME_HEADER structures (which "build" generates for the
OVMF_VARS.fd template) between the 4MB (total size) build, and the
1MB/2MB (total size) build.

The commit message below summarizes the internal layout differences,
from 1MB/2MB -> 4MB:

https://github.com/tianocore/edk2/commit/b24fca05751f

Excerpt (relevant for OVMF_VARS.fd):

  DescriptionCompression typeSize [KB]
  -  -  --
  Non-volatile data storage  open-coded binary128 ->   528 ( +400)
   data
Variable store 56 ->   256 ( +200)
Event log   4 -> 4 (   +0)
Working block   4 -> 4 (   +0)
Spare area 64 ->   264 ( +200)

Thanks
Laszlo


>> On Feb 25, 2020, at 9:53 AM, Laszlo Ersek  wrote:
>>
>> On 02/24/20 16:28, Daniel P. Berrangé wrote:
>>> On Tue, Feb 11, 2020 at 05:39:59PM +, Alex Bennée wrote:

 wuchenye1995  writes:

> Hi all,
>   We found a problem with live migration of UEFI virtual machines
>   due to size of OVMF.fd changes.
>   Specifically, the size of OVMF.fd in edk with low version such as
>   edk-2.0-25 is 2MB while the size of it in higher version such as
>   edk-2.0-30 is 4MB.
>   When we migrate a UEFI virtual machine from the host with low
>   version of edk2 to the host with higher one, qemu component will
>   report an error in function qemu_ram_resize while
> checking size of ovmf_pcbios: Length mismatch: pc.bios: 0x20 in
> != 0x40: Invalid argument.
>   We want to know how to solve this problem after updating the
>   version of edk2.

 You can only migrate a machine that is identical - so instantiating a
 empty machine with a different EDK image is bound to cause a problem
 because the machines don't match.
>>>
>>> I don't believe we are that strict for firmware in general. The
>>> firmware is loaded when QEMU starts, but that only matters for the
>>> original source host QEMU. During migration, the memory content of the
>>> original firmware will be copied during live migration, overwriting
>>> whatever the target QEMU loaded off disk. This worksprovided the
>>> memory region is the same size on source & target host, which is where
>>> the problem arises in this case.
>>>
>>> If there's a risk that newer firmware will be larger than old firmware
>>> there's only really two options:
>>>
>>>  - Keep all firmware images forever, each with a unique versioned
>>>filename. This ensures target QEMU will always load the original
>>>smaller firmware
>>>
>>>  - Add padding to the firmware images. IOW, if the firmware is 2 MB,
>>>add zero-padding to the end of the image to round it upto 4 MB
>>>(whatever you anticipate the largest size wil be in future).
>>>
>>> Distros have often taken the latter approach for QEMU firmware in the
>>> past. The main issue is that you have to plan ahead of time and get
>>> this padding right from the very start. You can't add the padding
>>> after the fact on an existing VM.
>>
>> Following up here *too*, just for completeness.
>>
>> The query in this thread has been posted three times now (and I have
>> zero idea why). Each time it generated a different set of respons

Re: [PATCH RISU] aarch64.risu: Add patterns for v8.3-RCPC and v8.4-RCPC insns



Peter Maydell  writes:

> Add patterns for the new instructions in the v8.3-RCPC and
> v8.4-RCPC extensions.
>
> Signed-off-by: Peter Maydell 
> ---
> This is what I used for testing the RCPC QEMU patches I sent out
> the other day. Did I get the @ section syntax here right?

Yep ;-)

Reviewed-by: Alex Bennée 

>
>
>  aarch64.risu | 32 
>  1 file changed, 32 insertions(+)
>
> diff --git a/aarch64.risu b/aarch64.risu
> index c4eda7a..8f08cd0 100644
> --- a/aarch64.risu
> +++ b/aarch64.risu
> @@ -3019,3 +3019,35 @@ SM3TT2B A64_V 1100 1110 010 rm:5 10 imm:2 11 rn:5 rd:5
>  XAR A64_V 1100 1110 100 rm:5 imm:6 rn:5 rd:5
>  
>  @
> +
> +# v8.3-RCPC instructions
> +@v8_3_rcpc
> +
> +# LDAPR, LDAPRH, LDAPRB
> +# As usual, the $rn != $rt constraint is risu-imposed, not architectural
> +LDAPR A64 sz:2 111000 101 1 1100 00 rn:5 rt:5 \
> +!constraints { $rn != 31 && $rn != $rt } \
> +!memory { align(1 << $sz); reg_plus_imm($rn, 0); }
> +
> +@
> +
> +# v8.4-RCPC instructions
> +# As usual, the $rn != $rt constraint is risu-imposed, not architectural
> +@v8_4_rcpc
> +STLUR A64 sz:2 011001 00 0 imm:9 00 rn:5 rt:5 \
> +!constraints { $rn != 31 && $rn != $rt } \
> +!memory { align(1 << $sz); reg_plus_imm($rn, $imm); }
> +
> +LDAPUR A64 sz:2 011001 01 0 imm:9 00 rn:5 rt:5 \
> +!constraints { $rn != 31 && $rn != $rt } \
> +!memory { align(1 << $sz); reg_plus_imm($rn, $imm); }
> +
> +LDAPURS64 A64 sz:2 011001 10 0 imm:9 00 rn:5 rt:5 \
> +!constraints { $rn != 31 && $rn != $rt && $sz != 3 } \
> +!memory { align(1 << $sz); reg_plus_imm($rn, $imm); }
> +
> +LDAPURS32 A64 sz:2 011001 11 0 imm:9 00 rn:5 rt:5 \
> +!constraints { $rn != 31 && $rn != $rt && $sz < 2 } \
> +!memory { align(1 << $sz); reg_plus_imm($rn, $imm); }
> +
> +@


-- 
Alex Bennée

[Bug 1864704] [NEW] No compatible -machine option in qemu-system-ppc64 for e6500 core

2020-02-25 Thread Xiaoxing Fang

Public bug reported:

Hi,

I'm trying to use qemu-system-ppc64 for emulating a QorIQ T2080 (with e6500 
cores). 
However, I couldn't find any -machine option that matches -cpu e6500 option, 
which are listed below:

C:\Program Files\qemu>qemu-system-ppc64 -machine help
Supported machines are:
40p  IBM RS/6000 7020 (40p)
bamboo   bamboo
g3beige  Heathrow based PowerMAC
mac99Mac99 based PowerMAC
mpc8544dsmpc8544ds
none empty machine
powernv8 IBM PowerNV (Non-Virtualized) POWER8
powernv  IBM PowerNV (Non-Virtualized) POWER9 (alias of powernv9)
powernv9 IBM PowerNV (Non-Virtualized) POWER9
ppce500  generic paravirt e500 platform
prep PowerPC PREP platform (deprecated)
pseries-2.1  pSeries Logical Partition (PAPR compliant)
pseries-2.10 pSeries Logical Partition (PAPR compliant)
pseries-2.11 pSeries Logical Partition (PAPR compliant)
pseries-2.12 pSeries Logical Partition (PAPR compliant)
pseries-2.12-sxxmpSeries Logical Partition (PAPR compliant)
pseries-2.2  pSeries Logical Partition (PAPR compliant)
pseries-2.3  pSeries Logical Partition (PAPR compliant)
pseries-2.4  pSeries Logical Partition (PAPR compliant)
pseries-2.5  pSeries Logical Partition (PAPR compliant)
pseries-2.6  pSeries Logical Partition (PAPR compliant)
pseries-2.7  pSeries Logical Partition (PAPR compliant)
pseries-2.8  pSeries Logical Partition (PAPR compliant)
pseries-2.9  pSeries Logical Partition (PAPR compliant)
pseries-3.0  pSeries Logical Partition (PAPR compliant)
pseries-3.1  pSeries Logical Partition (PAPR compliant)
pseries-4.0  pSeries Logical Partition (PAPR compliant)
pseries-4.1  pSeries Logical Partition (PAPR compliant)
pseries  pSeries Logical Partition (PAPR compliant) (alias of 
pseries-4.2)
pseries-4.2  pSeries Logical Partition (PAPR compliant) (default)
ref405ep ref405ep
sam460ex aCube Sam460ex
taihutaihu
virtex-ml507 Xilinx Virtex ML507 reference design

I am wondering if anyone knows that is if any of them can be selected
for such emulation? Thank you!

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: ppc

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

Title:
  No compatible -machine option in qemu-system-ppc64 for e6500 core

Status in QEMU:
  New

Bug description:
  Hi,

  I'm trying to use qemu-system-ppc64 for emulating a QorIQ T2080 (with e6500 
cores). 
  However, I couldn't find any -machine option that matches -cpu e6500 option, 
which are listed below:

  C:\Program Files\qemu>qemu-system-ppc64 -machine help
  Supported machines are:
  40p  IBM RS/6000 7020 (40p)
  bamboo   bamboo
  g3beige  Heathrow based PowerMAC
  mac99Mac99 based PowerMAC
  mpc8544dsmpc8544ds
  none empty machine
  powernv8 IBM PowerNV (Non-Virtualized) POWER8
  powernv  IBM PowerNV (Non-Virtualized) POWER9 (alias of powernv9)
  powernv9 IBM PowerNV (Non-Virtualized) POWER9
  ppce500  generic paravirt e500 platform
  prep PowerPC PREP platform (deprecated)
  pseries-2.1  pSeries Logical Partition (PAPR compliant)
  pseries-2.10 pSeries Logical Partition (PAPR compliant)
  pseries-2.11 pSeries Logical Partition (PAPR compliant)
  pseries-2.12 pSeries Logical Partition (PAPR compliant)
  pseries-2.12-sxxmpSeries Logical Partition (PAPR compliant)
  pseries-2.2  pSeries Logical Partition (PAPR compliant)
  pseries-2.3  pSeries Logical Partition (PAPR compliant)
  pseries-2.4  pSeries Logical Partition (PAPR compliant)
  pseries-2.5  pSeries Logical Partition (PAPR compliant)
  pseries-2.6  pSeries Logical Partition (PAPR compliant)
  pseries-2.7  pSeries Logical Partition (PAPR compliant)
  pseries-2.8  pSeries Logical Partition (PAPR compliant)
  pseries-2.9  pSeries Logical Partition (PAPR compliant)
  pseries-3.0  pSeries Logical Partition (PAPR compliant)
  pseries-3.1  pSeries Logical Partition (PAPR compliant)
  pseries-4.0  pSeries Logical Partition (PAPR compliant)
  pseries-4.1  pSeries Logical Partition (PAPR compliant)
  pseries  pSeries Logical Partition (PAPR compliant) (alias of 
pseries-4.2)
  pseries-4.2  pSeries Logical Partition (PAPR compliant) (default)
  ref405ep ref405ep
  sam460ex aCube Sam460ex
  taihutaihu
  virtex-ml507 Xilinx Virtex ML507 reference design

  I am wondering if anyone knows that is if a

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

Il mar 25 feb 2020, 20:50 Peter Maydell  ha
scritto:

> On Tue, 25 Feb 2020 at 19:10, Paolo Bonzini  wrote:
> I feel like we're working a bit at cross purposes here so maybe
> we'd benefit from just nailing down who's going to do what and
> in which order?
>

I am not going to do much more than what I posted today, basically only the
automated conversion.

>
> My current thought on ordering is something like:
>  * commit this
>  * commit Kashyap's series
>  * commit (an adjusted version of) your split-out-the-texi series
>  * (automated) conversion of more texi -- all in one series I guess ?
>  * ???
>  * profit
>
> but I'm not very strongly attached to that.
>

The main issue with this series and Kashyap's is that if we don't manage to
get everything done in 5.0 we have a mutilated qemu-doc. Then either we
keep it mutilated or we scramble to undo the work. So I would agree to
commit the series in this order, but without the removal of the .texi files.

> Perhaps we could have the files in both .texi and (automatically
> > converted) .rst versions at the same time in the tree for a short
> > period. If that's okay for you, I can post tomorrow a series to do that.
>
> My instinct is to say that that's a bit dangerous as it means
> we might end up with changes to the "wrong" version of the
> two files. Would it let us do the conversion faster or
> more conveniently ?
>

It would be a kind of "insurance" against being late, basically. Doc
changes are rare enough that we could manage it, I think (and as long as
code review catches changes to only one version of the docs, no bitrot is
possible since we would build both).

Paolo

> thanks
> -- PMM
>
>

Re: [PATCH v3 0/5] linux-user: Implement x86_64 vsyscalls

On 2/12/20 7:22 PM, Richard Henderson wrote:
> Changes for v3:
> 
>   * Add TARGET_VSYSCALL_PAGE define.
>   * Move the sigsegv goto around.
> 
> v2: https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg03474.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg02841.html

Ping?


r~

Re: [PATCH] target/arm: Implement (trivially) ARMv8.2-TTCNP

On 2/25/20 11:38 AM, Peter Maydell wrote:
> @@ -705,6 +706,7 @@ static void aarch64_max_initfn(Object *obj)
>  
>  u = cpu->isar.id_mmfr4;
>  u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
> +t = FIELD_DP32(t, ID_MMFR4, CNP, 1); /* TTCNP */
>  cpu->isar.id_mmfr4 = u;

s/t/u/g

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

On Tue, 25 Feb 2020 at 19:10, Paolo Bonzini  wrote:
> This could go in independently. It would make Kashyap's series
> conflict, but I have already rebased it on top.

I'm happy to collect up 'docs' patches for pullreqs (and fix up
conflicts etc as they arise) if that helps in getting things into
the tree.

I feel like we're working a bit at cross purposes here so maybe
we'd benefit from just nailing down who's going to do what and
in which order?

My current thought on ordering is something like:
 * commit this
 * commit Kashyap's series
 * commit (an adjusted version of) your split-out-the-texi series
 * (automated) conversion of more texi -- all in one series I guess ?
 * ???
 * profit

but I'm not very strongly attached to that.

> Perhaps we could have the files in both .texi and (automatically
> converted) .rst versions at the same time in the tree for a short
> period. If that's okay for you, I can post tomorrow a series to do that.

My instinct is to say that that's a bit dangerous as it means
we might end up with changes to the "wrong" version of the
two files. Would it let us do the conversion faster or
more conveniently ?

thanks
-- PMM

[PATCH] target/arm: Implement (trivially) ARMv8.2-TTCNP

The ARMv8.2-TTCNP extension allows an implementation to optimize by
sharing TLB entries between multiple cores, provided that software
declares that it's ready to deal with this by setting a CnP bit in
the TTBRn_ELx.  It is mandatory from ARMv8.2 onward.

For QEMU's TLB implementation, sharing TLB entries between different
cores would not really benefit us and would be a lot of work to
implement.  So we implement this extension in the "trivial" manner:
we allow the guest to set and read back the CnP bit, but don't change
our behaviour (this is an architecturally valid implementation
choice).

The only code path which looks at the TTBRn_ELx values for the
long-descriptor format where the CnP bit is defined is already doing
enough masking to not get confused when the CnP bit at the bottom of
the register is set, so we can simply add a comment noting why we're
relying on that mask.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.c| 1 +
 target/arm/cpu64.c  | 2 ++
 target/arm/helper.c | 4 
 3 files changed, 7 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2eadf4dcb8b..64dc9509927 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2720,6 +2720,7 @@ static void arm_max_initfn(Object *obj)
 t = cpu->isar.id_mmfr4;
 t = FIELD_DP32(t, ID_MMFR4, HPDS, 1); /* AA32HPD */
 t = FIELD_DP32(t, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
+t = FIELD_DP32(t, ID_MMFR4, CNP, 1); /* TTCNP */
 cpu->isar.id_mmfr4 = t;
 }
 #endif
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 0929401a4dd..e4d793a2415 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -679,6 +679,7 @@ static void aarch64_max_initfn(Object *obj)
 
 t = cpu->isar.id_aa64mmfr2;
 t = FIELD_DP64(t, ID_AA64MMFR2, UAO, 1);
+t = FIELD_DP64(t, ID_AA64MMFR2, CNP, 1); /* TTCNP */
 cpu->isar.id_aa64mmfr2 = t;
 
 /* Replicate the same data to the 32-bit id registers.  */
@@ -705,6 +706,7 @@ static void aarch64_max_initfn(Object *obj)
 
 u = cpu->isar.id_mmfr4;
 u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
+t = FIELD_DP32(t, ID_MMFR4, CNP, 1); /* TTCNP */
 cpu->isar.id_mmfr4 = u;
 
 u = cpu->isar.id_aa64dfr0;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 79db169e046..911baf7bcb7 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10572,6 +10572,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
 
 /* Now we can extract the actual base address from the TTBR */
 descaddr = extract64(ttbr, 0, 48);
+/*
+ * We rely on this masking to clear the RES0 bits at the bottom of the TTBR
+ * and also to mask out CnP (bit 0) which could validly be non-zero.
+ */
 descaddr &= ~indexmask;
 
 /* The address field in the descriptor goes up to bit 39 for ARMv7
-- 
2.20.1

[Bug 1863819] Re: repeated KVM single step crashes leaks into SMP guest and crashes guest application

2020-02-25 Thread Dustin Spicuzza

Some experimentation with newer kernels indicate that this is most
likely a KVM bug.

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

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

Title:
  repeated KVM single step crashes leaks into SMP guest and crashes
  guest application

Status in QEMU:
  Invalid

Bug description:
  Guest: Windows 7 x64
  Host: Ubuntu 18.04.4 (kernel 5.3.0-40-generic)
  QEMU: master 6c599282f8ab382fe59f03a6cae755b89561a7b3

  If I try to use GDB to repeatedly single-step a userspace process
  while running a KVM guest, the userspace process will eventually crash
  with a 0x8004 exception (single step). This is easily reproducible
  on a Windows guest, I've not tried another guest type but I've been
  told it's the same there also.

  On a Ubuntu 16 host with an older kernel, this will hang the entire
  machine. However, it seems it may have been fixed by
  
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5cc244a20b86090c087073c124284381cdf47234
  ?

  It's not clear to me whether this is a KVM or a QEMU bug. A TCG guest
  does not crash the userspace process in the same way, but it does hang
  the VM.

  I've tried a variety of QEMU versions (3.0, 4.2, master) and they all
  exhibit the same behavior. I'm happy to dig into this more if someone
  can point me in the right direction.

  Here's the outline for reproducing the bug:

  * Compile iloop.cpp (attached) as a 32-bit application using MSVC
  * Start Windows 7 x64 guest under GDB
* Pass '-enable-kvm -smp 4,cores=2 -gdb tcp::4567' to QEMU along with other 
typical options

  (need to get CR3 to ensure we're in the right application context -- if 
there's an easier way to do this I'd love to hear it!)
  * Install WinDBG on guest
  * Copy SysInternals LiveKD to guest
  * Start iloop.exe in guest, note loop address
  * Run LiveKD from administrative prompt
* livekd64.exe -w
  * In WinDBG:
* !process 0 0
* Search for iloop.exe, note DirBase (this is CR3)

  In GDB:
  * Execute 'target remote tcp::4567'
  * Execute 'c'
  * Hit CTRL-C to pause the VM
  * Execute 'p/x $cr3'
.. continue if not equal to DirBase in WinDBG, keep stopping until it is 
equal
  * Once $cr3 is correct value, if you 'stepi' a few times you'll note the 
process going in a loop, it should keep hitting the address echoed to the 
console by iloop.exe

  Crash the process from GDB:
  * Execute 'stepi 1'
  * Watch the process, eventually it'll die with an 0x8004 error

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

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

Il mar 25 feb 2020, 19:57 Peter Maydell  ha
scritto:

> > The QAPI docs are in other manuals in docs/interop/, aren't they?
>
> Yes, but until we complete the conversion we can't get
> rid of the qemu-doc.html output, because that's where the
> HTML output from the QAPI doc generation goes.
>

Right.

> In general the result is more than acceptable, and I'd rather get a
> > quick-and-slightly-dirty conversion done quickly than do everything
> > manually but risk missing 5.0.
>
> Yeah, seems like a good plan. If the autoconversion works out
> then I think the main thing which makes "do all this for 5.0"
> at risk currently is that the qapidoc conversion series needs
> review and might need overhauling based on that review: it
> doesn't take many cycles of review-and-fix to push close to
> the softfreeze deadline.
>
> What do you want to do with this patchset?
>

This could go in independently. It would make Kashyap's series conflict,
but I have already rebased it on top.

Perhaps we could have the files in both .texi and (automatically converted)
.rst versions at the same time in the tree for a short period. If that's
okay for you, I can post tomorrow a series to do that.

Paolo

Re: [edk2-devel] A problem with live migration of UEFI virtual machines

2020-02-25 Thread Andrew Fish via

Laszlo,

If I understand this correctly is it not more complicated than just size. It 
also assumes the memory layout is the same? The legacy BIOS used fixed magic 
address ranges, but UEFI uses dynamically allocated memory so addresses are not 
fixed. While the UEFI firmware does try to keep S3 and S4 layouts consistent 
between boots, I'm not aware of any mechanism to keep the memory map address 
the same between versions of the firmware? 

Thanks,

Andrew Fish

> On Feb 25, 2020, at 9:53 AM, Laszlo Ersek  wrote:
> 
> On 02/24/20 16:28, Daniel P. Berrangé wrote:
>> On Tue, Feb 11, 2020 at 05:39:59PM +, Alex Bennée wrote:
>>> 
>>> wuchenye1995  writes:
>>> 
 Hi all,
   We found a problem with live migration of UEFI virtual machines
   due to size of OVMF.fd changes.
   Specifically, the size of OVMF.fd in edk with low version such as
   edk-2.0-25 is 2MB while the size of it in higher version such as
   edk-2.0-30 is 4MB.
   When we migrate a UEFI virtual machine from the host with low
   version of edk2 to the host with higher one, qemu component will
   report an error in function qemu_ram_resize while
 checking size of ovmf_pcbios: Length mismatch: pc.bios: 0x20 in
 != 0x40: Invalid argument.
   We want to know how to solve this problem after updating the
   version of edk2.
>>> 
>>> You can only migrate a machine that is identical - so instantiating a
>>> empty machine with a different EDK image is bound to cause a problem
>>> because the machines don't match.
>> 
>> I don't believe we are that strict for firmware in general. The
>> firmware is loaded when QEMU starts, but that only matters for the
>> original source host QEMU. During migration, the memory content of the
>> original firmware will be copied during live migration, overwriting
>> whatever the target QEMU loaded off disk. This worksprovided the
>> memory region is the same size on source & target host, which is where
>> the problem arises in this case.
>> 
>> If there's a risk that newer firmware will be larger than old firmware
>> there's only really two options:
>> 
>>  - Keep all firmware images forever, each with a unique versioned
>>filename. This ensures target QEMU will always load the original
>>smaller firmware
>> 
>>  - Add padding to the firmware images. IOW, if the firmware is 2 MB,
>>add zero-padding to the end of the image to round it upto 4 MB
>>(whatever you anticipate the largest size wil be in future).
>> 
>> Distros have often taken the latter approach for QEMU firmware in the
>> past. The main issue is that you have to plan ahead of time and get
>> this padding right from the very start. You can't add the padding
>> after the fact on an existing VM.
> 
> Following up here *too*, just for completeness.
> 
> The query in this thread has been posted three times now (and I have
> zero idea why). Each time it generated a different set of responses. For
> completes, I'm now going to link the other two threads here (because the
> present thread seems to have gotten the most feedback).
> 
> To the OP:
> 
> - please do *NOT* repost the same question once you get an answer. It
>  only fragments the discussion and creates confusion. It also doesn't
>  hurt if you *confirm* that you understood the answer.
> 
> - Yet further, if your email address has @gmail.com for domain, but your
>  msgids contain "tencent", that raises some eyebrows (mine for sure).
>  You say "we" in the query, but never identify the organization behind
>  the plural pronoun.
> 
> (I've been fuming about the triple-posting of the question for a while
> now, but it's only now that, upon seeing how much work Dan has put into
> his answer, I've decided that dishing out a bit of netiquette would be
> in order.)
> 
> * First posting:
> - msgid:   >
> - edk2-devel: https://edk2.groups.io/g/devel/message/54146 
> 
> - qemu-devel: 
> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02419.html 
> 
> 
>  * my response:
>- msgid:  <12553.1581366059422195...@groups.io 
> >
>- edk2-devel: https://edk2.groups.io/g/devel/message/54161 
> 
>- qemu-devel: none, because (as an exception) I used the stupid
>  groups.io  web interface to respond, and 
> so my response
>  never reached qemu-devel
> 
> * Second posting (~4 hours after the first)
> - msgid:   >
> - edk2-devel: https://edk2.groups.io/g/devel/message/54147 
> 
> - qemu-devel: 
> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02415.html 
>

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

On Tue, 25 Feb 2020 at 18:28, Paolo Bonzini  wrote:
>
> On 25/02/20 18:59, Peter Maydell wrote:
> > My assumption was that we would attack this by:
> >  * converting chunks of the documentation which are in qemu-doc.texi
> >but which aren't in the qemu.1 manpage (basically in the way this
> >series is doing)
> >  * get the qapidoc generation conversion reviewed and into
> >master (since at the moment it outputs into files included
> >from qemu-doc)
>
> The QAPI docs are in other manuals in docs/interop/, aren't they?

Yes, but until we complete the conversion we can't get
rid of the qemu-doc.html output, because that's where the
HTML output from the QAPI doc generation goes.

> > Incidentally:
> >> makeinfo -o - --docbook security.texi  | pandoc -f docbook -t rst
> > security texi was the really easy one here. I had to do more
> > manual formatting fixups on qemu-deprecated.texi which I'm
> > sceptical would have worked out as nicely done automatically.
>
> The automated conversion of qemu-deprecated.texi is indeed bad because
> the titles in the source are missing @code{...} to activate monospaced
> characters.

To be fair on the automated conversion, the markup in the
source texinfo here is suboptimal :-)

> > The automatic conversion rune also doesn't seem to get quotes
> > and apostrophes right: it has turned "guest B's disk image" into
> > something with a smartquote character in it, for instance.
>
> We probably don't want smartquotes at all, so you'd use "-t rst+smart"
> as the destination.  Also pandoc does not use the "::" at the end of the
> previous paragraph.  That can be fixed with for example
>
>   perl -e '$/=undef; $_ = <>; s/:\n\n::/::/g; print'
>
> In general the result is more than acceptable, and I'd rather get a
> quick-and-slightly-dirty conversion done quickly than do everything
> manually but risk missing 5.0.

Yeah, seems like a good plan. If the autoconversion works out
then I think the main thing which makes "do all this for 5.0"
at risk currently is that the qapidoc conversion series needs
review and might need overhauling based on that review: it
doesn't take many cycles of review-and-fix to push close to
the softfreeze deadline.

What do you want to do with this patchset?

thanks
-- PMM

Re: [PATCH v4 0/2] qemu-cpu-models: Convert to rST; document other MSR bits

On 25/02/20 17:56, Kashyap Chamarthy wrote:
> In v4:
>  - Correctly use the 'define-man-page' rule for qemu-cpu-models.7
>[pm215]
>  - Fix author attribution as per the thread:
>Message-ID:
>
>[danpb, pm215]
>  - Don't reverse the existing order of the list of CPU models [pm215]
>  - Use rST "definition lists" consistently throughout the document.
>  - Consistently capitalize the phrase: "The QEMU Project Developers"
>  - Update the year of copyright to 2020 in docs/conf.py
>  - Fix two minor rST-related things [pbonzini]

Thanks, I queued this series.  I'm not sure when it will be applied, as
that depends on how the rest of the rST conversion will be done.

Paolo

Re: [PATCH] hw/intc/arm_gic_kvm: Don't assume kernel can provide a GICv2

2020-02-25 Thread Philippe Mathieu-Daudé


On 2/25/20 7:24 PM, Peter Maydell wrote:

In our KVM GICv2 realize function, we try to cope with old kernels
that don't provide the device control API (KVM_CAP_DEVICE_CTRL): we
try to use the device control, and if that fails we fall back to
assuming that the kernel has the old style KVM_CREATE_IRQCHIP and
that it will provide a GICv2.

This doesn't cater for the possibility of a kernel and hardware which
only provide a GICv3, which is very common now.  On that setup we
will abort() later on in kvm_arm_pmu_set_irq() when we try to wire up
an interrupt to the GIC we failed to create:

qemu-system-aarch64: PMU: KVM_SET_DEVICE_ATTR: Invalid argument
qemu-system-aarch64: failed to set irq for PMU
Aborted

If the kernel advertises KVM_CAP_DEVICE_CTRL we should trust it if it
says it can't create a GICv2, rather than assuming it has one.  We
can then produce a more helpful error message including a hint about
the most probable reason for the failure.

If the kernel doesn't advertise KVM_CAP_DEVICE_CTRL then it is truly
ancient by this point but we might as well still fall back to a
KVM_CREATE_IRQCHIP GICv2.

With this patch then the user misconfiguration which previously
caused an abort now prints:
qemu-system-aarch64: Initialization of device kvm-arm-gic failed: error 
creating in-kernel VGIC: No such device
Perhaps the host CPU does not support GICv2?

Signed-off-by: Peter Maydell 
---
I spent a while wondering if the PMU code was broken before Marc
put me on the right track about what was going wrong (ie that
I hadn't put "-machine gic-version=host" on the commandline).

  hw/intc/arm_gic_kvm.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 9deb15e7e69..d7df423a7a3 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -551,7 +551,16 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error 
**errp)
KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true,
&error_abort);
  }
+} else if (kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
+error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
+error_append_hint(errp,
+  "Perhaps the host CPU does not support GICv2?\n");
  } else if (ret != -ENODEV && ret != -ENOTSUP) {
+/*
+ * Very ancient kernel without KVM_CAP_DEVICE_CTRL: assume that
+ * ENODEV or ENOTSUP mean "can't create GICv2 with KVM_CREATE_DEVICE",
+ * and that we will get a GICv2 via KVM_CREATE_IRQCHIP.
+ */
  error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
  return;
  }



Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v3 08/19] tests/iotests: be a little more forgiving on the size test

2020-02-25 Thread Philippe Mathieu-Daudé


On 2/25/20 7:22 PM, Stefan Berger wrote:

On 2/25/20 7:46 AM, Alex Bennée wrote:

At least on ZFS this was failing as 512 was less than or equal to 512.
I suspect the reason is additional compression done by ZFS and however
qemu-img gets the actual size.

Loosen the criteria to make sure after is not bigger than before and
also dump the values in the report.

Signed-off-by: Alex Bennée 
---
  tests/qemu-iotests/214 | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 3500e0c47a2..6d1324cd157 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -125,9 +125,9 @@ $QEMU_IO -c "write -P 0xcc $offset $data_size" 
"json:{\

  sizeB=$($QEMU_IMG info --output=json "$TEST_IMG" |
  sed -n '/"actual-size":/ s/[^0-9]//gp')

-if [ $sizeA -le $sizeB ]
+if [ $sizeA -lt $sizeB ]
  then
-    echo "Compression ERROR"
+    echo "Compression ERROR ($sizeA vs $sizeB)"
  fi


Nit: $sizeA < $sizeB ?


Reviewed-by: Philippe Mathieu-Daudé 



Reviewed-by: Stefan Berger 




  $QEMU_IMG check --output=json "$TEST_IMG" |

Re: [PULL 00/32] virtio, pc: fixes, features

2020-02-25 Thread Michael S. Tsirkin

On Tue, Feb 25, 2020 at 04:47:31PM +, Peter Maydell wrote:
> On Tue, 25 Feb 2020 at 15:12, Michael S. Tsirkin  wrote:
> >
> > The following changes since commit 9a8abceb5f01d1066d3a1ac5a33aabcbaeec1860:
> >
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-docs-20200225' 
> > into staging (2020-02-25 11:03:47 +)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to afc37debf27ecf34d6bc1d4b52fa0918d0bd3f3c:
> >
> >   Fixed assert in vhost_user_set_mem_table_postcopy (2020-02-25 08:47:47 
> > -0500)
> >
> > 
> > virtio, pc: fixes, features
> >
> > New virtio iommu.
> > Unrealize memory leaks.
> > In-band kick/call support.
> > Bugfixes, documentation all over the place.
> >
> > Signed-off-by: Michael S. Tsirkin 
> 
> Hi; this fails to build on OSX:
> 
>   CC  contrib/libvhost-user/libvhost-user.o
> /Users/pm215/src/qemu-for-merges/contrib/libvhost-user/libvhost-user.c:27:10:
> fatal error: 'sys/eventfd.h' file not found
> #include 
>  ^~~
> In file included from

weird this is not new.

> /Users/pm215/src/qemu-for-merges/contrib/vhost-user-bridge/main.c:37:
> /Users/pm215/src/qemu-for-merges/contrib/libvhost-user/libvhost-user.h:21:10:
> fatal error: 'linux/vhost.h' file not found
> #include 
>  ^~~
> 1 error generated.
> 
> thanks
> -- PMM

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

On 25/02/20 18:59, Peter Maydell wrote:
> My assumption was that we would attack this by:
>  * converting chunks of the documentation which are in qemu-doc.texi
>but which aren't in the qemu.1 manpage (basically in the way this
>series is doing)
>  * get the qapidoc generation conversion reviewed and into
>master (since at the moment it outputs into files included
>from qemu-doc)

The QAPI docs are in other manuals in docs/interop/, aren't they?

>  * convert the manpage parts; we have the machinery for dealing
>with the hxtool files, it just needs a little more work
>
>> (See also the patches I posted today, which take the opposite direction
>> of making qemu-doc.texi's structure more like what we'll have in the end
>> in docs/system).
> 
> This ought to make it easier to do the conversion of the
> various subparts, right?

Right, and easier to review as well; I called it "the opposite
direction" because the editing is done in Texinfo format and the rST
conversion becomes relatively trivial.  This would make it possible to
do the conversion in a branch and pull it all at once (apart from
qapidoc and possibly other small changes like removing obsolete parts).

> Incidentally:
>> makeinfo -o - --docbook security.texi  | pandoc -f docbook -t rst
> security texi was the really easy one here. I had to do more
> manual formatting fixups on qemu-deprecated.texi which I'm
> sceptical would have worked out as nicely done automatically.

The automated conversion of qemu-deprecated.texi is indeed bad because
the titles in the source are missing @code{...} to activate monospaced
characters.

> The automatic conversion rune also doesn't seem to get quotes
> and apostrophes right: it has turned "guest B's disk image" into
> something with a smartquote character in it, for instance.

We probably don't want smartquotes at all, so you'd use "-t rst+smart"
as the destination.  Also pandoc does not use the "::" at the end of the
previous paragraph.  That can be fixed with for example

  perl -e '$/=undef; $_ = <>; s/:\n\n::/::/g; print'

In general the result is more than acceptable, and I'd rather get a
quick-and-slightly-dirty conversion done quickly than do everything
manually but risk missing 5.0.

Paolo

[PATCH] hw/intc/arm_gic_kvm: Don't assume kernel can provide a GICv2

In our KVM GICv2 realize function, we try to cope with old kernels
that don't provide the device control API (KVM_CAP_DEVICE_CTRL): we
try to use the device control, and if that fails we fall back to
assuming that the kernel has the old style KVM_CREATE_IRQCHIP and
that it will provide a GICv2.

This doesn't cater for the possibility of a kernel and hardware which
only provide a GICv3, which is very common now.  On that setup we
will abort() later on in kvm_arm_pmu_set_irq() when we try to wire up
an interrupt to the GIC we failed to create:

qemu-system-aarch64: PMU: KVM_SET_DEVICE_ATTR: Invalid argument
qemu-system-aarch64: failed to set irq for PMU
Aborted

If the kernel advertises KVM_CAP_DEVICE_CTRL we should trust it if it
says it can't create a GICv2, rather than assuming it has one.  We
can then produce a more helpful error message including a hint about
the most probable reason for the failure.

If the kernel doesn't advertise KVM_CAP_DEVICE_CTRL then it is truly
ancient by this point but we might as well still fall back to a
KVM_CREATE_IRQCHIP GICv2.

With this patch then the user misconfiguration which previously
caused an abort now prints:
qemu-system-aarch64: Initialization of device kvm-arm-gic failed: error 
creating in-kernel VGIC: No such device
Perhaps the host CPU does not support GICv2?

Signed-off-by: Peter Maydell 
---
I spent a while wondering if the PMU code was broken before Marc
put me on the right track about what was going wrong (ie that
I hadn't put "-machine gic-version=host" on the commandline).

 hw/intc/arm_gic_kvm.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 9deb15e7e69..d7df423a7a3 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -551,7 +551,16 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error 
**errp)
   KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true,
   &error_abort);
 }
+} else if (kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
+error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
+error_append_hint(errp,
+  "Perhaps the host CPU does not support GICv2?\n");
 } else if (ret != -ENODEV && ret != -ENOTSUP) {
+/*
+ * Very ancient kernel without KVM_CAP_DEVICE_CTRL: assume that
+ * ENODEV or ENOTSUP mean "can't create GICv2 with KVM_CREATE_DEVICE",
+ * and that we will get a GICv2 via KVM_CREATE_IRQCHIP.
+ */
 error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
 return;
 }
-- 
2.20.1

Re: [PATCH v3 08/19] tests/iotests: be a little more forgiving on the size test

2020-02-25 Thread Stefan Berger


On 2/25/20 7:46 AM, Alex Bennée wrote:

At least on ZFS this was failing as 512 was less than or equal to 512.
I suspect the reason is additional compression done by ZFS and however
qemu-img gets the actual size.

Loosen the criteria to make sure after is not bigger than before and
also dump the values in the report.

Signed-off-by: Alex Bennée 
---
  tests/qemu-iotests/214 | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 3500e0c47a2..6d1324cd157 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -125,9 +125,9 @@ $QEMU_IO -c "write -P 0xcc $offset $data_size" "json:{\
  sizeB=$($QEMU_IMG info --output=json "$TEST_IMG" |
  sed -n '/"actual-size":/ s/[^0-9]//gp')

-if [ $sizeA -le $sizeB ]
+if [ $sizeA -lt $sizeB ]
  then
-echo "Compression ERROR"
+echo "Compression ERROR ($sizeA vs $sizeB)"
  fi


Nit: $sizeA < $sizeB ?

Reviewed-by: Stefan Berger 




  $QEMU_IMG check --output=json "$TEST_IMG" |

[PATCH v4 7/7] target/arm: Honor the HCR_EL2.TTLB bit

This bit traps EL1 access to tlb maintenance insns.

Signed-off-by: Richard Henderson 
---
 target/arm/helper.c | 85 +
 1 file changed, 55 insertions(+), 30 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ca67d6a770..20688d1a18 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -563,6 +563,16 @@ static CPAccessResult access_tacr(CPUARMState *env, const 
ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+/* Check for traps from EL1 due to HCR_EL2.TTLB. */
+static CPAccessResult access_ttlb(CPUARMState *env, const ARMCPRegInfo *ri,
+  bool isread)
+{
+if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TTLB)) {
+return CP_ACCESS_TRAP_EL2;
+}
+return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
value)
 {
 ARMCPU *cpu = env_archcpu(env);
@@ -2285,41 +2295,53 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
   .type = ARM_CP_NO_RAW, .access = PL1_R, .readfn = isr_read },
 /* 32 bit ITLB invalidates */
 { .name = "ITLBIALL", .cp = 15, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 0,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiall_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiall_write },
 { .name = "ITLBIMVA", .cp = 15, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 1,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbimva_write },
 { .name = "ITLBIASID", .cp = 15, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 2,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiasid_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiasid_write },
 /* 32 bit DTLB invalidates */
 { .name = "DTLBIALL", .cp = 15, .opc1 = 0, .crn = 8, .crm = 6, .opc2 = 0,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiall_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiall_write },
 { .name = "DTLBIMVA", .cp = 15, .opc1 = 0, .crn = 8, .crm = 6, .opc2 = 1,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbimva_write },
 { .name = "DTLBIASID", .cp = 15, .opc1 = 0, .crn = 8, .crm = 6, .opc2 = 2,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiasid_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiasid_write },
 /* 32 bit TLB invalidates */
 { .name = "TLBIALL", .cp = 15, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 0,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiall_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiall_write },
 { .name = "TLBIMVA", .cp = 15, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 1,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbimva_write },
 { .name = "TLBIASID", .cp = 15, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 2,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiasid_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiasid_write },
 { .name = "TLBIMVAA", .cp = 15, .opc1 = 0, .crn = 8, .crm = 7, .opc2 = 3,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimvaa_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbimvaa_write },
 REGINFO_SENTINEL
 };
 
 static const ARMCPRegInfo v7mp_cp_reginfo[] = {
 /* 32 bit TLB invalidates, Inner Shareable */
 { .name = "TLBIALLIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 0,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbiall_is_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbiall_is_write },
 { .name = "TLBIMVAIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 1,
-  .type = ARM_CP_NO_RAW, .access = PL1_W, .writefn = tlbimva_is_write },
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
+  .writefn = tlbimva_is_write },
 { .name = "TLBIASIDIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 2,
-  .type = ARM_CP_NO_RAW, .access = PL1_W,
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
   .writefn = tlbiasid_is_write },
 { .name = "TLBIMVAAIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 3,
-  .type = ARM_CP_NO_RAW, .access = PL1_W,
+  .type = ARM_CP_NO_RAW, .access = PL1_W, .accessfn = access_ttlb,
   .writefn = tlbimvaa_is_write },
 REGINFO_SENTINEL
 };
@@ -4780,51 +4802,51 @@ static

[PATCH v4 6/7] target/arm: Honor the HCR_EL2.TPU bit

This bit traps EL1 access to cache maintenance insns that operate
to the point of unification.  There are no longer any references to
plain aa64_cacheop_access, so remove it.

Signed-off-by: Richard Henderson 
---
v4: Fix el0 fallthru (pmm).
---
 target/arm/helper.c | 53 +++--
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2d488554b7..ca67d6a770 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4301,19 +4301,6 @@ static const ARMCPRegInfo uao_reginfo = {
 .readfn = aa64_uao_read, .writefn = aa64_uao_write
 };
 
-static CPAccessResult aa64_cacheop_access(CPUARMState *env,
-  const ARMCPRegInfo *ri,
-  bool isread)
-{
-/* Cache invalidate/clean: NOP, but EL0 must UNDEF unless
- * SCTLR_EL1.UCI is set.
- */
-if (arm_current_el(env) == 0 && !(arm_sctlr(env, 0) & SCTLR_UCI)) {
-return CP_ACCESS_TRAP;
-}
-return CP_ACCESS_OK;
-}
-
 static CPAccessResult aa64_cacheop_poc_access(CPUARMState *env,
   const ARMCPRegInfo *ri,
   bool isread)
@@ -4336,6 +4323,28 @@ static CPAccessResult 
aa64_cacheop_poc_access(CPUARMState *env,
 return CP_ACCESS_OK;
 }
 
+static CPAccessResult aa64_cacheop_pou_access(CPUARMState *env,
+  const ARMCPRegInfo *ri,
+  bool isread)
+{
+/* Cache invalidate/clean to Point of Unification... */
+switch (arm_current_el(env)) {
+case 0:
+/* ... EL0 must UNDEF unless SCTLR_EL1.UCI is set.  */
+if (!(arm_sctlr(env, 0) & SCTLR_UCI)) {
+return CP_ACCESS_TRAP;
+}
+/* fall through */
+case 1:
+/* ... EL1 must trap to EL2 if HCR_EL2.TPU is set.  */
+if (arm_hcr_el2_eff(env) & HCR_TPU) {
+return CP_ACCESS_TRAP_EL2;
+}
+break;
+}
+return CP_ACCESS_OK;
+}
+
 /* See: D4.7.2 TLB maintenance requirements and the TLB maintenance 
instructions
  * Page D4-1736 (DDI0487A.b)
  */
@@ -4733,14 +4742,16 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 /* Cache ops: all NOPs since we don't emulate caches */
 { .name = "IC_IALLUIS", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 0,
-  .access = PL1_W, .type = ARM_CP_NOP },
+  .access = PL1_W, .type = ARM_CP_NOP,
+  .accessfn = aa64_cacheop_pou_access },
 { .name = "IC_IALLU", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 5, .opc2 = 0,
-  .access = PL1_W, .type = ARM_CP_NOP },
+  .access = PL1_W, .type = ARM_CP_NOP,
+  .accessfn = aa64_cacheop_pou_access },
 { .name = "IC_IVAU", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 5, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
-  .accessfn = aa64_cacheop_access },
+  .accessfn = aa64_cacheop_pou_access },
 { .name = "DC_IVAC", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 1,
   .access = PL1_W, .accessfn = aa64_cacheop_poc_access,
@@ -4758,7 +4769,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = "DC_CVAU", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 11, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
-  .accessfn = aa64_cacheop_access },
+  .accessfn = aa64_cacheop_pou_access },
 { .name = "DC_CIVAC", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 14, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
@@ -4932,13 +4943,13 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .writefn = tlbiipas2_is_write },
 /* 32 bit cache operations */
 { .name = "ICIALLUIS", .cp = 15, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 0,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_pou_access 
},
 { .name = "BPIALLUIS", .cp = 15, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 6,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "ICIALLU", .cp = 15, .opc1 = 0, .crn = 7, .crm = 5, .opc2 = 0,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_pou_access 
},
 { .name = "ICIMVAU", .cp = 15, .opc1 = 0, .crn = 7, .crm = 5, .opc2 = 1,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_pou_access 
},
 { .name = "BPIALL", .cp = 15, .opc1 = 0, .crn = 7, .crm = 5, .opc2 = 6,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "BPIMVA", .cp = 15, .opc1 = 0, .crn = 7, .crm = 5, .opc2 = 7,
@@ -4952,7 +4963,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = "DCCSW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 10, .opc2

[PATCH v4 5/7] target/arm: Honor the HCR_EL2.TPCP bit

This bit traps EL1 access to cache maintenance insns that operate
to the point of coherency or persistence.

Signed-off-by: Richard Henderson 
---
v4: Fix el0 fallthru (pmm).
---
 target/arm/helper.c | 39 +++
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5cb7844f3f..2d488554b7 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4314,6 +4314,28 @@ static CPAccessResult aa64_cacheop_access(CPUARMState 
*env,
 return CP_ACCESS_OK;
 }
 
+static CPAccessResult aa64_cacheop_poc_access(CPUARMState *env,
+  const ARMCPRegInfo *ri,
+  bool isread)
+{
+/* Cache invalidate/clean to Point of Coherency or Persistence...  */
+switch (arm_current_el(env)) {
+case 0:
+/* ... EL0 must UNDEF unless SCTLR_EL1.UCI is set.  */
+if (!(arm_sctlr(env, 0) & SCTLR_UCI)) {
+return CP_ACCESS_TRAP;
+}
+/* fall through */
+case 1:
+/* ... EL1 must trap to EL2 if HCR_EL2.TPCP is set.  */
+if (arm_hcr_el2_eff(env) & HCR_TPCP) {
+return CP_ACCESS_TRAP_EL2;
+}
+break;
+}
+return CP_ACCESS_OK;
+}
+
 /* See: D4.7.2 TLB maintenance requirements and the TLB maintenance 
instructions
  * Page D4-1736 (DDI0487A.b)
  */
@@ -4721,14 +4743,15 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .accessfn = aa64_cacheop_access },
 { .name = "DC_IVAC", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 1,
-  .access = PL1_W, .type = ARM_CP_NOP },
+  .access = PL1_W, .accessfn = aa64_cacheop_poc_access,
+  .type = ARM_CP_NOP },
 { .name = "DC_ISW", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 2,
   .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
 { .name = "DC_CVAC", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 10, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
-  .accessfn = aa64_cacheop_access },
+  .accessfn = aa64_cacheop_poc_access },
 { .name = "DC_CSW", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 10, .opc2 = 2,
   .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
@@ -4739,7 +4762,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = "DC_CIVAC", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 14, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
-  .accessfn = aa64_cacheop_access },
+  .accessfn = aa64_cacheop_poc_access },
 { .name = "DC_CISW", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 2,
   .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
@@ -4921,17 +4944,17 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = "BPIMVA", .cp = 15, .opc1 = 0, .crn = 7, .crm = 5, .opc2 = 7,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "DCIMVAC", .cp = 15, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 1,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_poc_access 
},
 { .name = "DCISW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 2,
   .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw },
 { .name = "DCCMVAC", .cp = 15, .opc1 = 0, .crn = 7, .crm = 10, .opc2 = 1,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_poc_access 
},
 { .name = "DCCSW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 10, .opc2 = 2,
   .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw },
 { .name = "DCCMVAU", .cp = 15, .opc1 = 0, .crn = 7, .crm = 11, .opc2 = 1,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "DCCIMVAC", .cp = 15, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 1,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_poc_access 
},
 { .name = "DCCISW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 2,
   .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw },
 /* MMU Domain access control / MPU write buffer control */
@@ -6728,7 +6751,7 @@ static const ARMCPRegInfo dcpop_reg[] = {
 { .name = "DC_CVAP", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 12, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
-  .accessfn = aa64_cacheop_access, .writefn = dccvap_writefn },
+  .accessfn = aa64_cacheop_poc_access, .writefn = dccvap_writefn },
 REGINFO_SENTINEL
 };
 
@@ -6736,7 +6759,7 @@ static const ARMCPRegInfo dcpodp_reg[] = {
 { .name = "DC_CVADP", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 13, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_S

[PATCH v4 4/7] target/arm: Honor the HCR_EL2.TACR bit

This bit traps EL1 access to the auxiliary control registers.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/arm/helper.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ea42e0d26d..5cb7844f3f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -553,6 +553,16 @@ static CPAccessResult access_tsw(CPUARMState *env, const 
ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+/* Check for traps from EL1 due to HCR_EL2.TACR.  */
+static CPAccessResult access_tacr(CPUARMState *env, const ARMCPRegInfo *ri,
+  bool isread)
+{
+if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TACR)) {
+return CP_ACCESS_TRAP_EL2;
+}
+return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
value)
 {
 ARMCPU *cpu = env_archcpu(env);
@@ -6924,8 +6934,8 @@ static const ARMCPRegInfo ats1cp_reginfo[] = {
 static const ARMCPRegInfo actlr2_hactlr2_reginfo[] = {
 { .name = "ACTLR2", .state = ARM_CP_STATE_AA32,
   .cp = 15, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 3,
-  .access = PL1_RW, .type = ARM_CP_CONST,
-  .resetvalue = 0 },
+  .access = PL1_RW, .accessfn = access_tacr,
+  .type = ARM_CP_CONST, .resetvalue = 0 },
 { .name = "HACTLR2", .state = ARM_CP_STATE_AA32,
   .cp = 15, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 3,
   .access = PL2_RW, .type = ARM_CP_CONST,
@@ -7681,8 +7691,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 ARMCPRegInfo auxcr_reginfo[] = {
 { .name = "ACTLR_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 1,
-  .access = PL1_RW, .type = ARM_CP_CONST,
-  .resetvalue = cpu->reset_auxcr },
+  .access = PL1_RW, .accessfn = access_tacr,
+  .type = ARM_CP_CONST, .resetvalue = cpu->reset_auxcr },
 { .name = "ACTLR_EL2", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 1,
   .access = PL2_RW, .type = ARM_CP_CONST,
-- 
2.20.1

[PATCH v4 2/7] target/arm: Honor the HCR_EL2.{TVM,TRVM} bits

These bits trap EL1 access to various virtual memory controls.

Buglink: https://bugs.launchpad.net/bugs/1855072
Signed-off-by: Richard Henderson 
---
v2: Include TTBCR.
v4: Include not_v8_cp_reginfo, lpae_cp_reginfo, CONTEXTIDR_S;
exclude not_v7_cp_reginfo (pmm).
---
 target/arm/helper.c | 82 ++---
 1 file changed, 55 insertions(+), 27 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index d65160fdb3..e45d717cf3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -530,6 +530,19 @@ static CPAccessResult access_tpm(CPUARMState *env, const 
ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+/* Check for traps from EL1 due to HCR_EL2.TVM and HCR_EL2.TRVM.  */
+static CPAccessResult access_tvm_trvm(CPUARMState *env, const ARMCPRegInfo *ri,
+  bool isread)
+{
+if (arm_current_el(env) == 1) {
+uint64_t trap = isread ? HCR_TRVM : HCR_TVM;
+if (arm_hcr_el2_eff(env) & trap) {
+return CP_ACCESS_TRAP_EL2;
+}
+}
+return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
value)
 {
 ARMCPU *cpu = env_archcpu(env);
@@ -785,12 +798,14 @@ static const ARMCPRegInfo cp_reginfo[] = {
  */
 { .name = "CONTEXTIDR_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 13, .crm = 0, .opc2 = 1,
-  .access = PL1_RW, .secure = ARM_CP_SECSTATE_NS,
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
+  .secure = ARM_CP_SECSTATE_NS,
   .fieldoffset = offsetof(CPUARMState, cp15.contextidr_el[1]),
   .resetvalue = 0, .writefn = contextidr_write, .raw_writefn = raw_write, 
},
 { .name = "CONTEXTIDR_S", .state = ARM_CP_STATE_AA32,
   .cp = 15, .opc1 = 0, .crn = 13, .crm = 0, .opc2 = 1,
-  .access = PL1_RW, .secure = ARM_CP_SECSTATE_S,
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
+  .secure = ARM_CP_SECSTATE_S,
   .fieldoffset = offsetof(CPUARMState, cp15.contextidr_s),
   .resetvalue = 0, .writefn = contextidr_write, .raw_writefn = raw_write, 
},
 REGINFO_SENTINEL
@@ -803,7 +818,7 @@ static const ARMCPRegInfo not_v8_cp_reginfo[] = {
 /* MMU Domain access control / MPU write buffer control */
 { .name = "DACR",
   .cp = 15, .opc1 = CP_ANY, .crn = 3, .crm = CP_ANY, .opc2 = CP_ANY,
-  .access = PL1_RW, .resetvalue = 0,
+  .access = PL1_RW, .accessfn = access_tvm_trvm, .resetvalue = 0,
   .writefn = dacr_write, .raw_writefn = raw_write,
   .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.dacr_s),
  offsetoflow32(CPUARMState, cp15.dacr_ns) } },
@@ -996,7 +1011,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
 { .name = "DMB", .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 5,
   .access = PL0_W, .type = ARM_CP_NOP },
 { .name = "IFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 2,
-  .access = PL1_RW,
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
   .bank_fieldoffsets = { offsetof(CPUARMState, cp15.ifar_s),
  offsetof(CPUARMState, cp15.ifar_ns) },
   .resetvalue = 0, },
@@ -2208,16 +2223,19 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
  */
 { .name = "AFSR0_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 1, .opc2 = 0,
-  .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
+  .type = ARM_CP_CONST, .resetvalue = 0 },
 { .name = "AFSR1_EL1", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 1, .opc2 = 1,
-  .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
+  .type = ARM_CP_CONST, .resetvalue = 0 },
 /* MAIR can just read-as-written because we don't implement caches
  * and so don't need to care about memory attributes.
  */
 { .name = "MAIR_EL1", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0,
-  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[1]),
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
+  .fieldoffset = offsetof(CPUARMState, cp15.mair_el[1]),
   .resetvalue = 0 },
 { .name = "MAIR_EL3", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 6, .crn = 10, .crm = 2, .opc2 = 0,
@@ -2231,12 +2249,14 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
   * handled in the field definitions.
   */
 { .name = "MAIR0", .state = ARM_CP_STATE_AA32,
-  .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0, .access = PL1_RW,
+  .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0,
+  .access = PL1_RW, .accessfn = access_tvm_trvm,
   .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair0_s),
  offsetof(CPUARMState, cp15.mair0_ns) },
   .resetfn = arm_cp_reset_ignore },
 { .name = "

[PATCH v4 1/7] target/arm: Improve masking of HCR RES0 bits

Don't merely start with v8.0, handle v7VE as well.
Notice writes from aarch32 mode, and the bits that
ought not be settable from there.

Suggested-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/arm/helper.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 79db169e04..d65160fdb3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5089,8 +5089,13 @@ static const ARMCPRegInfo el3_no_el2_v8_cp_reginfo[] = {
 static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
 ARMCPU *cpu = env_archcpu(env);
-/* Begin with bits defined in base ARMv8.0.  */
-uint64_t valid_mask = MAKE_64BIT_MASK(0, 34);
+uint64_t valid_mask;
+
+if (arm_feature(env, ARM_FEATURE_V8)) {
+valid_mask = MAKE_64BIT_MASK(0, 34);  /* ARMv8.0 */
+} else {
+valid_mask = MAKE_64BIT_MASK(0, 28);  /* ARMv7VE */
+}
 
 if (arm_feature(env, ARM_FEATURE_EL3)) {
 valid_mask &= ~HCR_HCD;
@@ -5114,6 +5119,14 @@ static void hcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri, uint64_t value)
 valid_mask |= HCR_API | HCR_APK;
 }
 
+if (ri->state == ARM_CP_STATE_AA32) {
+/*
+ * Writes from aarch32 mode have more RES0 bits.
+ * This includes TDZ, RW, E2H, and more.
+ */
+valid_mask &= ~0xff80ff8c9000ull;
+}
+
 /* Clear RES0 bits.  */
 value &= valid_mask;
 
-- 
2.20.1

[PATCH v4 3/7] target/arm: Honor the HCR_EL2.TSW bit

These bits trap EL1 access to set/way cache maintenance insns.

Buglink: https://bugs.launchpad.net/bugs/1863685
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/arm/helper.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index e45d717cf3..ea42e0d26d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -543,6 +543,16 @@ static CPAccessResult access_tvm_trvm(CPUARMState *env, 
const ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+/* Check for traps from EL1 due to HCR_EL2.TSW.  */
+static CPAccessResult access_tsw(CPUARMState *env, const ARMCPRegInfo *ri,
+ bool isread)
+{
+if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TSW)) {
+return CP_ACCESS_TRAP_EL2;
+}
+return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
value)
 {
 ARMCPU *cpu = env_archcpu(env);
@@ -4704,14 +4714,14 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .access = PL1_W, .type = ARM_CP_NOP },
 { .name = "DC_ISW", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 2,
-  .access = PL1_W, .type = ARM_CP_NOP },
+  .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
 { .name = "DC_CVAC", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 10, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
   .accessfn = aa64_cacheop_access },
 { .name = "DC_CSW", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 10, .opc2 = 2,
-  .access = PL1_W, .type = ARM_CP_NOP },
+  .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
 { .name = "DC_CVAU", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 11, .opc2 = 1,
   .access = PL0_W, .type = ARM_CP_NOP,
@@ -4722,7 +4732,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .accessfn = aa64_cacheop_access },
 { .name = "DC_CISW", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 2,
-  .access = PL1_W, .type = ARM_CP_NOP },
+  .access = PL1_W, .accessfn = access_tsw, .type = ARM_CP_NOP },
 /* TLBI operations */
 { .name = "TLBI_VMALLE1IS", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 0,
@@ -4903,17 +4913,17 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = "DCIMVAC", .cp = 15, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 1,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "DCISW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 6, .opc2 = 2,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw },
 { .name = "DCCMVAC", .cp = 15, .opc1 = 0, .crn = 7, .crm = 10, .opc2 = 1,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "DCCSW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 10, .opc2 = 2,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw },
 { .name = "DCCMVAU", .cp = 15, .opc1 = 0, .crn = 7, .crm = 11, .opc2 = 1,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "DCCIMVAC", .cp = 15, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 1,
   .type = ARM_CP_NOP, .access = PL1_W },
 { .name = "DCCISW", .cp = 15, .opc1 = 0, .crn = 7, .crm = 14, .opc2 = 2,
-  .type = ARM_CP_NOP, .access = PL1_W },
+  .type = ARM_CP_NOP, .access = PL1_W, .accessfn = access_tsw },
 /* MMU Domain access control / MPU write buffer control */
 { .name = "DACR", .cp = 15, .opc1 = 0, .crn = 3, .crm = 0, .opc2 = 0,
   .access = PL1_RW, .accessfn = access_tvm_trvm, .resetvalue = 0,
-- 
2.20.1

[PATCH v4 0/7] target/arm: Honor more HCR_EL2 traps

Changes for v4:
  * Mask more res0 bits for HCR.
  * Fixes for TVM/TRVM.
  * Fixes for el0 for TPCP & TPU.

r~

Richard Henderson (7):
  target/arm: Improve masking of HCR RES0 bits
  target/arm: Honor the HCR_EL2.{TVM,TRVM} bits
  target/arm: Honor the HCR_EL2.TSW bit
  target/arm: Honor the HCR_EL2.TACR bit
  target/arm: Honor the HCR_EL2.TPCP bit
  target/arm: Honor the HCR_EL2.TPU bit
  target/arm: Honor the HCR_EL2.TTLB bit

 target/arm/helper.c | 306 ++--
 1 file changed, 213 insertions(+), 93 deletions(-)

-- 
2.20.1

Re: [PATCH 0/4] docs: Miscellaneous rST conversions

On Tue, 25 Feb 2020 at 17:48, Paolo Bonzini  wrote:
>
> On 25/02/20 18:11, Peter Maydell wrote:
> >> I assume these are not meant to be applied now, except patch 2?
> > No, I intended them to be reviewable and applied now. Why
> > do you think we should wait?
>
> Because they remove information from qemu-doc.texi.  I think it's
> feasible to do a mass conversion quite soon, within a single pull
> request, the only important part that is missing is the hxtool conversion.

My assumption was that we would attack this by:
 * converting chunks of the documentation which are in qemu-doc.texi
   but which aren't in the qemu.1 manpage (basically in the way this
   series is doing)
 * get the qapidoc generation conversion reviewed and into
   master (since at the moment it outputs into files included
   from qemu-doc)
 * convert the manpage parts; we have the machinery for dealing
   with the hxtool files, it just needs a little more work

> (See also the patches I posted today, which take the opposite direction
> of making qemu-doc.texi's structure more like what we'll have in the end
> in docs/system).

This ought to make it easier to do the conversion of the
various subparts, right?

Incidentally:
> makeinfo -o - --docbook security.texi  | pandoc -f docbook -t rst

security texi was the really easy one here. I had to do more
manual formatting fixups on qemu-deprecated.texi which I'm
sceptical would have worked out as nicely done automatically.

The automatic conversion rune also doesn't seem to get quotes
and apostrophes right: it has turned "guest B's disk image" into
something with a smartquote character in it, for instance.

thanks
-- PMM

Re: [edk2-devel] A problem with live migration of UEFI virtual machines

2020-02-25 Thread Laszlo Ersek

On 02/24/20 16:28, Daniel P. Berrangé wrote:
> On Tue, Feb 11, 2020 at 05:39:59PM +, Alex Bennée wrote:
>>
>> wuchenye1995  writes:
>>
>>> Hi all,
>>>We found a problem with live migration of UEFI virtual machines
>>>due to size of OVMF.fd changes.
>>>Specifically, the size of OVMF.fd in edk with low version such as
>>>edk-2.0-25 is 2MB while the size of it in higher version such as
>>>edk-2.0-30 is 4MB.
>>>When we migrate a UEFI virtual machine from the host with low
>>>version of edk2 to the host with higher one, qemu component will
>>>report an error in function qemu_ram_resize while
>>> checking size of ovmf_pcbios: Length mismatch: pc.bios: 0x20 in
>>> != 0x40: Invalid argument.
>>>We want to know how to solve this problem after updating the
>>>version of edk2.
>>
>> You can only migrate a machine that is identical - so instantiating a
>> empty machine with a different EDK image is bound to cause a problem
>> because the machines don't match.
>
> I don't believe we are that strict for firmware in general. The
> firmware is loaded when QEMU starts, but that only matters for the
> original source host QEMU. During migration, the memory content of the
> original firmware will be copied during live migration, overwriting
> whatever the target QEMU loaded off disk. This worksprovided the
> memory region is the same size on source & target host, which is where
> the problem arises in this case.
>
> If there's a risk that newer firmware will be larger than old firmware
> there's only really two options:
>
>   - Keep all firmware images forever, each with a unique versioned
> filename. This ensures target QEMU will always load the original
> smaller firmware
>
>   - Add padding to the firmware images. IOW, if the firmware is 2 MB,
> add zero-padding to the end of the image to round it upto 4 MB
> (whatever you anticipate the largest size wil be in future).
>
> Distros have often taken the latter approach for QEMU firmware in the
> past. The main issue is that you have to plan ahead of time and get
> this padding right from the very start. You can't add the padding
> after the fact on an existing VM.

Following up here *too*, just for completeness.

The query in this thread has been posted three times now (and I have
zero idea why). Each time it generated a different set of responses. For
completes, I'm now going to link the other two threads here (because the
present thread seems to have gotten the most feedback).

To the OP:

- please do *NOT* repost the same question once you get an answer. It
  only fragments the discussion and creates confusion. It also doesn't
  hurt if you *confirm* that you understood the answer.

- Yet further, if your email address has @gmail.com for domain, but your
  msgids contain "tencent", that raises some eyebrows (mine for sure).
  You say "we" in the query, but never identify the organization behind
  the plural pronoun.

(I've been fuming about the triple-posting of the question for a while
now, but it's only now that, upon seeing how much work Dan has put into
his answer, I've decided that dishing out a bit of netiquette would be
in order.)

* First posting:
- msgid:  
- edk2-devel: https://edk2.groups.io/g/devel/message/54146
- qemu-devel: 
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02419.html

  * my response:
- msgid:  <12553.1581366059422195...@groups.io>
- edk2-devel: https://edk2.groups.io/g/devel/message/54161
- qemu-devel: none, because (as an exception) I used the stupid
  groups.io web interface to respond, and so my response
  never reached qemu-devel

* Second posting (~4 hours after the first)
- msgid:  
- edk2-devel: https://edk2.groups.io/g/devel/message/54147
- qemu-devel: 
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02415.html

  * Dave's response:
- msgid:  <20200220154742.GC2882@work-vm>
- edk2-devel: https://edk2.groups.io/g/devel/message/54681
- qemu-devel: 
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg05632.html

* Third posting (next day, present thread) -- cross posted to yet
  another list (!), because apparently Dave's feedback and mine had not
  been enough:
- msgid:
- edk2-devel:   https://edk2.groups.io/g/devel/message/54220
- edk2-discuss: https://edk2.groups.io/g/discuss/message/135
- qemu-devel:   
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02735.html

Back on topic: see my response again. The answer is, you can't solve the
problem (specifically with OVMF), and QEMU in fact does you service by
preventing the migration.

Laszlo

Re: [PATCH v3 7/7] target/arm: Honor the HCR_EL2.TTLB bit

On Tue, 25 Feb 2020 at 17:46, Richard Henderson
 wrote:
>
> On 2/25/20 4:01 AM, Peter Maydell wrote:
> > The set of operations this traps differs when ARMv8.4-TLBI is
> > implemented. It looks like you've applied this access fn to
> > the wider with-v8.4-TLBI set? (eg TLBI_VMALLE1 is only trapped
> > with ARMv8.4-TLBI, not without.)
>
> Um, that's not true.
>
> ARMv8.4-TLBI adds the *OS and R* insns, and extends this bit to apply there.
> But all of the original v8.0 insns are unchanged.

Oh, the spec has confused me by listing the TLBI operations
in a different order in the "with v8.4-TLBI" section (where
it starts with 'TLBI VMALLE1') and in the "otherwise" section
(where it starts with 'TLBI VMALLE1IS' but 'TLBI VMALLE1'
is still in the list, just later on).

thanks
-- PMM

Re: [PATCH 0/4] docs: Miscellaneous rST conversions