1 | Some small arm bug fixes for rc3. | 1 | The following changes since commit e3debd5e7d0ce031356024878a0a18b9d109354a: |
---|---|---|---|
2 | 2 | ||
3 | -- PMM | 3 | Merge tag 'pull-request-2023-03-24' of https://gitlab.com/thuth/qemu into staging (2023-03-24 16:08:46 +0000) |
4 | |||
5 | The following changes since commit 9b617b1bb4056e60b39be4c33be20c10928a6a5c: | ||
6 | |||
7 | Merge tag 'trivial-branch-for-7.0-pull-request' of https://gitlab.com/laurent_vivier/qemu into staging (2022-04-01 10:23:27 +0100) | ||
8 | 4 | ||
9 | are available in the Git repository at: | 5 | are available in the Git repository at: |
10 | 6 | ||
11 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220401 | 7 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230328 |
12 | 8 | ||
13 | for you to fetch changes up to a5b1e1ab662aa6dc42d5a913080fccbb8bf82e9b: | 9 | for you to fetch changes up to 46e3b237c52e0c48bfd81bce020b51fbe300b23a: |
14 | 10 | ||
15 | target/arm: Don't use DISAS_NORETURN in STXP !HAVE_CMPXCHG128 codegen (2022-04-01 15:35:49 +0100) | 11 | target/arm/gdbstub: Only advertise M-profile features if TCG available (2023-03-28 10:53:40 +0100) |
16 | 12 | ||
17 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
18 | target-arm queue: | 14 | target-arm queue: |
19 | * target/arm: Fix some bugs in secure EL2 handling | 15 | * fix part of the "TCG-disabled builds are broken" issue |
20 | * target/arm: Fix assert when !HAVE_CMPXCHG128 | ||
21 | * MAINTAINERS: change Fred Konrad's email address | ||
22 | 16 | ||
23 | ---------------------------------------------------------------- | 17 | ---------------------------------------------------------------- |
24 | Frederic Konrad (1): | 18 | Philippe Mathieu-Daudé (1): |
25 | MAINTAINERS: change Fred Konrad's email address | 19 | target/arm/gdbstub: Only advertise M-profile features if TCG available |
26 | 20 | ||
27 | Idan Horowitz (4): | 21 | target/arm/gdbstub.c | 5 +++-- |
28 | target/arm: Fix MTE access checks for disabled SEL2 | 22 | 1 file changed, 3 insertions(+), 2 deletions(-) |
29 | target/arm: Check VSTCR.SW when assigning the stage 2 output PA space | ||
30 | target/arm: Take VSTCR.SW, VTCR.NSW into account in final stage 2 walk | ||
31 | target/arm: Determine final stage 2 output PA space based on original IPA | ||
32 | 23 | ||
33 | Peter Maydell (1): | ||
34 | target/arm: Don't use DISAS_NORETURN in STXP !HAVE_CMPXCHG128 codegen | ||
35 | |||
36 | target/arm/internals.h | 2 +- | ||
37 | target/arm/helper.c | 18 +++++++++++++++--- | ||
38 | target/arm/translate-a64.c | 7 ++++++- | ||
39 | .mailmap | 3 ++- | ||
40 | MAINTAINERS | 2 +- | ||
41 | 5 files changed, 25 insertions(+), 7 deletions(-) | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Idan Horowitz <idan.horowitz@gmail.com> | ||
2 | 1 | ||
3 | While not mentioned anywhere in the actual specification text, the | ||
4 | HCR_EL2.ATA bit is treated as '1' when EL2 is disabled at the current | ||
5 | security state. This can be observed in the psuedo-code implementation | ||
6 | of AArch64.AllocationTagAccessIsEnabled(). | ||
7 | |||
8 | Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com> | ||
9 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
10 | Message-id: 20220328173107.311267-1-idan.horowitz@gmail.com | ||
11 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
12 | --- | ||
13 | target/arm/internals.h | 2 +- | ||
14 | target/arm/helper.c | 2 +- | ||
15 | 2 files changed, 2 insertions(+), 2 deletions(-) | ||
16 | |||
17 | diff --git a/target/arm/internals.h b/target/arm/internals.h | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/target/arm/internals.h | ||
20 | +++ b/target/arm/internals.h | ||
21 | @@ -XXX,XX +XXX,XX @@ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el, | ||
22 | && !(env->cp15.scr_el3 & SCR_ATA)) { | ||
23 | return false; | ||
24 | } | ||
25 | - if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) { | ||
26 | + if (el < 2 && arm_is_el2_enabled(env)) { | ||
27 | uint64_t hcr = arm_hcr_el2_eff(env); | ||
28 | if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) { | ||
29 | return false; | ||
30 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
31 | index XXXXXXX..XXXXXXX 100644 | ||
32 | --- a/target/arm/helper.c | ||
33 | +++ b/target/arm/helper.c | ||
34 | @@ -XXX,XX +XXX,XX @@ static CPAccessResult access_mte(CPUARMState *env, const ARMCPRegInfo *ri, | ||
35 | { | ||
36 | int el = arm_current_el(env); | ||
37 | |||
38 | - if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) { | ||
39 | + if (el < 2 && arm_is_el2_enabled(env)) { | ||
40 | uint64_t hcr = arm_hcr_el2_eff(env); | ||
41 | if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) { | ||
42 | return CP_ACCESS_TRAP_EL2; | ||
43 | -- | ||
44 | 2.25.1 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Idan Horowitz <idan.horowitz@gmail.com> | ||
2 | 1 | ||
3 | As per the AArch64.SS2OutputPASpace() psuedo-code in the ARMv8 ARM when the | ||
4 | PA space of the IPA is non secure, the output PA space is secure if and only | ||
5 | if all of the bits VTCR.<NSW, NSA>, VSTCR.<SW, SA> are not set. | ||
6 | |||
7 | Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com> | ||
8 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
9 | Message-id: 20220327093427.1548629-2-idan.horowitz@gmail.com | ||
10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
11 | --- | ||
12 | target/arm/helper.c | 2 +- | ||
13 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
14 | |||
15 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/target/arm/helper.c | ||
18 | +++ b/target/arm/helper.c | ||
19 | @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, | ||
20 | } else { | ||
21 | attrs->secure = | ||
22 | !((env->cp15.vtcr_el2.raw_tcr & (VTCR_NSA | VTCR_NSW)) | ||
23 | - || (env->cp15.vstcr_el2.raw_tcr & VSTCR_SA)); | ||
24 | + || (env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW))); | ||
25 | } | ||
26 | } | ||
27 | return 0; | ||
28 | -- | ||
29 | 2.25.1 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Idan Horowitz <idan.horowitz@gmail.com> | ||
2 | 1 | ||
3 | As per the AArch64.SS2InitialTTWState() psuedo-code in the ARMv8 ARM the | ||
4 | initial PA space used for stage 2 table walks is assigned based on the SW | ||
5 | and NSW bits of the VSTCR and VTCR registers. | ||
6 | This was already implemented for the recursive stage 2 page table walks | ||
7 | in S1_ptw_translate(), but was missing for the final stage 2 walk. | ||
8 | |||
9 | Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com> | ||
10 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
11 | Message-id: 20220327093427.1548629-3-idan.horowitz@gmail.com | ||
12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
13 | --- | ||
14 | target/arm/helper.c | 10 ++++++++++ | ||
15 | 1 file changed, 10 insertions(+) | ||
16 | |||
17 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/target/arm/helper.c | ||
20 | +++ b/target/arm/helper.c | ||
21 | @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, | ||
22 | return ret; | ||
23 | } | ||
24 | |||
25 | + if (arm_is_secure_below_el3(env)) { | ||
26 | + if (attrs->secure) { | ||
27 | + attrs->secure = !(env->cp15.vstcr_el2.raw_tcr & VSTCR_SW); | ||
28 | + } else { | ||
29 | + attrs->secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW); | ||
30 | + } | ||
31 | + } else { | ||
32 | + assert(!attrs->secure); | ||
33 | + } | ||
34 | + | ||
35 | s2_mmu_idx = attrs->secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; | ||
36 | is_el0 = mmu_idx == ARMMMUIdx_E10_0 || mmu_idx == ARMMMUIdx_SE10_0; | ||
37 | |||
38 | -- | ||
39 | 2.25.1 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Idan Horowitz <idan.horowitz@gmail.com> | ||
2 | 1 | ||
3 | As per the AArch64.S2Walk() pseudo-code in the ARMv8 ARM, the final | ||
4 | decision as to the output address's PA space based on the SA/SW/NSA/NSW | ||
5 | bits needs to take the input IPA's PA space into account, and not the | ||
6 | PA space of the result of the stage 2 walk itself. | ||
7 | |||
8 | Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com> | ||
9 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
10 | Message-id: 20220327093427.1548629-4-idan.horowitz@gmail.com | ||
11 | [PMM: fixed commit message typo] | ||
12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
13 | --- | ||
14 | target/arm/helper.c | 8 +++++--- | ||
15 | 1 file changed, 5 insertions(+), 3 deletions(-) | ||
16 | |||
17 | diff --git a/target/arm/helper.c b/target/arm/helper.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/target/arm/helper.c | ||
20 | +++ b/target/arm/helper.c | ||
21 | @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, | ||
22 | hwaddr ipa; | ||
23 | int s2_prot; | ||
24 | int ret; | ||
25 | + bool ipa_secure; | ||
26 | ARMCacheAttrs cacheattrs2 = {}; | ||
27 | ARMMMUIdx s2_mmu_idx; | ||
28 | bool is_el0; | ||
29 | @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, | ||
30 | return ret; | ||
31 | } | ||
32 | |||
33 | + ipa_secure = attrs->secure; | ||
34 | if (arm_is_secure_below_el3(env)) { | ||
35 | - if (attrs->secure) { | ||
36 | + if (ipa_secure) { | ||
37 | attrs->secure = !(env->cp15.vstcr_el2.raw_tcr & VSTCR_SW); | ||
38 | } else { | ||
39 | attrs->secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW); | ||
40 | } | ||
41 | } else { | ||
42 | - assert(!attrs->secure); | ||
43 | + assert(!ipa_secure); | ||
44 | } | ||
45 | |||
46 | s2_mmu_idx = attrs->secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; | ||
47 | @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, | ||
48 | |||
49 | /* Check if IPA translates to secure or non-secure PA space. */ | ||
50 | if (arm_is_secure_below_el3(env)) { | ||
51 | - if (attrs->secure) { | ||
52 | + if (ipa_secure) { | ||
53 | attrs->secure = | ||
54 | !(env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW)); | ||
55 | } else { | ||
56 | -- | ||
57 | 2.25.1 | diff view generated by jsdifflib |
1 | From: Frederic Konrad <konrad@adacore.com> | 1 | From: Philippe Mathieu-Daudé <philmd@linaro.org> |
---|---|---|---|
2 | 2 | ||
3 | frederic.konrad@adacore.com and konrad@adacore.com will stop working starting | 3 | Cortex-M profile is only emulable from TCG accelerator. Restrict |
4 | 2022-04-01. | 4 | the GDBstub features to its availability in order to avoid a link |
5 | error when TCG is not enabled: | ||
5 | 6 | ||
6 | Use my personal email instead. | 7 | Undefined symbols for architecture arm64: |
8 | "_arm_v7m_get_sp_ptr", referenced from: | ||
9 | _m_sysreg_get in target_arm_gdbstub.c.o | ||
10 | "_arm_v7m_mrs_control", referenced from: | ||
11 | _arm_gdb_get_m_systemreg in target_arm_gdbstub.c.o | ||
12 | ld: symbol(s) not found for architecture arm64 | ||
13 | clang: error: linker command failed with exit code 1 (use -v to see invocation) | ||
7 | 14 | ||
8 | Signed-off-by: Frederic Konrad <frederic.konrad@adacore.com> | 15 | Fixes: 7d8b28b8b5 ("target/arm: Implement gdbstub m-profile systemreg and secext") |
9 | Reviewed-by: Fabien Chouteau <chouteau@adacore.com <clg@kaod.org>> | 16 | Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
10 | Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | 17 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
11 | Message-id: 1648643217-15811-1-git-send-email-frederic.konrad@adacore.com | 18 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> |
19 | Message-id: 20230322142902.69511-3-philmd@linaro.org | ||
20 | [PMM: add #include since I cherry-picked this patch from the series] | ||
12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 21 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
13 | --- | 22 | --- |
14 | .mailmap | 3 ++- | 23 | target/arm/gdbstub.c | 5 +++-- |
15 | MAINTAINERS | 2 +- | 24 | 1 file changed, 3 insertions(+), 2 deletions(-) |
16 | 2 files changed, 3 insertions(+), 2 deletions(-) | ||
17 | 25 | ||
18 | diff --git a/.mailmap b/.mailmap | 26 | diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c |
19 | index XXXXXXX..XXXXXXX 100644 | 27 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/.mailmap | 28 | --- a/target/arm/gdbstub.c |
21 | +++ b/.mailmap | 29 | +++ b/target/arm/gdbstub.c |
22 | @@ -XXX,XX +XXX,XX @@ Alexander Graf <agraf@csgraf.de> <agraf@suse.de> | 30 | @@ -XXX,XX +XXX,XX @@ |
23 | Anthony Liguori <anthony@codemonkey.ws> Anthony Liguori <aliguori@us.ibm.com> | 31 | #include "cpu.h" |
24 | Christian Borntraeger <borntraeger@linux.ibm.com> <borntraeger@de.ibm.com> | 32 | #include "exec/gdbstub.h" |
25 | Filip Bozuta <filip.bozuta@syrmia.com> <filip.bozuta@rt-rk.com.com> | 33 | #include "gdbstub/helpers.h" |
26 | -Frederic Konrad <konrad@adacore.com> <fred.konrad@greensocs.com> | 34 | +#include "sysemu/tcg.h" |
27 | +Frederic Konrad <konrad.frederic@yahoo.fr> <fred.konrad@greensocs.com> | 35 | #include "internals.h" |
28 | +Frederic Konrad <konrad.frederic@yahoo.fr> <konrad@adacore.com> | 36 | #include "cpregs.h" |
29 | Greg Kurz <groug@kaod.org> <gkurz@linux.vnet.ibm.com> | 37 | |
30 | Huacai Chen <chenhuacai@kernel.org> <chenhc@lemote.com> | 38 | @@ -XXX,XX +XXX,XX @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu) |
31 | Huacai Chen <chenhuacai@kernel.org> <chenhuacai@loongson.cn> | 39 | 2, "arm-vfp-sysregs.xml", 0); |
32 | diff --git a/MAINTAINERS b/MAINTAINERS | 40 | } |
33 | index XXXXXXX..XXXXXXX 100644 | 41 | } |
34 | --- a/MAINTAINERS | 42 | - if (cpu_isar_feature(aa32_mve, cpu)) { |
35 | +++ b/MAINTAINERS | 43 | + if (cpu_isar_feature(aa32_mve, cpu) && tcg_enabled()) { |
36 | @@ -XXX,XX +XXX,XX @@ F: include/hw/rtc/sun4v-rtc.h | 44 | gdb_register_coprocessor(cs, mve_gdb_get_reg, mve_gdb_set_reg, |
37 | 45 | 1, "arm-m-profile-mve.xml", 0); | |
38 | Leon3 | 46 | } |
39 | M: Fabien Chouteau <chouteau@adacore.com> | 47 | @@ -XXX,XX +XXX,XX @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu) |
40 | -M: KONRAD Frederic <frederic.konrad@adacore.com> | 48 | arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs), |
41 | +M: Frederic Konrad <konrad.frederic@yahoo.fr> | 49 | "system-registers.xml", 0); |
42 | S: Maintained | 50 | |
43 | F: hw/sparc/leon3.c | 51 | - if (arm_feature(env, ARM_FEATURE_M)) { |
44 | F: hw/*/grlib* | 52 | + if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) { |
53 | gdb_register_coprocessor(cs, | ||
54 | arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg, | ||
55 | arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs), | ||
45 | -- | 56 | -- |
46 | 2.25.1 | 57 | 2.34.1 |
47 | 58 | ||
48 | 59 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | In gen_store_exclusive(), if the host does not have a cmpxchg128 | ||
2 | primitive then we generate bad code for STXP for storing two 64-bit | ||
3 | values. We generate a call to the exit_atomic helper, which never | ||
4 | returns, and set is_jmp to DISAS_NORETURN. However, this is | ||
5 | forgetting that we have already emitted a brcond that jumps over this | ||
6 | call for the case where we don't hold the exclusive. The effect is | ||
7 | that we don't generate any code to end the TB for the | ||
8 | exclusive-not-held execution path, which falls into the "exit with | ||
9 | TB_EXIT_REQUESTED" code that gen_tb_end() emits. This then causes an | ||
10 | assert at runtime when cpu_loop_exec_tb() sees an EXIT_REQUESTED TB | ||
11 | return that wasn't for an interrupt or icount. | ||
12 | 1 | ||
13 | In particular, you can hit this case when using the clang sanitizers | ||
14 | and trying to run the xlnx-versal-virt acceptance test in 'make | ||
15 | check-acceptance'. This bug was masked until commit 848126d11e93ff | ||
16 | ("meson: move int128 checks from configure") because we used to set | ||
17 | CONFIG_CMPXCHG128=1 and avoid the buggy codepath, but after that we | ||
18 | do not. | ||
19 | |||
20 | Fix the bug by not setting is_jmp. The code after the exit_atomic | ||
21 | call up to the fail_label is dead, but TCG is smart enough to | ||
22 | eliminate it. We do need to set 'tmp' to some valid value, though | ||
23 | (in the same way the exit_atomic-using code in tcg/tcg-op.c does). | ||
24 | |||
25 | Resolves: https://gitlab.com/qemu-project/qemu/-/issues/953 | ||
26 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
27 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
28 | Message-id: 20220331150858.96348-1-peter.maydell@linaro.org | ||
29 | --- | ||
30 | target/arm/translate-a64.c | 7 ++++++- | ||
31 | 1 file changed, 6 insertions(+), 1 deletion(-) | ||
32 | |||
33 | diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c | ||
34 | index XXXXXXX..XXXXXXX 100644 | ||
35 | --- a/target/arm/translate-a64.c | ||
36 | +++ b/target/arm/translate-a64.c | ||
37 | @@ -XXX,XX +XXX,XX @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2, | ||
38 | } else if (tb_cflags(s->base.tb) & CF_PARALLEL) { | ||
39 | if (!HAVE_CMPXCHG128) { | ||
40 | gen_helper_exit_atomic(cpu_env); | ||
41 | - s->base.is_jmp = DISAS_NORETURN; | ||
42 | + /* | ||
43 | + * Produce a result so we have a well-formed opcode | ||
44 | + * stream when the following (dead) code uses 'tmp'. | ||
45 | + * TCG will remove the dead ops for us. | ||
46 | + */ | ||
47 | + tcg_gen_movi_i64(tmp, 0); | ||
48 | } else if (s->be_data == MO_LE) { | ||
49 | gen_helper_paired_cmpxchg64_le_parallel(tmp, cpu_env, | ||
50 | cpu_exclusive_addr, | ||
51 | -- | ||
52 | 2.25.1 | diff view generated by jsdifflib |