On Wed, Nov 14, 2012 at 3:33 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > 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. > no need, I fixed an applied. thanks. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
