On Tue, 13 Nov 2012 20:39:21 -0500, Christoffer Dall
<c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Nov 12, 2012 at 12:02 PM, Marc Zyngier <marc.zyngier@xxxxxxx>
> wrote:
>> This patch fixes a very long standing bug in the KVM fault handling and
>> is basically a backport of the equivalent arm64 code.
>>
>> The ARM ARM is quite explicit in saying the HPFAR cannot be trusted
>> for a permission fault, unless it is a Stage-1 page table walk.
>>
>> The fact that it *seems* to work well enough on A15 is even more scary.
>> We may well be silently corrupting memory without even noticing (hint,
>> hint...). It is also quite possible that this bug would prevent KVM
>> from running on a CPU which caches TLBs covering VA to PA with a single
>> entry.
>>
>> The fix is to detect aborts that are permission faults, and not stage-1
>> page table walk. We can then resolve the IPA, and store it just like
the
>> HPFAR register. If the IPA cannot be resolved, it means another CPU is
>> playing with the page tables, and we simply restart the guest.
>>
>> This also paves the way for lighter Icache invalidation, using the NX
>> bit.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> ---
>> >From v1:
>> - Use ATS1CPR instead of ATS1CPW
>>
>> arch/arm/kvm/interrupts.S | 37 ++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
>> index 5a09e89..b2d52a9 100644
>> --- a/arch/arm/kvm/interrupts.S
>> +++ b/arch/arm/kvm/interrupts.S
>> @@ -401,12 +401,47 @@ guest_trap:
>> mrc p15, 4, r2, c6, c0, 0 @ HDFAR
>>
>> 2: str r2, [r1, #VCPU_HxFAR]
>> - mrc p15, 4, r2, c6, c0, 4 @ HPFAR
>> +
>> + /*
>> + * B3.13.5 Reporting exceptions taken to the Non-secure PL2
mode:
>> + *
>> + * Abort on the stage 2 translation for a memory access from a
>> + * Non-secure PL1 or PL0 mode:
>> + *
>> + * For any Access flag fault or Translation fault, and also for
>> any
>> + * Permission fault on the stage 2 translation of a memory
access
>> + * made as part of a translation table walk for a stage 1
>> translation,
>> + * the HPFAR holds the IPA that caused the fault. Otherwise,
the
>> HPFAR
>> + * is UNKNOWN.
>> + */
>> +
>> + /* Check for permission fault, and S1PTW */
>> + mrc p15, 4, r1, c5, c2, 0 @ HSR
>> + and r0, r1, #HSR_FSC_TYPE
>> + cmp r0, #FSC_PERM
>> + tsteq r1, #7 @ S1PTW
>
>
> shouldn't this be
>
> tsteq r1, #(1 << 7) @ S1PTW
>
> ?
Yup. arm64 contamination. Will fix and repost.
Thanks,
M.
--
Fast, cheap, reliable. Pick two.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
