Subject: [PATCH v2 01/10] arm64: KVM: force cache clean on page fault when caches are off

From: [email-masked] (Christoffer Dall)
Date: Wed, 29 Jan 2014 12:06:34 -0800

On Wed, Jan 22, 2014 at 02:56:33PM +0000, Marc Zyngier wrote:
> In order for the guest with caches off to observe data written
> contained in a given page, we need to make sure that page is
> committed to memory, and not just hanging in the cache (as
> guest accesses are completely bypassing the cache until it
> decides to enable it).
> 
> For this purpose, hook into the coherent_icache_guest_page
> function and flush the region if the guest SCTLR_EL1
> register doesn't show the MMU  and caches as being enabled.
> The function also get renamed to coherent_cache_guest_page.
> 
> Signed-off-by: Marc Zyngier <[email-masked]>
> Reviewed-by: Catalin Marinas <[email-masked]>
> ---
>  arch/arm/include/asm/kvm_mmu.h   |  4 ++--
>  arch/arm/kvm/mmu.c               |  4 ++--
>  arch/arm64/include/asm/kvm_mmu.h | 11 +++++++----
>  3 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index 77de4a4..f997b9e 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -116,8 +116,8 @@ static inline void kvm_set_s2pmd_writable(pmd_t *pmd)
>  
>  struct kvm;
>  
> -static inline void coherent_icache_guest_page(struct kvm *kvm, hva_t hva,
> -					      unsigned long size)
> +static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> +					     unsigned long size)
>  {
>  	/*
>  	 * If we are going to insert an instruction page and the icache is
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 5809069..415fd63 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -713,7 +713,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  			kvm_set_s2pmd_writable(&new_pmd);
>  			kvm_set_pfn_dirty(pfn);
>  		}
> -		coherent_icache_guest_page(kvm, hva & PMD_MASK, PMD_SIZE);
> +		coherent_cache_guest_page(vcpu, hva & PMD_MASK, PMD_SIZE);
>  		ret = stage2_set_pmd_huge(kvm, memcache, fault_ipa, &new_pmd);
>  	} else {
>  		pte_t new_pte = pfn_pte(pfn, PAGE_S2);
> @@ -721,7 +721,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  			kvm_set_s2pte_writable(&new_pte);
>  			kvm_set_pfn_dirty(pfn);
>  		}
> -		coherent_icache_guest_page(kvm, hva, PAGE_SIZE);
> +		coherent_cache_guest_page(vcpu, hva, PAGE_SIZE);
>  		ret = stage2_set_pte(kvm, memcache, fault_ipa, &new_pte, false);
>  	}
>  
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 680f74e..2232dd0 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -106,7 +106,6 @@ static inline bool kvm_is_write_fault(unsigned long esr)
>  	return true;
>  }
>  
> -static inline void kvm_clean_dcache_area(void *addr, size_t size) {}
>  static inline void kvm_clean_pgd(pgd_t *pgd) {}
>  static inline void kvm_clean_pmd_entry(pmd_t *pmd) {}
>  static inline void kvm_clean_pte(pte_t *pte) {}
> @@ -124,9 +123,14 @@ static inline void kvm_set_s2pmd_writable(pmd_t *pmd)
>  
>  struct kvm;
>  
> -static inline void coherent_icache_guest_page(struct kvm *kvm, hva_t hva,
> -					      unsigned long size)
> +#define kvm_flush_dcache_to_poc(a,l)	__flush_dcache_area((a), (l))
> +
> +static inline void coherent_cache_guest_page(struct kvm_vcpu *vcpu, hva_t hva,
> +					     unsigned long size)
>  {
> +	if ((vcpu_sys_reg(vcpu, SCTLR_EL1) & 0b101) != 0b101)
> +		kvm_flush_dcache_to_poc((void *)hva, size);
> +

This deserves a comment or a static inline...

>  	if (!icache_is_aliasing()) {		/* PIPT */
>  		flush_icache_range(hva, hva + size);
>  	} else if (!icache_is_aivivt()) {	/* non ASID-tagged VIVT */
> @@ -135,7 +139,6 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, hva_t hva,
>  	}
>  }
>  
> -#define kvm_flush_dcache_to_poc(a,l)	__flush_dcache_area((a), (l))
>  
>  #endif /* __ASSEMBLY__ */
>  #endif /* __ARM64_KVM_MMU_H__ */
> -- 
> 1.8.3.4
> 

Otherwise:
Reviewed-by: Christoffer Dall <[email-masked]>