Hi Ard,
On 2014-09-08 21:29, Ard Biesheuvel wrote:
The ISS encoding for an exception from a Data Abort has a WnR
bit[6] that indicates whether the Data Abort was caused by a
read or a write instruction. While there are several fields
in the encoding that are only valid if the ISV bit[24] is set,
WnR is not one of them, so we can read it unconditionally.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
---
This fixes an issue I observed with UEFI running under QEMU/KVM using
NOR flash emulation and the upcoming KVM_CAP_READONLY_MEM support,
where
NOR flash reads were mistaken for NOR flash writes, resulting in all
read
accesses to go through the MMIO emulation layer.
arch/arm/include/asm/kvm_mmu.h | 5 +----
arch/arm64/include/asm/kvm_mmu.h | 5 +----
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/kvm_mmu.h
b/arch/arm/include/asm/kvm_mmu.h
index 5cc0b0f5f72f..fad5648980ad 100644
--- a/arch/arm/include/asm/kvm_mmu.h
+++ b/arch/arm/include/asm/kvm_mmu.h
@@ -83,10 +83,7 @@ static inline bool kvm_is_write_fault(unsigned
long hsr)
unsigned long hsr_ec = hsr >> HSR_EC_SHIFT;
if (hsr_ec == HSR_EC_IABT)
return false;
- else if ((hsr & HSR_ISV) && !(hsr & HSR_WNR))
- return false;
- else
- return true;
+ return hsr & HSR_WNR;
}
static inline void kvm_clean_pgd(pgd_t *pgd)
diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index 8e138c7c53ac..09fd9e4c13d8 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -100,10 +100,7 @@ static inline bool kvm_is_write_fault(unsigned
long esr)
if (esr_ec == ESR_EL2_EC_IABT)
return false;
- if ((esr & ESR_EL2_ISV) && !(esr & ESR_EL2_WNR))
- return false;
-
- return true;
+ return esr & ESR_EL2_WNR;
}
static inline void kvm_clean_pgd(pgd_t *pgd) {}
Nice catch. One thing though.
This is a case where code duplication has led to this glaring bug:
On both arm and arm64, kvm_emulate.h has code that implements this
correctly, just that we failed to use it. Blame me.
I think this should be rewritten entierely in mmu.c, with something
like this (fully untested, of course):
static bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
{
if (kvm_vcpu_trap_is_iabt(vcpu))
return false;
return kvm_vcpu_dabt_iswrite(vcpu);
}
Care to respin it?
Thanks,
M.
--
Fast, cheap, reliable. Pick two.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
