On Wed, Feb 13, 2013 at 03:47:03PM +0000, Marc Zyngier wrote:
> Permission fault can be more than just a write fault. Execution
> fault can also occur if the page is flagged with the XN bit.
>
> kvm_decode_fault() returns a value that indicates the fault type
> (read, write, exec).
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm/include/asm/kvm_mmu.h | 14 ++++++++++----
> arch/arm/kvm/mmu.c | 10 ++++++----
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index 970f3b5..257e2bc 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -58,15 +58,21 @@ static inline void kvm_set_pte(pte_t *pte, pte_t new_pte)
> flush_pmd_entry(pte);
> }
>
> -static inline bool kvm_is_write_fault(unsigned long hsr)
> +enum kvm_fault_type {
> + KVM_READ_FAULT,
> + KVM_WRITE_FAULT,
> + KVM_EXEC_FAULT,
> +};
> +
> +static inline enum kvm_fault_type kvm_decode_fault(unsigned long hsr)
> {
> unsigned long hsr_ec = hsr >> HSR_EC_SHIFT;
> if (hsr_ec == HSR_EC_IABT)
> - return false;
> + return KVM_EXEC_FAULT;
> else if ((hsr & HSR_ISV) && !(hsr & HSR_WNR))
> - return false;
> + return KVM_READ_FAULT;
> else
> - return true;
> + return KVM_WRITE_FAULT;
> }
>
> static inline void kvm_clean_pgd(pgd_t *pgd)
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 633b546..58a45d1 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -510,12 +510,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> pte_t new_pte, *ptep;
> pfn_t pfn;
> int ret;
> - bool write_fault, writable;
> + bool writable;
> + enum kvm_fault_type fault_type;
> unsigned long mmu_seq;
> struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache;
>
> - write_fault = kvm_is_write_fault(kvm_vcpu_get_hsr(vcpu));
> - if (fault_status == FSC_PERM && !write_fault) {
> + fault_type = kvm_decode_fault(kvm_vcpu_get_hsr(vcpu));
> + if (fault_status == FSC_PERM && fault_type == KVM_READ_FAULT) {
> kvm_err("Unexpected L2 read permission error\n");
> return -EFAULT;
> }
> @@ -537,7 +538,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> */
> smp_rmb();
>
> - pfn = gfn_to_pfn_prot(vcpu->kvm, gfn, write_fault, &writable);
> + pfn = gfn_to_pfn_prot(vcpu->kvm, gfn,
> + fault_type == KVM_WRITE_FAULT, &writable);
This comparison within the function call looks quite ugly to me, could
you use a variabled instead?
> if (is_error_pfn(pfn))
> return -EFAULT;
>
> --
> 1.8.1.2
>
>
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
