* [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions @ 2019-08-16 12:58 Peter Maydell 2019-08-16 12:58 ` [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions Peter Maydell ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Peter Maydell @ 2019-08-16 12:58 UTC (permalink / raw) To: qemu-arm, qemu-devel The translation table walk for an ATS instruction can result in various faults. In general these are just reported back via the PAR_EL1 fault status fields, but in some cases the architecture requires that the fault is turned into an exception: * synchronous stage 2 faults of any kind during AT S1E0* and AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 * synchronous external aborts are taken as Data Abort exceptions (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and G5.13.4.) I noticed this by code inspection back last year sometime when I was investigating a guest boot failure that turned out to be due to an entirely different cause. I got about halfway through trying to code up a fix before I realised it was irrelevant to that bug. This patchset is just tidying up and completing that work so it doesn't get lost. Use of ATS insns in the cases where they might actually fault is quite rare (obviously nobody sets up page tables where there's no memory and they'll take external aborts, and even for the "take a hyp trap for a stage 2 fault" case you need a setup with a hypervisor and a guest that uses ATS insns, and Linux as a guest doesn't use ATS at all. So my testing of this patchset has been more "check it doesn't break things" rather than actively finding and testing a use of the throw-an-exception path... thanks -- PMM Peter Maydell (2): target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions target/arm: Take exceptions on ATS instructions when needed target/arm/cpu.h | 6 ++- target/arm/helper.c | 107 +++++++++++++++++++++++++++++++------ target/arm/translate-a64.c | 6 +++ target/arm/translate.c | 7 +++ 4 files changed, 110 insertions(+), 16 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions 2019-08-16 12:58 [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions Peter Maydell @ 2019-08-16 12:58 ` Peter Maydell 2019-08-18 6:12 ` Richard Henderson 2019-08-16 12:58 ` [Qemu-devel] [PATCH 2/2] target/arm: Take exceptions on ATS instructions when needed Peter Maydell ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Peter Maydell @ 2019-08-16 12:58 UTC (permalink / raw) To: qemu-arm, qemu-devel Currently the only part of an ARMCPRegInfo which is allowed to cause a CPU exception is the access function, which returns a value indicating that some flavour of UNDEF should be generated. For the ATS system instructions, we would like to conditionally generate exceptions as part of the writefn, because some faults during the page table walk (like external aborts) should cause an exception to be raised rather than returning a value. There are several ways we could do this: * plumb the GETPC() value from the top level set_cp_reg/get_cp_reg helper functions through into the readfn and writefn hooks * add extra readfn_with_ra/writefn_with_ra hooks that take the GETPC() value * require the ATS instructions to provide a dummy accessfn, which serves no purpose except to cause the code generation to emit TCG ops to sync the CPU state * add an ARM_CP_ flag to mark the ARMCPRegInfo as possibly throwing an exception in its read/write hooks, and make the codegen sync the CPU state before calling the hooks if the flag is set This patch opts for the last of these, as it is fairly simple to implement and doesn't require invasive changes like updating the readfn/writefn hook function prototype signature. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 6 +++++- target/arm/translate-a64.c | 6 ++++++ target/arm/translate.c | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 94c990cddbd..021b552334b 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -2206,6 +2206,9 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * IO indicates that this register does I/O and therefore its accesses * need to be surrounded by gen_io_start()/gen_io_end(). In particular, * registers which implement clocks or timers require this. + * RAISES_EXC is for when the read or write hook might raise an exception; + * the generated code will synchronize the CPU state before calling the hook + * so that it is safe for the hook to call raise_exception(). */ #define ARM_CP_SPECIAL 0x0001 #define ARM_CP_CONST 0x0002 @@ -2224,10 +2227,11 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) #define ARM_CP_FPU 0x1000 #define ARM_CP_SVE 0x2000 #define ARM_CP_NO_GDB 0x4000 +#define ARM_CP_RAISES_EXC 0x8000 /* Used only as a terminator for ARMCPRegInfo lists */ #define ARM_CP_SENTINEL 0xffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0x70ff +#define ARM_CP_FLAG_MASK 0xf0ff /* Valid values for ARMCPRegInfo state field, indicating which of * the AArch32 and AArch64 execution states this register is visible in. diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index d3231477a27..908a186bfec 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -1729,6 +1729,12 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, tcg_temp_free_ptr(tmpptr); tcg_temp_free_i32(tcg_syn); tcg_temp_free_i32(tcg_isread); + } else if (ri->type & ARM_CP_RAISES_EXC) { + /* + * The readfn or writefn might raise an exception; + * synchronize the CPU state in case it does. + */ + gen_a64_set_pc_im(s->pc - 4); } /* Handle special cases first */ diff --git a/target/arm/translate.c b/target/arm/translate.c index 7853462b21b..da38e15be8d 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -7228,6 +7228,13 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) tcg_temp_free_ptr(tmpptr); tcg_temp_free_i32(tcg_syn); tcg_temp_free_i32(tcg_isread); + } else if (ri->type & ARM_CP_RAISES_EXC) { + /* + * The readfn or writefn might raise an exception; + * synchronize the CPU state in case it does. + */ + gen_set_condexec(s); + gen_set_pc_im(s, s->pc - 4); } /* Handle special cases first */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions 2019-08-16 12:58 ` [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions Peter Maydell @ 2019-08-18 6:12 ` Richard Henderson 2019-08-27 14:47 ` Peter Maydell 0 siblings, 1 reply; 10+ messages in thread From: Richard Henderson @ 2019-08-18 6:12 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel On 8/16/19 1:58 PM, Peter Maydell wrote: > @@ -1729,6 +1729,12 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, > tcg_temp_free_ptr(tmpptr); > tcg_temp_free_i32(tcg_syn); > tcg_temp_free_i32(tcg_isread); > + } else if (ri->type & ARM_CP_RAISES_EXC) { > + /* > + * The readfn or writefn might raise an exception; > + * synchronize the CPU state in case it does. > + */ > + gen_a64_set_pc_im(s->pc - 4); This will now need an update for master, but otherwise, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions 2019-08-18 6:12 ` Richard Henderson @ 2019-08-27 14:47 ` Peter Maydell 0 siblings, 0 replies; 10+ messages in thread From: Peter Maydell @ 2019-08-27 14:47 UTC (permalink / raw) To: Richard Henderson; +Cc: qemu-arm, QEMU Developers On Sun, 18 Aug 2019 at 07:12, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/16/19 1:58 PM, Peter Maydell wrote: > > @@ -1729,6 +1729,12 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, > > tcg_temp_free_ptr(tmpptr); > > tcg_temp_free_i32(tcg_syn); > > tcg_temp_free_i32(tcg_isread); > > + } else if (ri->type & ARM_CP_RAISES_EXC) { > > + /* > > + * The readfn or writefn might raise an exception; > > + * synchronize the CPU state in case it does. > > + */ > > + gen_a64_set_pc_im(s->pc - 4); > > This will now need an update for master, but otherwise, > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Thanks; applied series to target-arm.next with this squashed in to handle the changes in master: diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index b3053d3fb89..4d09ae6f424 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -1719,7 +1719,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread, * The readfn or writefn might raise an exception; * synchronize the CPU state in case it does. */ - gen_a64_set_pc_im(s->pc - 4); + gen_a64_set_pc_im(s->pc_curr); } /* Handle special cases first */ diff --git a/target/arm/translate.c b/target/arm/translate.c index adb97dc6a3d..78d93f63cab 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -7197,7 +7197,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) * synchronize the CPU state in case it does. */ gen_set_condexec(s); - gen_set_pc_im(s, s->pc - 4); + gen_set_pc_im(s, s->pc_curr); } /* Handle special cases first */ -- PMM ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/2] target/arm: Take exceptions on ATS instructions when needed 2019-08-16 12:58 [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions Peter Maydell 2019-08-16 12:58 ` [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions Peter Maydell @ 2019-08-16 12:58 ` Peter Maydell 2019-08-18 6:23 ` Richard Henderson 2019-08-16 18:13 ` [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions no-reply 2019-08-19 12:44 ` Peter Maydell 3 siblings, 1 reply; 10+ messages in thread From: Peter Maydell @ 2019-08-16 12:58 UTC (permalink / raw) To: qemu-arm, qemu-devel The translation table walk for an ATS instruction can result in various faults. In general these are just reported back via the PAR_EL1 fault status fields, but in some cases the architecture requires that the fault is turned into an exception: * synchronous stage 2 faults of any kind during AT S1E0* and AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 * synchronous external aborts are taken as Data Abort exceptions (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and G5.13.4.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 107 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 92 insertions(+), 15 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index b74c23a9bc0..7d82195c960 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -2944,6 +2944,73 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, ret = get_phys_addr(env, value, access_type, mmu_idx, &phys_addr, &attrs, &prot, &page_size, &fi, &cacheattrs); + if (ret) { + /* + * Some kinds of translation fault must cause exceptions rather + * than being reported in the PAR. + */ + int current_el = arm_current_el(env); + int target_el; + uint32_t syn, fsr, fsc; + bool take_exc = false; + + if (fi.s1ptw && current_el == 1 && !arm_is_secure(env) + && (mmu_idx == ARMMMUIdx_S1NSE1 || mmu_idx == ARMMMUIdx_S1NSE0)) { + /* + * Synchronous stage 2 fault on an access made as part of the + * translation table walk for AT S1E0* or AT S1E1* insn + * executed from NS EL1. If this is a synchronous external abort + * and SCR_EL3.EA == 1, then we take a synchronous external abort + * to EL3. Otherwise the fault is taken as an exception to EL2, + * and HPFAR_EL2 holds the faulting IPA. + */ + if (fi.type == ARMFault_SyncExternalOnWalk && + (env->cp15.scr_el3 & SCR_EA)) { + target_el = 3; + } else { + env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4; + target_el = 2; + } + take_exc = true; + } else if (fi.type == ARMFault_SyncExternalOnWalk) { + /* + * Synchronous external aborts during a translation table walk + * are taken as Data Abort exceptions. + */ + if (fi.stage2) { + if (current_el == 3) { + target_el = 3; + } else { + target_el = 2; + } + } else { + target_el = exception_target_el(env); + } + take_exc = true; + } + + if (take_exc) { + /* Construct FSR and FSC using same logic as arm_deliver_fault() */ + if (target_el == 2 || arm_el_is_aa64(env, target_el) || + arm_s1_regime_using_lpae_format(env, mmu_idx)) { + fsr = arm_fi_to_lfsc(&fi); + fsc = extract32(fsr, 0, 6); + } else { + fsr = arm_fi_to_sfsc(&fi); + fsc = 0x3f; + } + /* + * Report exception with ESR indicating a fault due to a + * translation table walk for a cache maintenance instruction. + */ + syn = syn_data_abort_no_iss(current_el == target_el, + fi.ea, 1, fi.s1ptw, 1, fsc); + env->exception.vaddress = value; + env->exception.fsr = fsr; + raise_exception(env, EXCP_DATA_ABORT, syn, target_el); + } + } + if (is_a64(env)) { format64 = true; } else if (arm_feature(env, ARM_FEATURE_LPAE)) { @@ -3148,7 +3215,7 @@ static const ARMCPRegInfo vapa_cp_reginfo[] = { /* This underdecoding is safe because the reginfo is NO_RAW. */ { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY, .access = PL1_W, .accessfn = ats_access, - .writefn = ats_write, .type = ARM_CP_NO_RAW }, + .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, #endif REGINFO_SENTINEL }; @@ -4281,35 +4348,45 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { /* 64 bit address translation operations */ { .name = "AT_S1E1R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 0, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E1W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 1, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E0R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 2, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E0W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E1R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 4, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E1W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 5, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E0R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 6, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E0W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 7, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, /* AT S1E2* are elsewhere as they UNDEF from EL3 if EL2 is not present */ { .name = "AT_S1E3R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 0, - .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E3W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 1, - .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "PAR_EL1", .state = ARM_CP_STATE_AA64, .type = ARM_CP_ALIAS, .opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0, @@ -4891,11 +4968,11 @@ static const ARMCPRegInfo el2_cp_reginfo[] = { { .name = "AT_S1E2R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 0, .access = PL2_W, .accessfn = at_s1e2_access, - .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 }, { .name = "AT_S1E2W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 1, .access = PL2_W, .accessfn = at_s1e2_access, - .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 }, /* The AArch32 ATS1H* operations are CONSTRAINED UNPREDICTABLE * if EL2 is not implemented; we choose to UNDEF. Behaviour at EL3 * with SCR.NS == 0 outside Monitor mode is UNPREDICTABLE; we choose @@ -4903,10 +4980,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = { */ { .name = "ATS1HR", .cp = 15, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 0, .access = PL2_W, - .writefn = ats1h_write, .type = ARM_CP_NO_RAW }, + .writefn = ats1h_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, { .name = "ATS1HW", .cp = 15, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 1, .access = PL2_W, - .writefn = ats1h_write, .type = ARM_CP_NO_RAW }, + .writefn = ats1h_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0, /* ARMv7 requires bit 0 and 1 to reset to 1. ARMv8 defines the -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] target/arm: Take exceptions on ATS instructions when needed 2019-08-16 12:58 ` [Qemu-devel] [PATCH 2/2] target/arm: Take exceptions on ATS instructions when needed Peter Maydell @ 2019-08-18 6:23 ` Richard Henderson 0 siblings, 0 replies; 10+ messages in thread From: Richard Henderson @ 2019-08-18 6:23 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel On 8/16/19 1:58 PM, Peter Maydell wrote: > The translation table walk for an ATS instruction can result in > various faults. In general these are just reported back via the > PAR_EL1 fault status fields, but in some cases the architecture > requires that the fault is turned into an exception: > * synchronous stage 2 faults of any kind during AT S1E0* and > AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 > * synchronous external aborts are taken as Data Abort exceptions > > (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and > G5.13.4.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/helper.c | 107 +++++++++++++++++++++++++++++++++++++------- > 1 file changed, 92 insertions(+), 15 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions 2019-08-16 12:58 [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions Peter Maydell 2019-08-16 12:58 ` [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions Peter Maydell 2019-08-16 12:58 ` [Qemu-devel] [PATCH 2/2] target/arm: Take exceptions on ATS instructions when needed Peter Maydell @ 2019-08-16 18:13 ` no-reply 2019-08-19 12:44 ` Peter Maydell 3 siblings, 0 replies; 10+ messages in thread From: no-reply @ 2019-08-16 18:13 UTC (permalink / raw) To: peter.maydell; +Cc: qemu-arm, qemu-devel Patchew URL: https://patchew.org/QEMU/20190816125802.25877-1-peter.maydell@linaro.org/ Hi, This series failed build test on s390x host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo echo "=== ENV ===" env echo echo "=== PACKAGES ===" rpm -qa echo echo "=== UNAME ===" uname -a CC=$HOME/bin/cc INSTALL=$PWD/install BUILD=$PWD/build mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable clean up # make check -j4 V=1 make install === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20190816125802.25877-1-peter.maydell@linaro.org/testing.s390x/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions 2019-08-16 12:58 [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions Peter Maydell ` (2 preceding siblings ...) 2019-08-16 18:13 ` [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions no-reply @ 2019-08-19 12:44 ` Peter Maydell 2019-08-19 17:33 ` Edgar E. Iglesias 2019-08-20 12:59 ` Edgar E. Iglesias 3 siblings, 2 replies; 10+ messages in thread From: Peter Maydell @ 2019-08-19 12:44 UTC (permalink / raw) To: qemu-arm, QEMU Developers, Stefano Stabellini, Edgar Iglesias, Anthony PERARD On Fri, 16 Aug 2019 at 13:58, Peter Maydell <peter.maydell@linaro.org> wrote: > > The translation table walk for an ATS instruction can result in > various faults. In general these are just reported back via the > PAR_EL1 fault status fields, but in some cases the architecture > requires that the fault is turned into an exception: > * synchronous stage 2 faults of any kind during AT S1E0* and > AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 > * synchronous external aborts are taken as Data Abort exceptions > > (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and G5.13.4.) > > I noticed this by code inspection back last year sometime when > I was investigating a guest boot failure that turned out to be > due to an entirely different cause. I got about halfway through > trying to code up a fix before I realised it was irrelevant to > that bug. This patchset is just tidying up and completing that > work so it doesn't get lost. > > Use of ATS insns in the cases where they might actually fault > is quite rare (obviously nobody sets up page tables where there's > no memory and they'll take external aborts, and even for the > "take a hyp trap for a stage 2 fault" case you need a setup > with a hypervisor and a guest that uses ATS insns, and Linux as > a guest doesn't use ATS at all. So my testing of this patchset > has been more "check it doesn't break things" rather than > actively finding and testing a use of the throw-an-exception path... I'm told that Xen for Arm makes more active use of ATS instructions, so I've cc'd a few Xen people -- do any of you have handy testing setups to try running Xen in emulation under QEMU? Configs where the guest (EL1) actually uses ATS instructions are the particularly interesting point for this patchset. (if there's a good set of instructions for creating a test image I could probably add it to the ad-hoc set of things I sometimes test with.) > Peter Maydell (2): > target/arm: Allow ARMCPRegInfo read/write functions to throw > exceptions > target/arm: Take exceptions on ATS instructions when needed > > target/arm/cpu.h | 6 ++- > target/arm/helper.c | 107 +++++++++++++++++++++++++++++++------ > target/arm/translate-a64.c | 6 +++ > target/arm/translate.c | 7 +++ > 4 files changed, 110 insertions(+), 16 deletions(-) thanks -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions 2019-08-19 12:44 ` Peter Maydell @ 2019-08-19 17:33 ` Edgar E. Iglesias 2019-08-20 12:59 ` Edgar E. Iglesias 1 sibling, 0 replies; 10+ messages in thread From: Edgar E. Iglesias @ 2019-08-19 17:33 UTC (permalink / raw) To: Peter Maydell Cc: Anthony PERARD, qemu-arm, Stefano Stabellini, QEMU Developers On Mon, Aug 19, 2019 at 01:44:37PM +0100, Peter Maydell wrote: > On Fri, 16 Aug 2019 at 13:58, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > The translation table walk for an ATS instruction can result in > > various faults. In general these are just reported back via the > > PAR_EL1 fault status fields, but in some cases the architecture > > requires that the fault is turned into an exception: > > * synchronous stage 2 faults of any kind during AT S1E0* and > > AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 > > * synchronous external aborts are taken as Data Abort exceptions > > > > (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and G5.13.4.) > > > > I noticed this by code inspection back last year sometime when > > I was investigating a guest boot failure that turned out to be > > due to an entirely different cause. I got about halfway through > > trying to code up a fix before I realised it was irrelevant to > > that bug. This patchset is just tidying up and completing that > > work so it doesn't get lost. > > > > Use of ATS insns in the cases where they might actually fault > > is quite rare (obviously nobody sets up page tables where there's > > no memory and they'll take external aborts, and even for the > > "take a hyp trap for a stage 2 fault" case you need a setup > > with a hypervisor and a guest that uses ATS insns, and Linux as > > a guest doesn't use ATS at all. So my testing of this patchset > > has been more "check it doesn't break things" rather than > > actively finding and testing a use of the throw-an-exception path... > > I'm told that Xen for Arm makes more active use of ATS > instructions, so I've cc'd a few Xen people -- do any > of you have handy testing setups to try running Xen in > emulation under QEMU? Configs where the guest (EL1) actually > uses ATS instructions are the particularly interesting point > for this patchset. > > (if there's a good set of instructions for creating a test > image I could probably add it to the ad-hoc set of things > I sometimes test with.) > Yes, I'm not very up to date with the Xen code but it used to be the case that Xen used ATS a fair bit. We have some other tests that use it but they don't rely on exceptions IIRC. I'll take your patches and test them with our internal testsuites, including Xen images. I may also be able to find a Xen image I can share that works with upstream QEMU. Stefano may have something in his pocket already? Cheers, Edgar ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions 2019-08-19 12:44 ` Peter Maydell 2019-08-19 17:33 ` Edgar E. Iglesias @ 2019-08-20 12:59 ` Edgar E. Iglesias 1 sibling, 0 replies; 10+ messages in thread From: Edgar E. Iglesias @ 2019-08-20 12:59 UTC (permalink / raw) To: Peter Maydell Cc: Anthony PERARD, qemu-arm, Stefano Stabellini, QEMU Developers On Mon, Aug 19, 2019 at 01:44:37PM +0100, Peter Maydell wrote: > On Fri, 16 Aug 2019 at 13:58, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > The translation table walk for an ATS instruction can result in > > various faults. In general these are just reported back via the > > PAR_EL1 fault status fields, but in some cases the architecture > > requires that the fault is turned into an exception: > > * synchronous stage 2 faults of any kind during AT S1E0* and > > AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 > > * synchronous external aborts are taken as Data Abort exceptions > > > > (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and G5.13.4.) > > > > I noticed this by code inspection back last year sometime when > > I was investigating a guest boot failure that turned out to be > > due to an entirely different cause. I got about halfway through > > trying to code up a fix before I realised it was irrelevant to > > that bug. This patchset is just tidying up and completing that > > work so it doesn't get lost. > > > > Use of ATS insns in the cases where they might actually fault > > is quite rare (obviously nobody sets up page tables where there's > > no memory and they'll take external aborts, and even for the > > "take a hyp trap for a stage 2 fault" case you need a setup > > with a hypervisor and a guest that uses ATS insns, and Linux as > > a guest doesn't use ATS at all. So my testing of this patchset > > has been more "check it doesn't break things" rather than > > actively finding and testing a use of the throw-an-exception path... > > I'm told that Xen for Arm makes more active use of ATS > instructions, so I've cc'd a few Xen people -- do any > of you have handy testing setups to try running Xen in > emulation under QEMU? Configs where the guest (EL1) actually > uses ATS instructions are the particularly interesting point > for this patchset. > > (if there's a good set of instructions for creating a test > image I could probably add it to the ad-hoc set of things > I sometimes test with.) Hi, All tests passed. Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Cheers, Edgar ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-08-27 14:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-16 12:58 [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions Peter Maydell 2019-08-16 12:58 ` [Qemu-devel] [PATCH 1/2] target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions Peter Maydell 2019-08-18 6:12 ` Richard Henderson 2019-08-27 14:47 ` Peter Maydell 2019-08-16 12:58 ` [Qemu-devel] [PATCH 2/2] target/arm: Take exceptions on ATS instructions when needed Peter Maydell 2019-08-18 6:23 ` Richard Henderson 2019-08-16 18:13 ` [Qemu-devel] [PATCH 0/2] target/arm: Take exceptions on ATS instructions no-reply 2019-08-19 12:44 ` Peter Maydell 2019-08-19 17:33 ` Edgar E. Iglesias 2019-08-20 12:59 ` Edgar E. Iglesias
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).