* [PULL 1/6] target/arm: Fix MemOp for STGP
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
@ 2023-07-31 14:15 ` Peter Maydell
2023-07-31 14:15 ` [PULL 2/6] elf2dmp: Don't abandon when Prcb is set to 0 Peter Maydell
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2023-07-31 14:15 UTC (permalink / raw)
To: qemu-devel
From: Richard Henderson <richard.henderson@linaro.org>
When converting to decodetree, the code to rebuild mop for the pair
only made it into trans_STP and not into trans_STGP.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1790
Fixes: 8c212eb6594 ("target/arm: Convert load/store-pair to decodetree")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20230726165416.309624-1-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/tcg/translate-a64.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index ef0c47407a9..5fa1257d32a 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -3004,6 +3004,9 @@ static bool trans_STGP(DisasContext *s, arg_ldstpair *a)
MemOp mop;
TCGv_i128 tmp;
+ /* STGP only comes in one size. */
+ tcg_debug_assert(a->sz == MO_64);
+
if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
return false;
}
@@ -3029,13 +3032,25 @@ static bool trans_STGP(DisasContext *s, arg_ldstpair *a)
gen_helper_stg(cpu_env, dirty_addr, dirty_addr);
}
- mop = finalize_memop(s, a->sz);
- clean_addr = gen_mte_checkN(s, dirty_addr, true, false, 2 << a->sz, mop);
+ mop = finalize_memop(s, MO_64);
+ clean_addr = gen_mte_checkN(s, dirty_addr, true, false, 2 << MO_64, mop);
tcg_rt = cpu_reg(s, a->rt);
tcg_rt2 = cpu_reg(s, a->rt2);
- assert(a->sz == 3);
+ /*
+ * STGP is defined as two 8-byte memory operations and one tag operation.
+ * We implement it as one single 16-byte memory operation for convenience.
+ * Rebuild mop as for STP.
+ * TODO: The atomicity with LSE2 is stronger than required.
+ * Need a form of MO_ATOM_WITHIN16_PAIR that never requires
+ * 16-byte atomicity.
+ */
+ mop = MO_128;
+ if (s->align_mem) {
+ mop |= MO_ALIGN_8;
+ }
+ mop = finalize_memop_pair(s, mop);
tmp = tcg_temp_new_i128();
if (s->be_data == MO_LE) {
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 2/6] elf2dmp: Don't abandon when Prcb is set to 0
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
2023-07-31 14:15 ` [PULL 1/6] target/arm: Fix MemOp for STGP Peter Maydell
@ 2023-07-31 14:15 ` Peter Maydell
2023-07-31 14:15 ` [PULL 3/6] target/arm: Avoid writing to constant TCGv in trans_CSEL() Peter Maydell
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2023-07-31 14:15 UTC (permalink / raw)
To: qemu-devel
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Prcb may be set to 0 for some CPUs if the dump was taken before they
start. The dump may still contain valuable information for started CPUs
so don't abandon conversion in such a case.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
Message-id: 20230611033434.14659-1-akihiko.odaki@daynix.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
contrib/elf2dmp/main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 89f0c69ab0f..6d4d18501a3 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -316,6 +316,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
return 1;
}
+ if (!Prcb) {
+ eprintf("Context for CPU #%d is missing\n", i);
+ continue;
+ }
+
if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext,
&Context, sizeof(Context), 0)) {
eprintf("Failed to read CPU #%d ContextFrame location\n", i);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 3/6] target/arm: Avoid writing to constant TCGv in trans_CSEL()
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
2023-07-31 14:15 ` [PULL 1/6] target/arm: Fix MemOp for STGP Peter Maydell
2023-07-31 14:15 ` [PULL 2/6] elf2dmp: Don't abandon when Prcb is set to 0 Peter Maydell
@ 2023-07-31 14:15 ` Peter Maydell
2023-07-31 14:15 ` [PULL 4/6] target/arm/tcg: Don't build AArch64 decodetree files for qemu-system-arm Peter Maydell
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2023-07-31 14:15 UTC (permalink / raw)
To: qemu-devel
In commit 0b188ea05acb5 we changed the implementation of
trans_CSEL() to use tcg_constant_i32(). However, this change
was incorrect, because the implementation of the function
sets up the TCGv_i32 rn and rm to be either zero or else
a TCG temp created in load_reg(), and these TCG temps are
then in both cases written to by the emitted TCG ops.
The result is that we hit a TCG assertion:
qemu-system-arm: ../../tcg/tcg.c:4455: tcg_reg_alloc_mov: Assertion `!temp_readonly(ots)' failed.
(or on a non-debug build, just produce a garbage result)
Adjust the code so that rn and rm are always writeable
temporaries whether the instruction is using the special
case "0" or a normal register as input.
Cc: qemu-stable@nongnu.org
Fixes: 0b188ea05acb5 ("target/arm: Use tcg_constant in trans_CSEL")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20230727103906.2641264-1-peter.maydell@linaro.org
---
target/arm/tcg/translate.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 13c88ba1b9f..b71ac2d0d53 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -8799,7 +8799,7 @@ static bool trans_IT(DisasContext *s, arg_IT *a)
/* v8.1M CSEL/CSINC/CSNEG/CSINV */
static bool trans_CSEL(DisasContext *s, arg_CSEL *a)
{
- TCGv_i32 rn, rm, zero;
+ TCGv_i32 rn, rm;
DisasCompare c;
if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
@@ -8817,16 +8817,17 @@ static bool trans_CSEL(DisasContext *s, arg_CSEL *a)
}
/* In this insn input reg fields of 0b1111 mean "zero", not "PC" */
- zero = tcg_constant_i32(0);
+ rn = tcg_temp_new_i32();
+ rm = tcg_temp_new_i32();
if (a->rn == 15) {
- rn = zero;
+ tcg_gen_movi_i32(rn, 0);
} else {
- rn = load_reg(s, a->rn);
+ load_reg_var(s, rn, a->rn);
}
if (a->rm == 15) {
- rm = zero;
+ tcg_gen_movi_i32(rm, 0);
} else {
- rm = load_reg(s, a->rm);
+ load_reg_var(s, rm, a->rm);
}
switch (a->op) {
@@ -8846,7 +8847,7 @@ static bool trans_CSEL(DisasContext *s, arg_CSEL *a)
}
arm_test_cc(&c, a->fcond);
- tcg_gen_movcond_i32(c.cond, rn, c.value, zero, rn, rm);
+ tcg_gen_movcond_i32(c.cond, rn, c.value, tcg_constant_i32(0), rn, rm);
store_reg(s, a->rd, rn);
return true;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 4/6] target/arm/tcg: Don't build AArch64 decodetree files for qemu-system-arm
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
` (2 preceding siblings ...)
2023-07-31 14:15 ` [PULL 3/6] target/arm: Avoid writing to constant TCGv in trans_CSEL() Peter Maydell
@ 2023-07-31 14:15 ` Peter Maydell
2023-07-31 14:15 ` [PULL 5/6] kvm: Fix crash due to access uninitialized kvm_state Peter Maydell
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2023-07-31 14:15 UTC (permalink / raw)
To: qemu-devel
Currently we list all the Arm decodetree files together and add them
unconditionally to arm_ss. This means we build them for both
qemu-system-aarch64 and qemu-system-arm. However, some of them are
AArch64-specific, so there is no need to build them for
qemu-system-arm. (Meson is smart enough to notice that the generated
.c.inc file is not used by any objects that go into qemu-system-arm,
so we only unnecessarily run decodetree, not anything more
heavyweight like a recompile or relink, but it's still unnecessary
work.)
Split gen into gen_a32 and gen_a64, and only add gen_a64 for
TARGET_AARCH64 compiles.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20230718104628.1137734-1-peter.maydell@linaro.org
---
target/arm/tcg/meson.build | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
index bdcab564899..6fca38f2ccb 100644
--- a/target/arm/tcg/meson.build
+++ b/target/arm/tcg/meson.build
@@ -1,7 +1,11 @@
-gen = [
+gen_a64 = [
+ decodetree.process('a64.decode', extra_args: ['--static-decode=disas_a64']),
decodetree.process('sve.decode', extra_args: '--decode=disas_sve'),
decodetree.process('sme.decode', extra_args: '--decode=disas_sme'),
decodetree.process('sme-fa64.decode', extra_args: '--static-decode=disas_sme_fa64'),
+]
+
+gen_a32 = [
decodetree.process('neon-shared.decode', extra_args: '--decode=disas_neon_shared'),
decodetree.process('neon-dp.decode', extra_args: '--decode=disas_neon_dp'),
decodetree.process('neon-ls.decode', extra_args: '--decode=disas_neon_ls'),
@@ -13,10 +17,10 @@ gen = [
decodetree.process('a32-uncond.decode', extra_args: '--static-decode=disas_a32_uncond'),
decodetree.process('t32.decode', extra_args: '--static-decode=disas_t32'),
decodetree.process('t16.decode', extra_args: ['-w', '16', '--static-decode=disas_t16']),
- decodetree.process('a64.decode', extra_args: ['--static-decode=disas_a64']),
]
-arm_ss.add(gen)
+arm_ss.add(gen_a32)
+arm_ss.add(when: 'TARGET_AARCH64', if_true: gen_a64)
arm_ss.add(files(
'cpu32.c',
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 5/6] kvm: Fix crash due to access uninitialized kvm_state
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
` (3 preceding siblings ...)
2023-07-31 14:15 ` [PULL 4/6] target/arm/tcg: Don't build AArch64 decodetree files for qemu-system-arm Peter Maydell
@ 2023-07-31 14:15 ` Peter Maydell
2023-07-31 14:15 ` [PULL 6/6] gdbstub: Fix client Ctrl-C handling Peter Maydell
2023-07-31 18:00 ` [PULL 0/6] target-arm queue Richard Henderson
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2023-07-31 14:15 UTC (permalink / raw)
To: qemu-devel
From: Gavin Shan <gshan@redhat.com>
Runs into core dump on arm64 and the backtrace extracted from the
core dump is shown as below. It's caused by accessing uninitialized
@kvm_state in kvm_flush_coalesced_mmio_buffer() due to commit 176d073029
("hw/arm/virt: Use machine_memory_devices_init()"), where the machine's
memory region is added earlier than before.
main
qemu_init
configure_accelerators
qemu_opts_foreach
do_configure_accelerator
accel_init_machine
kvm_init
virt_kvm_type
virt_set_memmap
machine_memory_devices_init
memory_region_add_subregion
memory_region_add_subregion_common
memory_region_update_container_subregions
memory_region_transaction_begin
qemu_flush_coalesced_mmio_buffer
kvm_flush_coalesced_mmio_buffer
Fix it by bailing early in kvm_flush_coalesced_mmio_buffer() on the
uninitialized @kvm_state. With this applied, no crash is observed on
arm64.
Fixes: 176d073029 ("hw/arm/virt: Use machine_memory_devices_init()")
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20230731125946.2038742-1-gshan@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
accel/kvm/kvm-all.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 373d876c058..7b3da8dc3ab 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2812,7 +2812,7 @@ void kvm_flush_coalesced_mmio_buffer(void)
{
KVMState *s = kvm_state;
- if (s->coalesced_flush_in_progress) {
+ if (!s || s->coalesced_flush_in_progress) {
return;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 6/6] gdbstub: Fix client Ctrl-C handling
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
` (4 preceding siblings ...)
2023-07-31 14:15 ` [PULL 5/6] kvm: Fix crash due to access uninitialized kvm_state Peter Maydell
@ 2023-07-31 14:15 ` Peter Maydell
2023-07-31 18:00 ` [PULL 0/6] target-arm queue Richard Henderson
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2023-07-31 14:15 UTC (permalink / raw)
To: qemu-devel
From: Nicholas Piggin <npiggin@gmail.com>
The gdb remote protocol has a special interrupt character (0x03) that is
transmitted outside the regular packet processing, and represents a
Ctrl-C pressed in the client. Despite not being a regular packet, it
does expect a regular stop response if the stub successfully stops the
running program.
See: https://sourceware.org/gdb/onlinedocs/gdb/Interrupts.html
Inhibiting the stop reply packet can lead to gdb client hang. So permit
a stop response when receiving a character from gdb that stops the vm.
Additionally, add a warning if that was not a 0x03 character, because
the gdb session is likely to end up getting confused if this happens.
Cc: qemu-stable@nongnu.org
Fixes: 758370052fb ("gdbstub: only send stop-reply packets when allowed to")
Reported-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Tested-by: Joel Stanley <joel@jms.id.au>
Message-id: 20230711085903.304496-1-npiggin@gmail.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
gdbstub/gdbstub.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 6911b73c079..ce8b42eb159 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -2051,8 +2051,17 @@ void gdb_read_byte(uint8_t ch)
return;
}
if (runstate_is_running()) {
- /* when the CPU is running, we cannot do anything except stop
- it when receiving a char */
+ /*
+ * When the CPU is running, we cannot do anything except stop
+ * it when receiving a char. This is expected on a Ctrl-C in the
+ * gdb client. Because we are in all-stop mode, gdb sends a
+ * 0x03 byte which is not a usual packet, so we handle it specially
+ * here, but it does expect a stop reply.
+ */
+ if (ch != 0x03) {
+ warn_report("gdbstub: client sent packet while target running\n");
+ }
+ gdbserver_state.allow_stop_reply = true;
vm_stop(RUN_STATE_PAUSED);
} else
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PULL 0/6] target-arm queue
2023-07-31 14:15 [PULL 0/6] target-arm queue Peter Maydell
` (5 preceding siblings ...)
2023-07-31 14:15 ` [PULL 6/6] gdbstub: Fix client Ctrl-C handling Peter Maydell
@ 2023-07-31 18:00 ` Richard Henderson
6 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2023-07-31 18:00 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
On 7/31/23 07:15, Peter Maydell wrote:
> Hi; here's a target-arm pull for rc2. Four arm-related fixes,
> and a couple of bug fixes for other areas of the codebase
> that seemed like they'd fallen through the cracks.
>
> thanks
> -- PMM
>
> The following changes since commit ccb86f079a9e4d94918086a9df18c1844347aff8:
>
> Merge tag 'pull-nbd-2023-07-28' ofhttps://repo.or.cz/qemu/ericb into staging (2023-07-28 09:56:57 -0700)
>
> are available in the Git repository at:
>
> https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230731
>
> for you to fetch changes up to 108e8180c6b0c315711aa54e914030a313505c17:
>
> gdbstub: Fix client Ctrl-C handling (2023-07-31 14:57:32 +0100)
>
> ----------------------------------------------------------------
> target-arm queue:
> * Don't build AArch64 decodetree files for qemu-system-arm
> * Fix TCG assert in v8.1M CSEL etc
> * Fix MemOp for STGP
> * gdbstub: Fix client Ctrl-C handling
> * kvm: Fix crash due to access uninitialized kvm_state
> * elf2dmp: Don't abandon when Prcb is set to 0
Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread