Hi Marc,
On 31/10/17 05:47, Marc Zyngier wrote:
> On Tue, Oct 31 2017 at 4:26:01 am GMT, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
>> On Thu, Oct 19 2017 at 4:58:03 pm BST, James Morse <james.morse@xxxxxxx> wrote:
>>> When we exit a guest due to an SError the vcpu fault info isn't updated
>>> with the ESR. Today this is only done for traps.
>>>
>>> The v8.2 RAS Extensions define ISS values for SError. Update the vcpu's
>>> fault_info with the ESR on SError so that handle_exit() can determine
>>> if this was a RAS SError and decode its severity.
>>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>>> index af37658223a0..cba6d8ac105c 100644
>>> --- a/arch/arm64/kvm/hyp/switch.c
>>> +++ b/arch/arm64/kvm/hyp/switch.c
>>> @@ -230,13 +230,20 @@ static bool __hyp_text __translate_far_to_hpfar(u64 far, u64 *hpfar)
>>> return true;
>>> }
>>>
>>> +static void __hyp_text __populate_fault_info_esr(struct kvm_vcpu *vcpu)
>>> +{
>>> + vcpu->arch.fault.esr_el2 = read_sysreg_el2(esr);
>>> +}
>>> +
>>> static bool __hyp_text __populate_fault_info(struct kvm_vcpu *vcpu)
>>> {
>>> - u64 esr = read_sysreg_el2(esr);
>>> - u8 ec = ESR_ELx_EC(esr);
>>> + u8 ec;
>>> + u64 esr;
>>> u64 hpfar, far;
>>>
>>> - vcpu->arch.fault.esr_el2 = esr;
>>> + __populate_fault_info_esr(vcpu);
>>> + esr = vcpu->arch.fault.esr_el2;
>>> + ec = ESR_ELx_EC(esr);
>>>
>>> if (ec != ESR_ELx_EC_DABT_LOW && ec != ESR_ELx_EC_IABT_LOW)
>>> return true;
>>> @@ -325,6 +332,8 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>>> */
>>> if (exit_code == ARM_EXCEPTION_TRAP && !__populate_fault_info(vcpu))
>>> goto again;
>>> + else if (ARM_EXCEPTION_CODE(exit_code) == ARM_EXCEPTION_EL1_SERROR)
>>> + __populate_fault_info_esr(vcpu);
>>>
>>> if (static_branch_unlikely(&vgic_v2_cpuif_trap) &&
>>> exit_code == ARM_EXCEPTION_TRAP) {
>>
>> With this patch, the only case were we don't save ESR_EL2 is when we
>> take an interrupt. I think we should bite the bullet and make it
>> slightly more streamlined, always saving ESR_EL2.
We always read it __guest_exit, just in case we take an SError and have to put
it back.
> Otherwise, an alternative would be to write something like:
>
> if (ARM_EXCEPTION_CODE(exit_code) != ARM_EXCEPTION_IRQ)
> vcpu->arch.fault.esr_el2 = read_sysreg_el2(esr);
>
> which still avoids saving it, and is a lot more readable.
I'll switch to this in the next version.
Thanks,
James
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
