| Message ID | 1514440458-10515-6-git-send-email-gengdongjiu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 28 Dec 2017 13:54:14 +0800 Dongjiu Geng <gengdongjiu@huawei.com> 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 <gengdongjiu@huawei.com> > --- > 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]. is it possible to inject SEA when running in TCG mode? it would be useful from testing/verification point of view (i.e. we could test logic on non ARM host during 'make check') > [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/kvm64.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index a16abc8..c00450d 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -582,6 +582,71 @@ int kvm_arm_cpreg_level(uint64_t regidx) > return KVM_PUT_RUNTIME_STATE; > } > > +static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset) > +{ > + int i; > + > + for (i = 0; i < cpu->cpreg_array_len; i++) { > + uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); > + const ARMCPRegInfo *ri; > + 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 0; > + } > + } > + return -EINVAL; > +} > + > +/* Inject synchronous external abort */ > +static void kvm_inject_arm_sea(CPUState *c) > +{ > + ARMCPU *cpu = ARM_CPU(c); > + CPUARMState *env = &cpu->env; > + unsigned long cpsr = pstate_read(env); > + uint32_t esr, ret; > + > + /* This exception is synchronous data abort*/ > + c->exception_index = EXCP_DATA_ABORT; > + /* Inject the exception to guest El1 */ > + env->exception.target_el = 1; > + CPUClass *cc = CPU_GET_CLASS(c); > + > + /* 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. > + */ > + esr = (0x10 | (1 << 10)); > + > + /* This exception comes from lower or current exception level. */ > + if ((cpsr & 0xf) == PSTATE_MODE_EL0t) { > + esr |= (EC_DATAABORT << ARM_EL_EC_SHIFT); > + } else { > + esr |= (EC_DATAABORT_SAME_EL << ARM_EL_EC_SHIFT); > + } > + > + /* For the AArch64, instruction length is 32-bit */ > + esr |= ARM_EL_IL; > + env->exception.syndrome = esr; > + > + cc->do_interrupt(c); > + > + /* set ESR_EL1 */ > + ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); > + > + if (ret) { > + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); > + abort(); > + } > +} > + > #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ > KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) >
Hi, Igor, On 2017/12/28 21:49, Igor Mammedov wrote: >> so user space how to inject it. The test result that injection SEA to guest by Qemu >> is shown in [2]. > is it possible to inject SEA when running in TCG mode? I have tested it in TCG mode, It supports to inject SEA when running in TCG mode. Thanks Start vm commands: ./qemu-system-aarch64 -m 1024 -cpu cortex-a57 -machine virt,gic-version=2 -bios QEMU_EFI.fd -smp 4 -nographic -kernel Image -append "root=/dev/sda1 \ console=ttyAMA0" -device virtio-scsi-device,id=scsi -drive file=./linaro.img,id=rootimg,cache=unsafe,if=none -device scsi-hd,drive=rootimg > > it would be useful from testing/verification point of view > (i.e. we could test logic on non ARM host during 'make check')> >
On 28 December 2017 at 05:54, Dongjiu Geng <gengdongjiu@huawei.com> 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 <gengdongjiu@huawei.com> > --- > 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/kvm64.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index a16abc8..c00450d 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -582,6 +582,71 @@ int kvm_arm_cpreg_level(uint64_t regidx) > return KVM_PUT_RUNTIME_STATE; > } > > +static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset) > +{ > + int i; > + > + for (i = 0; i < cpu->cpreg_array_len; i++) { This is still absolutely the wrong thing to do. Nothing should need to scan this array like this. > + uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); > + const ARMCPRegInfo *ri; > + 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 0; > + } > + } > + return -EINVAL; > +} > + > +/* Inject synchronous external abort */ > +static void kvm_inject_arm_sea(CPUState *c) > +{ > + ARMCPU *cpu = ARM_CPU(c); > + CPUARMState *env = &cpu->env; > + unsigned long cpsr = pstate_read(env); > + uint32_t esr, ret; > + > + /* This exception is synchronous data abort*/ Missing space before */ > + c->exception_index = EXCP_DATA_ABORT; > + /* Inject the exception to guest El1 */ "EL1", all caps. > + env->exception.target_el = 1; > + CPUClass *cc = CPU_GET_CLASS(c); Don't declare variables in the middle of the code -- check QEMU's CODING_STYLE doc for more info. > + > + /* 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. > + */ > + esr = (0x10 | (1 << 10)); > + > + /* This exception comes from lower or current exception level. */ > + if ((cpsr & 0xf) == PSTATE_MODE_EL0t) { This looks like it'll be wrong for AArch32 guests (which you can still have with KVM with a 64-bit host), and even for AArch32 userspace in a 64-bit guest. The correct way to find out what the current EL is is to use arm_current_el(). > + esr |= (EC_DATAABORT << ARM_EL_EC_SHIFT); > + } else { > + esr |= (EC_DATAABORT_SAME_EL << ARM_EL_EC_SHIFT); > + } I'm pretty sure in a previous round of review I said you shouldn't be manually constructing ESR values. We have helper functions for those (syn_data_abort_*). > + > + /* For the AArch64, instruction length is 32-bit */ > + esr |= ARM_EL_IL; > + env->exception.syndrome = esr; > + > + cc->do_interrupt(c); > + > + /* set ESR_EL1 */ > + ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); Breakpoint injection doesn't need to do this. Neither should this code. > + if (ret) { > + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); > + abort(); > + } > +} > + > #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ > KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) > > -- > 1.8.3.1 thanks -- PMM
Hi Peter. On 2018/1/10 1:30, Peter Maydell wrote: > On 28 December 2017 at 05:54, Dongjiu Geng <gengdongjiu@huawei.com> 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 <gengdongjiu@huawei.com> >> --- >> 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/kvm64.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 65 insertions(+) >> >> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c >> index a16abc8..c00450d 100644 >> --- a/target/arm/kvm64.c >> +++ b/target/arm/kvm64.c >> @@ -582,6 +582,71 @@ int kvm_arm_cpreg_level(uint64_t regidx) >> return KVM_PUT_RUNTIME_STATE; >> } >> >> +static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset) >> +{ >> + int i; >> + >> + for (i = 0; i < cpu->cpreg_array_len; i++) { > > This is still absolutely the wrong thing to do. Nothing should > need to scan this array like this. I will confirm that whether KVM mode will need this code, if not, I will remove this function. > >> + uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); >> + const ARMCPRegInfo *ri; >> + 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 0; >> + } >> + } >> + return -EINVAL; >> +} >> + >> +/* Inject synchronous external abort */ >> +static void kvm_inject_arm_sea(CPUState *c) >> +{ >> + ARMCPU *cpu = ARM_CPU(c); >> + CPUARMState *env = &cpu->env; >> + unsigned long cpsr = pstate_read(env); >> + uint32_t esr, ret; >> + >> + /* This exception is synchronous data abort*/ > > Missing space before */ will fix it. > >> + c->exception_index = EXCP_DATA_ABORT; >> + /* Inject the exception to guest El1 */ > > "EL1", all caps. will fix it. > >> + env->exception.target_el = 1; >> + CPUClass *cc = CPU_GET_CLASS(c); > > Don't declare variables in the middle of the code -- check QEMU's > CODING_STYLE doc for more info. will fix it. > >> + >> + /* 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. >> + */ >> + esr = (0x10 | (1 << 10)); >> + >> + /* This exception comes from lower or current exception level. */ >> + if ((cpsr & 0xf) == PSTATE_MODE_EL0t) { > > This looks like it'll be wrong for AArch32 guests (which you can > still have with KVM with a 64-bit host), and even for AArch32 > userspace in a 64-bit guest. The correct way to find out what the > current EL is is to use arm_current_el(). sorry, in the OS(include guest OS), for software error recovery, we only support AArch64 kernel, not support AArch32 kernel or AArch32 user space. > >> + esr |= (EC_DATAABORT << ARM_EL_EC_SHIFT); >> + } else { >> + esr |= (EC_DATAABORT_SAME_EL << ARM_EL_EC_SHIFT); >> + } > > I'm pretty sure in a previous round of review I said you shouldn't > be manually constructing ESR values. We have helper functions for > those (syn_data_abort_*). sorry, it is my mistake. I will use syn_data_abort_*. > >> + >> + /* For the AArch64, instruction length is 32-bit */ >> + esr |= ARM_EL_IL; >> + env->exception.syndrome = esr; >> + >> + cc->do_interrupt(c); >> + >> + /* set ESR_EL1 */ >> + ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); > > Breakpoint injection doesn't need to do this. Neither should this code. Good point. After I confirmed, in the TCG mode, it does not need this code. I am not sure whether KVM mode will need it. I will test it in the KVM mode. If KVM mode also does not need it, I will remove this code. > >> + if (ret) { >> + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); >> + abort(); >> + } >> +} >> + >> #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ >> KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) >> >> -- >> 1.8.3.1 > > thanks > -- PMM > > . >
On 11 January 2018 at 05:59, gengdongjiu <gengdongjiu@huawei.com> wrote: > Hi Peter. > > On 2018/1/10 1:30, Peter Maydell wrote: >> On 28 December 2017 at 05:54, Dongjiu Geng <gengdongjiu@huawei.com> wrote: >>> + >>> + /* This exception comes from lower or current exception level. */ >>> + if ((cpsr & 0xf) == PSTATE_MODE_EL0t) { >> >> This looks like it'll be wrong for AArch32 guests (which you can >> still have with KVM with a 64-bit host), and even for AArch32 >> userspace in a 64-bit guest. The correct way to find out what the >> current EL is is to use arm_current_el(). > sorry, in the OS(include guest OS), for software error recovery, > we only support AArch64 kernel, not support AArch32 > kernel or AArch32 user space. Nope, you must handle AArch32 EL1 correctly in some way, even if that is only "this guest CPU doesn't support RAS notification and we will not notify it". And you must absolutely support AArch32 EL0, that's a requirement for getting this merged. It's not difficult. thanks -- PMM
On 2018/1/11 17:53, Peter Maydell wrote: >> we only support AArch64 kernel, not support AArch32 >> kernel or AArch32 user space. > Nope, you must handle AArch32 EL1 correctly in some way, even if that > is only "this guest CPU doesn't support RAS notification and we > will not notify it". And you must absolutely support AArch32 EL0, > that's a requirement for getting this merged. It's not difficult. sure, it needed. I absolutely follow your suggestion and handle AArch32 EL1 correctly in some way. it is not difficult.
Hi Peter, >> +static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset) >> +{ >> + int i; >> + >> + for (i = 0; i < cpu->cpreg_array_len; i++) { > > This is still absolutely the wrong thing to do. Nothing should > need to scan this array like this. For the KVM mode, I use this function to set the ESR_ELx's value. If not set it using this way, do you have better method? Thanks! From my test, if kvm_inject_arm_sea() does not call this function kvm_arm_cpreg_value() to set the ESR_ELx's register, the guest will have wrong ESR value. As shown the log in [1], QEMU sets the ESR to 0x96000414, but the guest's ESR value is 0x56000000 instead of 0x96000414. [1]: Taking exception 4 [Data Abort] ...from EL1 to EL1 ...with ESR 0x25/0x96000414 ...with FAR 0x0 ...with ELR 0xffffff8008081a80 ...to EL1 PC 0xffffff8008081a00 PSTATE 0x3c5 [ 16.974756] Bad mode in Synchronous Abort handler detected on CPU0, code 0x56000000 -- SVC (AArch64) [ 16.989504] Internal error: Oops - bad mode: 0 [#1] SMP [ 16.990753] Modules linked in: [ 16.991462] CPU: 0 PID: 204 Comm: sh Tainted: G W 4.13.0-rc4ajb-00005-g1353b1e-dirty #40 [ 16.993533] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015 [ 16.995083] task: ffffffc03d3c2b00 task.stack: ffffffc03d2b0000 [ 16.996448] PC is at vectors+0x280/0x784 [ 16.997340] LR is at pl011_tx_empty+0x18/0x40 > >> + uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); >> + const ARMCPRegInfo *ri; >> + ri = get_arm_cp_reginfo(cpu->cp_regs, regidx); >> + if (!ri) { >> + continue; >> + } [...] > >> + >> + /* For the AArch64, instruction length is 32-bit */ >> + esr |= ARM_EL_IL; >> + env->exception.syndrome = esr; >> + >> + cc->do_interrupt(c); >> + >> + /* set ESR_EL1 */ >> + ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); > > Breakpoint injection doesn't need to do this. Neither should this code. As my above explanation, in the KVM mode, it needs to set the ESR_ELx in extra method. the cc->do_interrupt(c) does not set ESR_ELx. so I use kvm_arm_cpreg_value() to set it. whether you have better method to set the ESR_Elx except for my method? Thanks. > >> + if (ret) { >> + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); >> + abort(); >> + } >> +} >> + >> #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ >> KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) >> >> -- >> 1.8.3.1 > > thanks > -- PMM > > . >
Hi Peter, On 2018/1/13 13:24, gengdongjiu wrote: >>> + >>> + /* For the AArch64, instruction length is 32-bit */ >>> + esr |= ARM_EL_IL; >>> + env->exception.syndrome = esr; >>> + >>> + cc->do_interrupt(c); >>> + >>> + /* set ESR_EL1 */ >>> + ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); >> Breakpoint injection doesn't need to do this. Neither should this code. > As my above explanation, in the KVM mode, it needs to set the ESR_ELx in extra method. > the cc->do_interrupt(c) does not set ESR_ELx. so I use kvm_arm_cpreg_value() > to set it. whether you have better method to set the ESR_Elx except for my method? Thanks. If QEMU changes the KVM's registers, it needs to call write_list_to_kvmstate() to write the cpu->cpreg_values[] list to KVM through KVM_SET_ONE_REG IOCTL[1]. In Qemu, now it should not have software path to change the cpu->cpreg_values[] list except write_cpustate_to_list(). Here I can also call write_cpustate_to_list() instead of kvm_arm_cpreg_value() to change cpu->cpreg_values[] list, but the write_cpustate_to_list() will write all the coprocessor state to the cpu->cpreg_values[] list, we can not sure all the coprocessor states are right, so here I only change corresponding index value in this list using kvm_arm_cpreg_value(). Breakpoint injection that you mentioned should not change KVM register or not in the KVM mode. [1]: kvm_arch_put_registers() -> write_list_to_kvmstate() -> write cpu->cpreg_values[] to the kernel KVM through KVM_SET_ONE_REG > > >>> + if (ret) { >>> + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); >>> + abort(); >>> + } >>> +} >>> + >>> #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ >>> KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) >>> >>> -- >>> 1.8.3.1
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index a16abc8..c00450d 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -582,6 +582,71 @@ int kvm_arm_cpreg_level(uint64_t regidx) return KVM_PUT_RUNTIME_STATE; } +static int kvm_arm_cpreg_value(ARMCPU *cpu, ptrdiff_t fieldoffset) +{ + int i; + + for (i = 0; i < cpu->cpreg_array_len; i++) { + uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); + const ARMCPRegInfo *ri; + 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 0; + } + } + return -EINVAL; +} + +/* Inject synchronous external abort */ +static void kvm_inject_arm_sea(CPUState *c) +{ + ARMCPU *cpu = ARM_CPU(c); + CPUARMState *env = &cpu->env; + unsigned long cpsr = pstate_read(env); + uint32_t esr, ret; + + /* This exception is synchronous data abort*/ + c->exception_index = EXCP_DATA_ABORT; + /* Inject the exception to guest El1 */ + env->exception.target_el = 1; + CPUClass *cc = CPU_GET_CLASS(c); + + /* 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. + */ + esr = (0x10 | (1 << 10)); + + /* This exception comes from lower or current exception level. */ + if ((cpsr & 0xf) == PSTATE_MODE_EL0t) { + esr |= (EC_DATAABORT << ARM_EL_EC_SHIFT); + } else { + esr |= (EC_DATAABORT_SAME_EL << ARM_EL_EC_SHIFT); + } + + /* For the AArch64, instruction length is 32-bit */ + esr |= ARM_EL_IL; + env->exception.syndrome = esr; + + cc->do_interrupt(c); + + /* set ESR_EL1 */ + ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); + + if (ret) { + fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); + abort(); + } +} + #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
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 <gengdongjiu@huawei.com> --- 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/kvm64.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)