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
>
> .
>
