Re: [RFC PATCH v2 20/38] KVM: arm64: Handle eret instruction traps

[Jintack Lim]

On Sun, Jul 30, 2017 at 4:00 PM, Christoffer Dall <cdall@xxxxxxxxxx> wrote:
> On Tue, Jul 18, 2017 at 11:58:46AM -0500, Jintack Lim wrote:
>> When HCR.NV bit is set, eret instructions trap to EL2 with EC code 0x1A.
>> Emulate eret instructions by setting pc and pstate.
>
> It may be worth noting in the commit message that this is all we have to
> do, because the rest of the logic will then discover that the mode could
> change from virtual EL2 to EL1 and will setup the hw registers etc. when
> changing modes.

Makes sense. I'll write it up in the commit message.

>
>>
>> Note that the current exception level is always the virtual EL2, since
>> we set HCR_EL2.NV bit only when entering the virtual EL2. So, we take
>> spsr and elr states from the virtual _EL2 registers.
>>
>> Signed-off-by: Jintack Lim <jintack.lim@xxxxxxxxxx>
>> ---
>>  arch/arm64/include/asm/esr.h |  1 +
>>  arch/arm64/kvm/handle_exit.c | 16 ++++++++++++++++
>>  arch/arm64/kvm/trace.h       | 21 +++++++++++++++++++++
>>  3 files changed, 38 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
>> index e7d8e28..210fde6 100644
>> --- a/arch/arm64/include/asm/esr.h
>> +++ b/arch/arm64/include/asm/esr.h
>> @@ -43,6 +43,7 @@
>>  #define ESR_ELx_EC_HVC64     (0x16)
>>  #define ESR_ELx_EC_SMC64     (0x17)
>>  #define ESR_ELx_EC_SYS64     (0x18)
>> +#define ESR_ELx_EC_ERET              (0x1A)
>>  /* Unallocated EC: 0x19 - 0x1E */
>>  #define ESR_ELx_EC_IMP_DEF   (0x1f)
>>  #define ESR_ELx_EC_IABT_LOW  (0x20)
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 17d8a16..9259881 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -147,6 +147,21 @@ static int kvm_handle_unknown_ec(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>       return 1;
>>  }
>>
>> +static int kvm_handle_eret(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> +{
>> +     trace_kvm_nested_eret(vcpu, vcpu_el2_sreg(vcpu, ELR_EL2),
>> +                           vcpu_el2_sreg(vcpu, SPSR_EL2));
>> +
>> +     /*
>> +      * Note that the current exception level is always the virtual EL2,
>> +      * since we set HCR_EL2.NV bit only when entering the virtual EL2.
>> +      */
>> +     *vcpu_pc(vcpu) = vcpu_el2_sreg(vcpu, ELR_EL2);
>> +     *vcpu_cpsr(vcpu) = vcpu_el2_sreg(vcpu, SPSR_EL2);
>> +
>> +     return 1;
>> +}
>> +
>>  static exit_handle_fn arm_exit_handlers[] = {
>>       [0 ... ESR_ELx_EC_MAX]  = kvm_handle_unknown_ec,
>>       [ESR_ELx_EC_WFx]        = kvm_handle_wfx,
>> @@ -160,6 +175,7 @@ static int kvm_handle_unknown_ec(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>       [ESR_ELx_EC_HVC64]      = handle_hvc,
>>       [ESR_ELx_EC_SMC64]      = handle_smc,
>>       [ESR_ELx_EC_SYS64]      = kvm_handle_sys_reg,
>> +     [ESR_ELx_EC_ERET]       = kvm_handle_eret,
>>       [ESR_ELx_EC_IABT_LOW]   = kvm_handle_guest_abort,
>>       [ESR_ELx_EC_DABT_LOW]   = kvm_handle_guest_abort,
>>       [ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
>> diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
>> index 7c86cfb..5f40987 100644
>> --- a/arch/arm64/kvm/trace.h
>> +++ b/arch/arm64/kvm/trace.h
>> @@ -187,6 +187,27 @@
>>       TP_printk("vcpu: %p, inject exception to vEL2: ESR_EL2 0x%lx, vector: 0x%016lx",
>>                 __entry->vcpu, __entry->esr_el2, __entry->pc)
>>  );
>> +
>> +TRACE_EVENT(kvm_nested_eret,
>> +     TP_PROTO(struct kvm_vcpu *vcpu, unsigned long elr_el2,
>> +              unsigned long spsr_el2),
>> +     TP_ARGS(vcpu, elr_el2, spsr_el2),
>> +
>> +     TP_STRUCT__entry(
>> +             __field(struct kvm_vcpu *,      vcpu)
>> +             __field(unsigned long,          elr_el2)
>> +             __field(unsigned long,          spsr_el2)
>> +     ),
>> +
>> +     TP_fast_assign(
>> +             __entry->vcpu = vcpu;
>> +             __entry->elr_el2 = elr_el2;
>> +             __entry->spsr_el2 = spsr_el2;
>> +     ),
>> +
>> +     TP_printk("vcpu: %p, eret to elr_el2: 0x%016lx, with spsr_el2: 0x%08lx",
>> +               __entry->vcpu, __entry->elr_el2, __entry->spsr_el2)
>> +);
>>  #endif /* _TRACE_ARM64_KVM_H */
>>
>>  #undef TRACE_INCLUDE_PATH
>> --
>> 1.9.1
>>
>
> Otherwise this patch looks good.
>
> Thanks,
> -Christoffer