Hi Quentin,
On 2021/6/2 18:58, Quentin Perret wrote:
On Thursday 15 Apr 2021 at 19:50:31 (+0800), Yanan Wang wrote:
In this patch, we move invalidation of I-cache to the fault handlers to
Nit: please avoid using 'This patch' in commit messages, see
Documentation/process/submitting-patches.rst.
Thanks!
I will get rid of this.
avoid unnecessary I-cache maintenances. On the map path, invalidate the
I-cache if we are going to create an executable stage-2 mapping for guest.
And on the permission path, invalidate the I-cache if we are going to add
an executable permission to the existing guest stage-2 mapping.
Signed-off-by: Yanan Wang <wangyanan55@xxxxxxxxxx>
---
arch/arm64/include/asm/kvm_mmu.h | 15 --------------
arch/arm64/kvm/hyp/pgtable.c | 35 +++++++++++++++++++++++++++++++-
arch/arm64/kvm/mmu.c | 9 +-------
3 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index e9b163c5f023..155492fe5b15 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -187,21 +187,6 @@ static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
return (vcpu_read_sys_reg(vcpu, SCTLR_EL1) & 0b101) == 0b101;
}
-static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
- unsigned long size)
-{
- if (icache_is_aliasing()) {
- /* any kind of VIPT cache */
- __flush_icache_all();
- } else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
- /* PIPT or VPIPT at EL2 (see comment in __kvm_tlb_flush_vmid_ipa) */
- void *va = page_address(pfn_to_page(pfn));
-
- invalidate_icache_range((unsigned long)va,
- (unsigned long)va + size);
- }
-}
-
void kvm_set_way_flush(struct kvm_vcpu *vcpu);
void kvm_toggle_cache(struct kvm_vcpu *vcpu, bool was_enabled);
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index b480f6d1171e..9f4429d80df0 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -568,6 +568,26 @@ static bool stage2_pte_cacheable(struct kvm_pgtable *pgt, kvm_pte_t pte)
return memattr == KVM_S2_MEMATTR(pgt, NORMAL);
}
+static bool stage2_pte_executable(kvm_pte_t pte)
+{
+ return !(pte & KVM_PTE_LEAF_ATTR_HI_S2_XN);
+}
+
+static void stage2_invalidate_icache(void *addr, u64 size)
+{
+ if (icache_is_aliasing()) {
+ /* Any kind of VIPT cache */
+ __flush_icache_all();
+ } else if (is_kernel_in_hyp_mode() || !icache_is_vpipt()) {
+ /*
+ * See comment in __kvm_tlb_flush_vmid_ipa().
+ * Invalidate PIPT, or VPIPT at EL2.
+ */
+ invalidate_icache_range((unsigned long)addr,
+ (unsigned long)addr + size);
+ }
+}
+
static void stage2_put_pte(kvm_pte_t *ptep, struct kvm_s2_mmu *mmu, u64 addr,
u32 level, struct kvm_pgtable_mm_ops *mm_ops)
{
@@ -618,6 +638,10 @@ static int stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
if (stage2_pte_cacheable(pgt, new) && !stage2_has_fwb(pgt))
__flush_dcache_area(mm_ops->phys_to_virt(phys),
granule);
+
+ if (stage2_pte_executable(new))
+ stage2_invalidate_icache(mm_ops->phys_to_virt(phys),
+ granule);
}
smp_store_release(ptep, new);
@@ -896,8 +920,17 @@ static int stage2_attr_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
* but worst-case the access flag update gets lost and will be
* set on the next access instead.
*/
- if (data->pte != pte)
+ if (data->pte != pte) {
+ /*
+ * Invalidate the instruction cache before updating
+ * if we are going to add the executable permission
+ * for the guest stage-2 PTE.
+ */
+ if (!stage2_pte_executable(*ptep) && stage2_pte_executable(pte))
+ stage2_invalidate_icache(kvm_pte_follow(pte, data->mm_ops),
+ kvm_granule_size(level));
WRITE_ONCE(*ptep, pte);
+ }
As for the dcache stuff, it seems like this would be best placed in an
optional mm_ops callback, and have the kernel implement it.
I think so, that is the preferred way.
Thanks,
Yanan
Thanks,
Quentin
.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm