Skip to content

Commit 406504c

Browse files
author
Marc Zyngier
committed
KVM: arm64: Fix S1PTW handling on RO memslots
A recent development on the EFI front has resulted in guests having their page tables baked in the firmware binary, and mapped into the IPA space as part of a read-only memslot. Not only is this legitimate, but it also results in added security, so thumbs up. It is possible to take an S1PTW translation fault if the S1 PTs are unmapped at stage-2. However, KVM unconditionally treats S1PTW as a write to correctly handle hardware AF/DB updates to the S1 PTs. Furthermore, KVM injects an exception into the guest for S1PTW writes. In the aforementioned case this results in the guest taking an abort it won't recover from, as the S1 PTs mapping the vectors suffer from the same problem. So clearly our handling is... wrong. Instead, switch to a two-pronged approach: - On S1PTW translation fault, handle the fault as a read - On S1PTW permission fault, handle the fault as a write This is of no consequence to SW that *writes* to its PTs (the write will trigger a non-S1PTW fault), and SW that uses RO PTs will not use HW-assisted AF/DB anyway, as that'd be wrong. Only in the case described in c4ad98e ("KVM: arm64: Assume write fault on S1PTW permission fault on instruction fetch") do we end-up with two back-to-back faults (page being evicted and faulted back). I don't think this is a case worth optimising for. Fixes: c4ad98e ("KVM: arm64: Assume write fault on S1PTW permission fault on instruction fetch") Reviewed-by: Oliver Upton <oliver.upton@linux.dev> Reviewed-by: Ard Biesheuvel <ardb@kernel.org> Regression-tested-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Marc Zyngier <maz@kernel.org> Cc: stable@vger.kernel.org
1 parent 1b929c0 commit 406504c

1 file changed

Lines changed: 20 additions & 2 deletions

File tree

arch/arm64/include/asm/kvm_emulate.h

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,26 @@ static __always_inline int kvm_vcpu_sys_get_rt(struct kvm_vcpu *vcpu)
373373

374374
static inline bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
375375
{
376-
if (kvm_vcpu_abt_iss1tw(vcpu))
377-
return true;
376+
if (kvm_vcpu_abt_iss1tw(vcpu)) {
377+
/*
378+
* Only a permission fault on a S1PTW should be
379+
* considered as a write. Otherwise, page tables baked
380+
* in a read-only memslot will result in an exception
381+
* being delivered in the guest.
382+
*
383+
* The drawback is that we end-up faulting twice if the
384+
* guest is using any of HW AF/DB: a translation fault
385+
* to map the page containing the PT (read only at
386+
* first), then a permission fault to allow the flags
387+
* to be set.
388+
*/
389+
switch (kvm_vcpu_trap_get_fault_type(vcpu)) {
390+
case ESR_ELx_FSC_PERM:
391+
return true;
392+
default:
393+
return false;
394+
}
395+
}
378396

379397
if (kvm_vcpu_trap_is_iabt(vcpu))
380398
return false;

0 commit comments

Comments
 (0)