[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH RESEND v15 08/10] target-arm: kvm64: inject synchr
From: |
Peter Maydell |
Subject: |
Re: [Qemu-arm] [PATCH RESEND v15 08/10] target-arm: kvm64: inject synchronous External Abort |
Date: |
Tue, 20 Nov 2018 15:07:44 +0000 |
On 8 November 2018 at 10:29, Dongjiu Geng <address@hidden> wrote:
> Add synchronous external abort injection logic, setup
> exception type and syndrome value. When switch to guest,
> guest will jump to the synchronous external abort vector
> table entry.
>
> The ESR_ELx.DFSC is set to synchronous external abort(0x10),
> and ESR_ELx.FnV is set to not valid(0x1), which will tell
> guest that FAR is not valid and holds an UNKNOWN value.
> These value will be set to KVM register structures through
> KVM_SET_ONE_REG IOCTL.
>
> Signed-off-by: Dongjiu Geng <address@hidden>
> ---
> Marc is against that KVM inject the synchronous external abort(SEA) in [1],
> so user space how to inject it. The test result that injection SEA to guest
> by Qemu
> is shown in [2].
>
> [1]: https://lkml.org/lkml/2017/3/2/110
> [2]:
> Taking exception 4 [Data Abort]
> ...from EL0 to EL1
> ...with ESR 0x24/0x92000410
> ...with FAR 0x0
> ...with ELR 0x40cf04
> ...to EL1 PC 0xffffffc000084c00 PSTATE 0x3c5
> after kvm_inject_arm_sea
> Unhandled fault: synchronous external abort (0x92000410) at 0x0000007fa234c12c
> CPU: 0 PID: 536 Comm: devmem Not tainted 4.1.0+ #20
> Hardware name: linux,dummy-virt (DT)
> task: ffffffc019ab2b00 ti: ffffffc008134000 task.ti: ffffffc008134000
> PC is at 0x40cf04
> LR is at 0x40cdec
> pc : [<000000000040cf04>] lr : [<000000000040cdec>] pstate: 60000000
> sp : 0000007ff7b24130
> x29: 0000007ff7b24260 x28: 0000000000000000
> x27: 00000000000000ad x26: 000000000049c000
> x25: 000000000048904b x24: 000000000049c000
> x23: 0000000040600000 x22: 0000007ff7b243a0
> x21: 0000000000000002 x20: 0000000000000000
> x19: 0000000000000020 x18: 0000000000000000
> x17: 000000000049c6d0 x16: 0000007fa22c85c0
> x15: 0000000000005798 x14: 0000007fa2205f1c
> x13: 0000007fa241ccb0 x12: 0000000000000137
> x11: 0000000000000000 x10: 0000000000000000
> x9 : 0000000000000000 x8 : 00000000000000de
> x7 : 0000000000000000 x6 : 0000000000002000
> x5 : 0000000040600000 x4 : 0000000000000003
> x3 : 0000000000000001 x2 : 0000000000000000
> x1 : 0000000000000000 x0 : 0000007fa2418000
> ---
> target/arm/cpu.h | 2 ++
> target/arm/helper.c | 23 +++++++++++++++++++++++
> target/arm/internals.h | 5 +++--
> target/arm/kvm64.c | 39 +++++++++++++++++++++++++++++++++++++++
> target/arm/op_helper.c | 2 +-
> 5 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index b5eff79..502507d 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2331,6 +2331,8 @@ bool write_list_to_cpustate(ARMCPU *cpu);
> */
> bool write_cpustate_to_list(ARMCPU *cpu);
>
> +bool write_part_cpustate_to_list(ARMCPU *cpu, ptrdiff_t fieldoffset);
> +
> #define ARM_CPUID_TI915T 0x54029152
> #define ARM_CPUID_TI925T 0x54029252
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 9630193..df078ff 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -263,6 +263,29 @@ static bool raw_accessors_invalid(const ARMCPRegInfo *ri)
> return true;
> }
>
> +bool write_part_cpustate_to_list(ARMCPU *cpu, ptrdiff_t fieldoffset)
> +{
> + const ARMCPRegInfo *ri;
> + uint32_t regidx, i;
> +
> + for (i = 0; i < cpu->cpreg_array_len; i++) {
> + regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
> + ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
> + if (!ri) {
> + continue;
> + }
> +
> + if (ri->type & ARM_CP_NO_RAW) {
> + continue;
> + }
> + if (ri->fieldoffset == fieldoffset) {
> + cpu->cpreg_values[i] = read_raw_cp_reg(&cpu->env, ri);
> + return true;
> + }
> + }
> + return false;
> +}
What is this about? Nothing else in QEMU needs to mess with the
cpustate synchronization. My first assumption is that you should
not need to do so either.
> +
> bool write_cpustate_to_list(ARMCPU *cpu)
> {
> /* Write the coprocessor state from cpu->env to the (index,value) list.
> */
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 6c2bb2d..04ea074 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -415,13 +415,14 @@ static inline uint32_t syn_insn_abort(int same_el, int
> ea, int s1ptw, int fsc)
> | ARM_EL_IL | (ea << 9) | (s1ptw << 7) | fsc;
> }
>
> -static inline uint32_t syn_data_abort_no_iss(int same_el,
> +static inline uint32_t syn_data_abort_no_iss(int same_el, int fnv,
> int ea, int cm, int s1ptw,
> int wnr, int fsc)
> {
> return (EC_DATAABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
> | ARM_EL_IL
> - | (ea << 9) | (cm << 8) | (s1ptw << 7) | (wnr << 6) | fsc;
> + | (fnv << 10) | (ea << 9) | (cm << 8) | (s1ptw << 7)
> + | (wnr << 6) | fsc;
> }
>
> static inline uint32_t syn_data_abort_with_iss(int same_el,
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 5de8ff0..0ca2b29 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -594,6 +594,45 @@ int kvm_arm_cpreg_level(uint64_t regidx)
> return KVM_PUT_RUNTIME_STATE;
> }
>
> +/* Inject synchronous external abort */
> +static void kvm_inject_arm_sea(CPUState *c)
> +{
> + ARMCPU *cpu = ARM_CPU(c);
> + CPUARMState *env = &cpu->env;
> + CPUClass *cc = CPU_GET_CLASS(c);
> + uint32_t esr;
> + int ret;
> +
> + /* This exception is synchronous data abort */
> + c->exception_index = EXCP_DATA_ABORT;
> + /* Inject the exception to guest EL1 */
> + env->exception.target_el = 1;
These comments don't tell us anything that the code does not.
> +
> + /* Set the DFSC to synchronous external abort and set FnV to not valid,
> + * this will tell guest the FAR_ELx is UNKNOWN for this abort.
> + */
> +
> + /* This exception comes from lower or current exception level. */
> + if (arm_current_el(env) == PSTATE_MODE_EL0t) {
arm_current_el() returns an integer; you should be comparing it to
0,1,2,3, not to PSTATE_MODE_* constants.
> + esr = syn_data_abort_no_iss(0, 1, 0, 0, 0, 0, 0x10);
> + } else {
> + esr = syn_data_abort_no_iss(1, 1, 0, 0, 0, 0, 0x10);
> + }
Better written as
bool same_el;
[...]
same_el = arm_current_el(env) == env->exception.target_el;
env->exception.syndrome = syn_data_abort_no_iss(same_el, 1, 0, 0,
0, 0, 0x10);
> +
> + env->exception.syndrome = esr;
> +
> + cc->do_interrupt(c);
You need to take the iothread lock before calling do_interrupt.
Compare the code in kvm_arm_handle_debug().
> + if (kvm_enabled()) {
How can we get here if KVM is not enabled ?
> + /* write ESR_EL1 from cpustate to list*/
> + ret = write_part_cpustate_to_list(cpu,
> + offsetof(CPUARMState, cp15.esr_el[1]));
> + if (!ret) {
> + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__);
> + abort();
> + }
kvm_arm_handle_debug() doesn't need to do this, I don't understand
why this would be different.
> + }
> +}
> +
> #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
> KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
>
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 90741f6..3f1d656 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -109,7 +109,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t
> template_syn,
> * ISV field.
> */
> if (!(template_syn & ARM_EL_ISV) || target_el != 2 || s1ptw) {
> - syn = syn_data_abort_no_iss(same_el,
> + syn = syn_data_abort_no_iss(same_el, 0,
> ea, 0, s1ptw, is_write, fsc);
> } else {
> /* Fields: IL, ISV, SAS, SSE, SRT, SF and AR come from the template
> --
> 1.8.3.1
>
thanks
-- PMM
- [Qemu-arm] 答复: [Qemu-devel] [PATCH RESEND v15 07/10] KVM: Move related hwpoison page functions to accel/kvm/ folder, (continued)
[Qemu-arm] [PATCH RESEND v15 09/10] hw/arm/virt: Add RAS platform version for migration, Dongjiu Geng, 2018/11/08
[Qemu-arm] [PATCH RESEND v15 08/10] target-arm: kvm64: inject synchronous External Abort, Dongjiu Geng, 2018/11/08
- Re: [Qemu-arm] [PATCH RESEND v15 08/10] target-arm: kvm64: inject synchronous External Abort,
Peter Maydell <=
Re: [Qemu-arm] [Qemu-devel] [PATCH RESEND v15 00/10] Add ARMv8 RAS virtualization support in QEMU, no-reply, 2018/11/08
Re: [Qemu-arm] [Qemu-devel] [PATCH RESEND v15 00/10] Add ARMv8 RAS virtualization support in QEMU, no-reply, 2018/11/08