All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM/arm64: Cache maintenance relaxations
@ 2018-05-17 10:35 ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel; +Cc: Catalin Marinas, Christoffer Dall

This small series makes use of features recently introduced in the
ARMv8 architecture to relax the cache maintenance operations on CPUs
that implement these features.

FWB is the most important one. It allows stage-2 to enforce the
cacheability of memory, no matter what the guest says. It also
mandates that the whole machine is cache coherent (no non-coherent
I/O), meaning we can drop a whole class of cache maintenance
operations.

FWB, when combined with CTL_EL0.{IDC,DIC} allows the removal of all
cache maintenance and tracking of pages being executed.

We also take the opportunity to drop a few useless CMOs that were
applied to the HYP page tables, but that were never necessary.

Marc Zyngier (4):
  arm64: KVM: Add support for Stage-2 control of memory types and
    cacheability
  arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
  KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present

 arch/arm64/include/asm/cpucaps.h      |  3 ++-
 arch/arm64/include/asm/kvm_arm.h      |  1 +
 arch/arm64/include/asm/kvm_emulate.h  |  2 ++
 arch/arm64/include/asm/kvm_mmu.h      | 24 +++++++++++++++++-------
 arch/arm64/include/asm/memory.h       |  7 +++++++
 arch/arm64/include/asm/pgtable-prot.h | 24 ++++++++++++++++++++++--
 arch/arm64/include/asm/sysreg.h       |  1 +
 arch/arm64/kernel/cpufeature.c        | 20 ++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c             |  8 +++++++-
 virt/kvm/arm/mmu.c                    |  9 +++++----
 10 files changed, 84 insertions(+), 15 deletions(-)

-- 
2.14.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 0/4] KVM/arm64: Cache maintenance relaxations
@ 2018-05-17 10:35 ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

This small series makes use of features recently introduced in the
ARMv8 architecture to relax the cache maintenance operations on CPUs
that implement these features.

FWB is the most important one. It allows stage-2 to enforce the
cacheability of memory, no matter what the guest says. It also
mandates that the whole machine is cache coherent (no non-coherent
I/O), meaning we can drop a whole class of cache maintenance
operations.

FWB, when combined with CTL_EL0.{IDC,DIC} allows the removal of all
cache maintenance and tracking of pages being executed.

We also take the opportunity to drop a few useless CMOs that were
applied to the HYP page tables, but that were never necessary.

Marc Zyngier (4):
  arm64: KVM: Add support for Stage-2 control of memory types and
    cacheability
  arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
  KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present

 arch/arm64/include/asm/cpucaps.h      |  3 ++-
 arch/arm64/include/asm/kvm_arm.h      |  1 +
 arch/arm64/include/asm/kvm_emulate.h  |  2 ++
 arch/arm64/include/asm/kvm_mmu.h      | 24 +++++++++++++++++-------
 arch/arm64/include/asm/memory.h       |  7 +++++++
 arch/arm64/include/asm/pgtable-prot.h | 24 ++++++++++++++++++++++--
 arch/arm64/include/asm/sysreg.h       |  1 +
 arch/arm64/kernel/cpufeature.c        | 20 ++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c             |  8 +++++++-
 virt/kvm/arm/mmu.c                    |  9 +++++----
 10 files changed, 84 insertions(+), 15 deletions(-)

-- 
2.14.2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/4] arm64: KVM: Add support for Stage-2 control of memory types and cacheability
  2018-05-17 10:35 ` Marc Zyngier
@ 2018-05-17 10:35   ` Marc Zyngier
  -1 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel; +Cc: Catalin Marinas

Up to ARMv8.3, the combinaison of Stage-1 and Stage-2 attributes
results in the strongest attribute of the two stages.  This means
that the hypervisor has to perform quite a lot of cache maintenance
just in case the guest has some non-cacheable mappings around.

ARMv8.4 solves this problem by offering a different mode (FWB) where
Stage-2 has total control over the memory attribute (this is limited
to systems where both I/O and instruction caches are coherent with
the dcache). This is achieved by having a different set of memory
attributes in the page tables, and a new bit set in HCR_EL2.

On such a system, we can then safely sidestep any form of dcache
management.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cpucaps.h      |  3 ++-
 arch/arm64/include/asm/kvm_arm.h      |  1 +
 arch/arm64/include/asm/kvm_emulate.h  |  2 ++
 arch/arm64/include/asm/kvm_mmu.h      | 24 +++++++++++++++++-------
 arch/arm64/include/asm/memory.h       |  7 +++++++
 arch/arm64/include/asm/pgtable-prot.h | 14 ++++++++++++--
 arch/arm64/include/asm/sysreg.h       |  1 +
 arch/arm64/kernel/cpufeature.c        | 20 ++++++++++++++++++++
 virt/kvm/arm/mmu.c                    |  4 ++++
 9 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index bc51b72fafd4..90e06a49f3e1 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -48,7 +48,8 @@
 #define ARM64_HAS_CACHE_IDC			27
 #define ARM64_HAS_CACHE_DIC			28
 #define ARM64_HW_DBM				29
+#define ARM64_HAS_STAGE2_FWB			30
 
-#define ARM64_NCAPS				30
+#define ARM64_NCAPS				31
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 6dd285e979c9..aa45df752a16 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -23,6 +23,7 @@
 #include <asm/types.h>
 
 /* Hyp Configuration Register (HCR) bits */
+#define HCR_FWB		(UL(1) << 46)
 #define HCR_TEA		(UL(1) << 37)
 #define HCR_TERR	(UL(1) << 36)
 #define HCR_TLOR	(UL(1) << 35)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 23b33e8ea03a..35e58da96602 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -63,6 +63,8 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 		/* trap error record accesses */
 		vcpu->arch.hcr_el2 |= HCR_TERR;
 	}
+	if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
+		vcpu->arch.hcr_el2 |= HCR_FWB;
 
 	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 082110993647..9dbca5355029 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -258,6 +258,7 @@ static inline bool kvm_page_empty(void *ptr)
 struct kvm;
 
 #define kvm_flush_dcache_to_poc(a,l)	__flush_dcache_area((a), (l))
+#define kvm_flush_dcache_to_pou(a,l)	__clean_dcache_area_pou((a), (l))
 
 static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 {
@@ -268,7 +269,10 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
 {
 	void *va = page_address(pfn_to_page(pfn));
 
-	kvm_flush_dcache_to_poc(va, size);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
+		kvm_flush_dcache_to_poc(va, size);
+	else
+		kvm_flush_dcache_to_pou(va, size);
 }
 
 static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
@@ -288,20 +292,26 @@ static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
 
 static inline void __kvm_flush_dcache_pte(pte_t pte)
 {
-	struct page *page = pte_page(pte);
-	kvm_flush_dcache_to_poc(page_address(page), PAGE_SIZE);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		struct page *page = pte_page(pte);
+		kvm_flush_dcache_to_poc(page_address(page), PAGE_SIZE);
+	}
 }
 
 static inline void __kvm_flush_dcache_pmd(pmd_t pmd)
 {
-	struct page *page = pmd_page(pmd);
-	kvm_flush_dcache_to_poc(page_address(page), PMD_SIZE);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		struct page *page = pmd_page(pmd);
+		kvm_flush_dcache_to_poc(page_address(page), PMD_SIZE);
+	}
 }
 
 static inline void __kvm_flush_dcache_pud(pud_t pud)
 {
-	struct page *page = pud_page(pud);
-	kvm_flush_dcache_to_poc(page_address(page), PUD_SIZE);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		struct page *page = pud_page(pud);
+		kvm_flush_dcache_to_poc(page_address(page), PUD_SIZE);
+	}
 }
 
 #define kvm_virt_to_phys(x)		__pa_symbol(x)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 49d99214f43c..b96442960aea 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -155,6 +155,13 @@
 #define MT_S2_NORMAL		0xf
 #define MT_S2_DEVICE_nGnRE	0x1
 
+/*
+ * Memory types for Stage-2 translation when ID_AA64MMFR2_EL1.FWB is 0001
+ * Stage-2 enforces Normal-WB and Device-nGnRE
+ */
+#define MT_S2_FWB_NORMAL	6
+#define MT_S2_FWB_DEVICE_nGnRE	1
+
 #ifdef CONFIG_ARM64_4K_PAGES
 #define IOREMAP_MAX_ORDER	(PUD_SHIFT)
 #else
diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index 108ecad7acc5..c66c3047400e 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -67,8 +67,18 @@
 #define PAGE_HYP_RO		__pgprot(_HYP_PAGE_DEFAULT | PTE_HYP | PTE_RDONLY | PTE_HYP_XN)
 #define PAGE_HYP_DEVICE		__pgprot(PROT_DEVICE_nGnRE | PTE_HYP)
 
-#define PAGE_S2			__pgprot(_PROT_DEFAULT | PTE_S2_MEMATTR(MT_S2_NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
-#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PTE_S2_MEMATTR(MT_S2_DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_MEMATTR(attr)						\
+	({								\
+		u64 __val;						\
+		if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))		\
+			__val = PTE_S2_MEMATTR(MT_S2_FWB_ ## attr);	\
+		else							\
+			__val = PTE_S2_MEMATTR(MT_S2_ ## attr);		\
+		__val;							\
+	 })
+
+#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
 
 #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)
 #define PAGE_SHARED		__pgprot(_PAGE_DEFAULT | PTE_USER | PTE_NG | PTE_PXN | PTE_UXN | PTE_WRITE)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 6171178075dc..385fb8c28981 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -576,6 +576,7 @@
 #define ID_AA64MMFR1_VMIDBITS_16	2
 
 /* id_aa64mmfr2 */
+#define ID_AA64MMFR2_FWB_SHIFT		40
 #define ID_AA64MMFR2_AT_SHIFT		32
 #define ID_AA64MMFR2_LVA_SHIFT		16
 #define ID_AA64MMFR2_IESB_SHIFT		12
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 536d572e5596..7d1794c0c832 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -192,6 +192,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_FWB_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_AT_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LVA_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_IESB_SHIFT, 4, 0),
@@ -1025,6 +1026,14 @@ static void cpu_copy_el2regs(const struct arm64_cpu_capabilities *__unused)
 }
 #endif
 
+static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
+{
+	u64 val = read_sysreg_s(SYS_CLIDR_EL1);
+
+	/* Check that CLIDR_EL1.LOU{U,IS} are both 0 */
+	WARN_ON(val & (7 << 27 | 7 << 21));
+}
+
 static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "GIC system register CPU interface",
@@ -1181,6 +1190,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
 		.matches = has_cache_dic,
 	},
+	{
+		.desc = "Stage-2 Force Write-Back",
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.capability = ARM64_HAS_STAGE2_FWB,
+		.sys_reg = SYS_ID_AA64MMFR2_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR2_FWB_SHIFT,
+		.min_field_value = 1,
+		.matches = has_cpuid_feature,
+		.cpu_enable = cpu_has_fwb,
+	},
 #ifdef CONFIG_ARM64_HW_AFDBM
 	{
 		/*
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 7f6a944db23d..ba66bf7ae299 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -196,6 +196,10 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr
  * This is why right after unmapping a page/section and invalidating
  * the corresponding TLBs, we call kvm_flush_dcache_p*() to make sure
  * the IO subsystem will never hit in the cache.
+ *
+ * This is all avoided on systems that have ARM64_HAS_STAGE2_FWB, as
+ * we then fully enforce cacheability of RAM, no matter what the guest
+ * does.
  */
 static void unmap_stage2_ptes(struct kvm *kvm, pmd_t *pmd,
 		       phys_addr_t addr, phys_addr_t end)
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 1/4] arm64: KVM: Add support for Stage-2 control of memory types and cacheability
@ 2018-05-17 10:35   ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

Up to ARMv8.3, the combinaison of Stage-1 and Stage-2 attributes
results in the strongest attribute of the two stages.  This means
that the hypervisor has to perform quite a lot of cache maintenance
just in case the guest has some non-cacheable mappings around.

ARMv8.4 solves this problem by offering a different mode (FWB) where
Stage-2 has total control over the memory attribute (this is limited
to systems where both I/O and instruction caches are coherent with
the dcache). This is achieved by having a different set of memory
attributes in the page tables, and a new bit set in HCR_EL2.

On such a system, we can then safely sidestep any form of dcache
management.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cpucaps.h      |  3 ++-
 arch/arm64/include/asm/kvm_arm.h      |  1 +
 arch/arm64/include/asm/kvm_emulate.h  |  2 ++
 arch/arm64/include/asm/kvm_mmu.h      | 24 +++++++++++++++++-------
 arch/arm64/include/asm/memory.h       |  7 +++++++
 arch/arm64/include/asm/pgtable-prot.h | 14 ++++++++++++--
 arch/arm64/include/asm/sysreg.h       |  1 +
 arch/arm64/kernel/cpufeature.c        | 20 ++++++++++++++++++++
 virt/kvm/arm/mmu.c                    |  4 ++++
 9 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index bc51b72fafd4..90e06a49f3e1 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -48,7 +48,8 @@
 #define ARM64_HAS_CACHE_IDC			27
 #define ARM64_HAS_CACHE_DIC			28
 #define ARM64_HW_DBM				29
+#define ARM64_HAS_STAGE2_FWB			30
 
-#define ARM64_NCAPS				30
+#define ARM64_NCAPS				31
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 6dd285e979c9..aa45df752a16 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -23,6 +23,7 @@
 #include <asm/types.h>
 
 /* Hyp Configuration Register (HCR) bits */
+#define HCR_FWB		(UL(1) << 46)
 #define HCR_TEA		(UL(1) << 37)
 #define HCR_TERR	(UL(1) << 36)
 #define HCR_TLOR	(UL(1) << 35)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 23b33e8ea03a..35e58da96602 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -63,6 +63,8 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 		/* trap error record accesses */
 		vcpu->arch.hcr_el2 |= HCR_TERR;
 	}
+	if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
+		vcpu->arch.hcr_el2 |= HCR_FWB;
 
 	if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 082110993647..9dbca5355029 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -258,6 +258,7 @@ static inline bool kvm_page_empty(void *ptr)
 struct kvm;
 
 #define kvm_flush_dcache_to_poc(a,l)	__flush_dcache_area((a), (l))
+#define kvm_flush_dcache_to_pou(a,l)	__clean_dcache_area_pou((a), (l))
 
 static inline bool vcpu_has_cache_enabled(struct kvm_vcpu *vcpu)
 {
@@ -268,7 +269,10 @@ static inline void __clean_dcache_guest_page(kvm_pfn_t pfn, unsigned long size)
 {
 	void *va = page_address(pfn_to_page(pfn));
 
-	kvm_flush_dcache_to_poc(va, size);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
+		kvm_flush_dcache_to_poc(va, size);
+	else
+		kvm_flush_dcache_to_pou(va, size);
 }
 
 static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
@@ -288,20 +292,26 @@ static inline void __invalidate_icache_guest_page(kvm_pfn_t pfn,
 
 static inline void __kvm_flush_dcache_pte(pte_t pte)
 {
-	struct page *page = pte_page(pte);
-	kvm_flush_dcache_to_poc(page_address(page), PAGE_SIZE);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		struct page *page = pte_page(pte);
+		kvm_flush_dcache_to_poc(page_address(page), PAGE_SIZE);
+	}
 }
 
 static inline void __kvm_flush_dcache_pmd(pmd_t pmd)
 {
-	struct page *page = pmd_page(pmd);
-	kvm_flush_dcache_to_poc(page_address(page), PMD_SIZE);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		struct page *page = pmd_page(pmd);
+		kvm_flush_dcache_to_poc(page_address(page), PMD_SIZE);
+	}
 }
 
 static inline void __kvm_flush_dcache_pud(pud_t pud)
 {
-	struct page *page = pud_page(pud);
-	kvm_flush_dcache_to_poc(page_address(page), PUD_SIZE);
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		struct page *page = pud_page(pud);
+		kvm_flush_dcache_to_poc(page_address(page), PUD_SIZE);
+	}
 }
 
 #define kvm_virt_to_phys(x)		__pa_symbol(x)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 49d99214f43c..b96442960aea 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -155,6 +155,13 @@
 #define MT_S2_NORMAL		0xf
 #define MT_S2_DEVICE_nGnRE	0x1
 
+/*
+ * Memory types for Stage-2 translation when ID_AA64MMFR2_EL1.FWB is 0001
+ * Stage-2 enforces Normal-WB and Device-nGnRE
+ */
+#define MT_S2_FWB_NORMAL	6
+#define MT_S2_FWB_DEVICE_nGnRE	1
+
 #ifdef CONFIG_ARM64_4K_PAGES
 #define IOREMAP_MAX_ORDER	(PUD_SHIFT)
 #else
diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index 108ecad7acc5..c66c3047400e 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -67,8 +67,18 @@
 #define PAGE_HYP_RO		__pgprot(_HYP_PAGE_DEFAULT | PTE_HYP | PTE_RDONLY | PTE_HYP_XN)
 #define PAGE_HYP_DEVICE		__pgprot(PROT_DEVICE_nGnRE | PTE_HYP)
 
-#define PAGE_S2			__pgprot(_PROT_DEFAULT | PTE_S2_MEMATTR(MT_S2_NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
-#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PTE_S2_MEMATTR(MT_S2_DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_MEMATTR(attr)						\
+	({								\
+		u64 __val;						\
+		if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))		\
+			__val = PTE_S2_MEMATTR(MT_S2_FWB_ ## attr);	\
+		else							\
+			__val = PTE_S2_MEMATTR(MT_S2_ ## attr);		\
+		__val;							\
+	 })
+
+#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
 
 #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)
 #define PAGE_SHARED		__pgprot(_PAGE_DEFAULT | PTE_USER | PTE_NG | PTE_PXN | PTE_UXN | PTE_WRITE)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 6171178075dc..385fb8c28981 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -576,6 +576,7 @@
 #define ID_AA64MMFR1_VMIDBITS_16	2
 
 /* id_aa64mmfr2 */
+#define ID_AA64MMFR2_FWB_SHIFT		40
 #define ID_AA64MMFR2_AT_SHIFT		32
 #define ID_AA64MMFR2_LVA_SHIFT		16
 #define ID_AA64MMFR2_IESB_SHIFT		12
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 536d572e5596..7d1794c0c832 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -192,6 +192,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_FWB_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_AT_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LVA_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_IESB_SHIFT, 4, 0),
@@ -1025,6 +1026,14 @@ static void cpu_copy_el2regs(const struct arm64_cpu_capabilities *__unused)
 }
 #endif
 
+static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
+{
+	u64 val = read_sysreg_s(SYS_CLIDR_EL1);
+
+	/* Check that CLIDR_EL1.LOU{U,IS} are both 0 */
+	WARN_ON(val & (7 << 27 | 7 << 21));
+}
+
 static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "GIC system register CPU interface",
@@ -1181,6 +1190,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
 		.matches = has_cache_dic,
 	},
+	{
+		.desc = "Stage-2 Force Write-Back",
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.capability = ARM64_HAS_STAGE2_FWB,
+		.sys_reg = SYS_ID_AA64MMFR2_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR2_FWB_SHIFT,
+		.min_field_value = 1,
+		.matches = has_cpuid_feature,
+		.cpu_enable = cpu_has_fwb,
+	},
 #ifdef CONFIG_ARM64_HW_AFDBM
 	{
 		/*
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 7f6a944db23d..ba66bf7ae299 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -196,6 +196,10 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr
  * This is why right after unmapping a page/section and invalidating
  * the corresponding TLBs, we call kvm_flush_dcache_p*() to make sure
  * the IO subsystem will never hit in the cache.
+ *
+ * This is all avoided on systems that have ARM64_HAS_STAGE2_FWB, as
+ * we then fully enforce cacheability of RAM, no matter what the guest
+ * does.
  */
 static void unmap_stage2_ptes(struct kvm *kvm, pmd_t *pmd,
 		       phys_addr_t addr, phys_addr_t end)
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/4] arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
  2018-05-17 10:35 ` Marc Zyngier
@ 2018-05-17 10:35   ` Marc Zyngier
  -1 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel; +Cc: Catalin Marinas

On systems where CTR_EL0.DIC is set, we don't need to perform
icache invalidation to guarantee that we'll fetch the right
instruction stream.

This also means that taking a permission fault to invalidate the
icache is an unnecessary overhead.

On such systems, we can safely leave the page as being executable.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/pgtable-prot.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index c66c3047400e..78b942c1bea4 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -77,8 +77,18 @@
 		__val;							\
 	 })
 
-#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
-#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_XN							\
+	({								\
+		u64 __val;						\
+		if (cpus_have_const_cap(ARM64_HAS_CACHE_DIC))		\
+			__val = 0;					\
+		else							\
+			__val = PTE_S2_XN;				\
+		__val;							\
+	})
+
+#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN)
+#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PAGE_S2_XN)
 
 #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)
 #define PAGE_SHARED		__pgprot(_PAGE_DEFAULT | PTE_USER | PTE_NG | PTE_PXN | PTE_UXN | PTE_WRITE)
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/4] arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
@ 2018-05-17 10:35   ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

On systems where CTR_EL0.DIC is set, we don't need to perform
icache invalidation to guarantee that we'll fetch the right
instruction stream.

This also means that taking a permission fault to invalidate the
icache is an unnecessary overhead.

On such systems, we can safely leave the page as being executable.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/pgtable-prot.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index c66c3047400e..78b942c1bea4 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -77,8 +77,18 @@
 		__val;							\
 	 })
 
-#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
-#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
+#define PAGE_S2_XN							\
+	({								\
+		u64 __val;						\
+		if (cpus_have_const_cap(ARM64_HAS_CACHE_DIC))		\
+			__val = 0;					\
+		else							\
+			__val = PTE_S2_XN;				\
+		__val;							\
+	})
+
+#define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN)
+#define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PAGE_S2_XN)
 
 #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)
 #define PAGE_SHARED		__pgprot(_PAGE_DEFAULT | PTE_USER | PTE_NG | PTE_PXN | PTE_UXN | PTE_WRITE)
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  2018-05-17 10:35 ` Marc Zyngier
@ 2018-05-17 10:35   ` Marc Zyngier
  -1 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel; +Cc: Catalin Marinas

There is no need to perform cache maintenance operations when
creating the HYP page tables if we have the multiprocessing
extensions. ARMv7 mandates them with the virtualization support,
and ARMv8 just mandates them unconditionally.

Let's remove these operations.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/mmu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index ba66bf7ae299..acbfea09578c 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
 		pte = pte_offset_kernel(pmd, addr);
 		kvm_set_pte(pte, pfn_pte(pfn, prot));
 		get_page(virt_to_page(pte));
-		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
 		pfn++;
 	} while (addr += PAGE_SIZE, addr != end);
 }
@@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
 			}
 			pmd_populate_kernel(NULL, pmd, pte);
 			get_page(virt_to_page(pmd));
-			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
 		}
 
 		next = pmd_addr_end(addr, end);
@@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
 			}
 			pud_populate(NULL, pud, pmd);
 			get_page(virt_to_page(pud));
-			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
 		}
 
 		next = pud_addr_end(addr, end);
@@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
 			}
 			pgd_populate(NULL, pgd, pud);
 			get_page(virt_to_page(pgd));
-			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
 		}
 
 		next = pgd_addr_end(addr, end);
@@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
 		pfn += (next - addr) >> PAGE_SHIFT;
 	} while (addr = next, addr != end);
 out:
+	dsb(ishst);
 	mutex_unlock(&kvm_hyp_pgd_mutex);
 	return err;
 }
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
@ 2018-05-17 10:35   ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

There is no need to perform cache maintenance operations when
creating the HYP page tables if we have the multiprocessing
extensions. ARMv7 mandates them with the virtualization support,
and ARMv8 just mandates them unconditionally.

Let's remove these operations.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/mmu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index ba66bf7ae299..acbfea09578c 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
 		pte = pte_offset_kernel(pmd, addr);
 		kvm_set_pte(pte, pfn_pte(pfn, prot));
 		get_page(virt_to_page(pte));
-		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
 		pfn++;
 	} while (addr += PAGE_SIZE, addr != end);
 }
@@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
 			}
 			pmd_populate_kernel(NULL, pmd, pte);
 			get_page(virt_to_page(pmd));
-			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
 		}
 
 		next = pmd_addr_end(addr, end);
@@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
 			}
 			pud_populate(NULL, pud, pmd);
 			get_page(virt_to_page(pud));
-			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
 		}
 
 		next = pud_addr_end(addr, end);
@@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
 			}
 			pgd_populate(NULL, pgd, pud);
 			get_page(virt_to_page(pgd));
-			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
 		}
 
 		next = pgd_addr_end(addr, end);
@@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
 		pfn += (next - addr) >> PAGE_SHIFT;
 	} while (addr = next, addr != end);
 out:
+	dsb(ishst);
 	mutex_unlock(&kvm_hyp_pgd_mutex);
 	return err;
 }
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/4] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present
  2018-05-17 10:35 ` Marc Zyngier
@ 2018-05-17 10:35   ` Marc Zyngier
  -1 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel; +Cc: Catalin Marinas, Christoffer Dall

Set/Way handling is one of the ugliest corners of KVM. We shouldn't
have to handle that, but better safe than sorry.

Thankfully, FWB fixes this for us by not requiering any maintenance
whatsoever, which means we don't have to emulate S/W CMOs, and don't
have to track VM ops either.

We still have to trap S/W though, if only to prevent the guest from
doing something bad.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 806b0b126a64..ef373963bf95 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -195,7 +195,13 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
 	if (!p->is_write)
 		return read_from_write_only(vcpu, p, r);
 
-	kvm_set_way_flush(vcpu);
+	/*
+	 * Only track S/W ops if we don't have FWB. It still indicates
+	 * that the guest is a bit broken...
+	 */
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
+		kvm_set_way_flush(vcpu);
+
 	return true;
 }
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/4] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present
@ 2018-05-17 10:35   ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-17 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

Set/Way handling is one of the ugliest corners of KVM. We shouldn't
have to handle that, but better safe than sorry.

Thankfully, FWB fixes this for us by not requiering any maintenance
whatsoever, which means we don't have to emulate S/W CMOs, and don't
have to track VM ops either.

We still have to trap S/W though, if only to prevent the guest from
doing something bad.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 806b0b126a64..ef373963bf95 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -195,7 +195,13 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
 	if (!p->is_write)
 		return read_from_write_only(vcpu, p, r);
 
-	kvm_set_way_flush(vcpu);
+	/*
+	 * Only track S/W ops if we don't have FWB. It still indicates
+	 * that the guest is a bit broken...
+	 */
+	if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
+		kvm_set_way_flush(vcpu);
+
 	return true;
 }
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  2018-05-17 10:35   ` Marc Zyngier
@ 2018-05-24 15:51     ` Catalin Marinas
  -1 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2018-05-24 15:51 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm, kvm, Christoffer Dall

On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
> There is no need to perform cache maintenance operations when
> creating the HYP page tables if we have the multiprocessing
> extensions. ARMv7 mandates them with the virtualization support,
> and ARMv8 just mandates them unconditionally.
> 
> Let's remove these operations.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  virt/kvm/arm/mmu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index ba66bf7ae299..acbfea09578c 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>  		pte = pte_offset_kernel(pmd, addr);
>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>  		get_page(virt_to_page(pte));
> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>  		pfn++;
>  	} while (addr += PAGE_SIZE, addr != end);
>  }
> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>  			}
>  			pmd_populate_kernel(NULL, pmd, pte);
>  			get_page(virt_to_page(pmd));
> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>  		}
>  
>  		next = pmd_addr_end(addr, end);
> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>  			}
>  			pud_populate(NULL, pud, pmd);
>  			get_page(virt_to_page(pud));
> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>  		}
>  
>  		next = pud_addr_end(addr, end);
> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  			}
>  			pgd_populate(NULL, pgd, pud);
>  			get_page(virt_to_page(pgd));
> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>  		}
>  
>  		next = pgd_addr_end(addr, end);
> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  		pfn += (next - addr) >> PAGE_SHIFT;
>  	} while (addr = next, addr != end);
>  out:
> +	dsb(ishst);
>  	mutex_unlock(&kvm_hyp_pgd_mutex);

Why do we need the DSB here? A comment would help.

If a DMB is sufficient, I think mutex_unlock has release semantics.

-- 
Catalin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
@ 2018-05-24 15:51     ` Catalin Marinas
  0 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2018-05-24 15:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
> There is no need to perform cache maintenance operations when
> creating the HYP page tables if we have the multiprocessing
> extensions. ARMv7 mandates them with the virtualization support,
> and ARMv8 just mandates them unconditionally.
> 
> Let's remove these operations.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  virt/kvm/arm/mmu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index ba66bf7ae299..acbfea09578c 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>  		pte = pte_offset_kernel(pmd, addr);
>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>  		get_page(virt_to_page(pte));
> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>  		pfn++;
>  	} while (addr += PAGE_SIZE, addr != end);
>  }
> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>  			}
>  			pmd_populate_kernel(NULL, pmd, pte);
>  			get_page(virt_to_page(pmd));
> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>  		}
>  
>  		next = pmd_addr_end(addr, end);
> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>  			}
>  			pud_populate(NULL, pud, pmd);
>  			get_page(virt_to_page(pud));
> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>  		}
>  
>  		next = pud_addr_end(addr, end);
> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  			}
>  			pgd_populate(NULL, pgd, pud);
>  			get_page(virt_to_page(pgd));
> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>  		}
>  
>  		next = pgd_addr_end(addr, end);
> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  		pfn += (next - addr) >> PAGE_SHIFT;
>  	} while (addr = next, addr != end);
>  out:
> +	dsb(ishst);
>  	mutex_unlock(&kvm_hyp_pgd_mutex);

Why do we need the DSB here? A comment would help.

If a DMB is sufficient, I think mutex_unlock has release semantics.

-- 
Catalin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/4] arm64: KVM: Add support for Stage-2 control of memory types and cacheability
  2018-05-17 10:35   ` Marc Zyngier
@ 2018-05-24 15:52     ` Catalin Marinas
  -1 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2018-05-24 15:52 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm, kvm, Christoffer Dall

On Thu, May 17, 2018 at 11:35:45AM +0100, Marc Zyngier wrote:
> Up to ARMv8.3, the combinaison of Stage-1 and Stage-2 attributes
> results in the strongest attribute of the two stages.  This means
> that the hypervisor has to perform quite a lot of cache maintenance
> just in case the guest has some non-cacheable mappings around.
> 
> ARMv8.4 solves this problem by offering a different mode (FWB) where
> Stage-2 has total control over the memory attribute (this is limited
> to systems where both I/O and instruction caches are coherent with
> the dcache). This is achieved by having a different set of memory
> attributes in the page tables, and a new bit set in HCR_EL2.
> 
> On such a system, we can then safely sidestep any form of dcache
> management.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

For the core arm64 bits:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/4] arm64: KVM: Add support for Stage-2 control of memory types and cacheability
@ 2018-05-24 15:52     ` Catalin Marinas
  0 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2018-05-24 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 17, 2018 at 11:35:45AM +0100, Marc Zyngier wrote:
> Up to ARMv8.3, the combinaison of Stage-1 and Stage-2 attributes
> results in the strongest attribute of the two stages.  This means
> that the hypervisor has to perform quite a lot of cache maintenance
> just in case the guest has some non-cacheable mappings around.
> 
> ARMv8.4 solves this problem by offering a different mode (FWB) where
> Stage-2 has total control over the memory attribute (this is limited
> to systems where both I/O and instruction caches are coherent with
> the dcache). This is achieved by having a different set of memory
> attributes in the page tables, and a new bit set in HCR_EL2.
> 
> On such a system, we can then safely sidestep any form of dcache
> management.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

For the core arm64 bits:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/4] arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
  2018-05-17 10:35   ` Marc Zyngier
@ 2018-05-24 15:52     ` Catalin Marinas
  -1 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2018-05-24 15:52 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm, kvm

On Thu, May 17, 2018 at 11:35:46AM +0100, Marc Zyngier wrote:
> On systems where CTR_EL0.DIC is set, we don't need to perform
> icache invalidation to guarantee that we'll fetch the right
> instruction stream.
> 
> This also means that taking a permission fault to invalidate the
> icache is an unnecessary overhead.
> 
> On such systems, we can safely leave the page as being executable.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/4] arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
@ 2018-05-24 15:52     ` Catalin Marinas
  0 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2018-05-24 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 17, 2018 at 11:35:46AM +0100, Marc Zyngier wrote:
> On systems where CTR_EL0.DIC is set, we don't need to perform
> icache invalidation to guarantee that we'll fetch the right
> instruction stream.
> 
> This also means that taking a permission fault to invalidate the
> icache is an unnecessary overhead.
> 
> On such systems, we can safely leave the page as being executable.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  2018-05-24 15:51     ` Catalin Marinas
@ 2018-05-24 16:36       ` Marc Zyngier
  -1 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-24 16:36 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: linux-arm-kernel, kvmarm, kvm

On 24/05/18 16:51, Catalin Marinas wrote:
> On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
>> There is no need to perform cache maintenance operations when
>> creating the HYP page tables if we have the multiprocessing
>> extensions. ARMv7 mandates them with the virtualization support,
>> and ARMv8 just mandates them unconditionally.
>>
>> Let's remove these operations.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index ba66bf7ae299..acbfea09578c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>>  		pte = pte_offset_kernel(pmd, addr);
>>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>>  		get_page(virt_to_page(pte));
>> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>>  		pfn++;
>>  	} while (addr += PAGE_SIZE, addr != end);
>>  }
>> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>>  			}
>>  			pmd_populate_kernel(NULL, pmd, pte);
>>  			get_page(virt_to_page(pmd));
>> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>>  		}
>>  
>>  		next = pmd_addr_end(addr, end);
>> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>>  			}
>>  			pud_populate(NULL, pud, pmd);
>>  			get_page(virt_to_page(pud));
>> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>>  		}
>>  
>>  		next = pud_addr_end(addr, end);
>> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  			}
>>  			pgd_populate(NULL, pgd, pud);
>>  			get_page(virt_to_page(pgd));
>> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>>  		}
>>  
>>  		next = pgd_addr_end(addr, end);
>> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  		pfn += (next - addr) >> PAGE_SHIFT;
>>  	} while (addr = next, addr != end);
>>  out:
>> +	dsb(ishst);
>>  	mutex_unlock(&kvm_hyp_pgd_mutex);
> 
> Why do we need the DSB here? A comment would help.

That's a combination of me being paranoid (we've just dropped a bunch of
CMOs that had DSBs there), and prior art (see dcadda146f4f -- adding
Mark who authored that change).

My understanding is also that we do need this as per K11.5.3 (ARMv8 ARM
rev C.a), which indicates that a DSB ISH "ensures visibility of the
update to translation table walks".

There is also this text:

<quote>

A translation table walk is considered to be a separate observer, and:
-  A write to the translation tables can be observed by that separate
observer at any time after the execution of the instruction that
performed that write, but is only guaranteed to be observable after the
execution of a DSB instruction by the PE that executed the instruction
that performed that write to the translation tables.

</quote>

I'm happy to add comment though.

> If a DMB is sufficient, I think mutex_unlock has release semantics.

I don't think it is, unfortunately.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
@ 2018-05-24 16:36       ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-24 16:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/05/18 16:51, Catalin Marinas wrote:
> On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
>> There is no need to perform cache maintenance operations when
>> creating the HYP page tables if we have the multiprocessing
>> extensions. ARMv7 mandates them with the virtualization support,
>> and ARMv8 just mandates them unconditionally.
>>
>> Let's remove these operations.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index ba66bf7ae299..acbfea09578c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>>  		pte = pte_offset_kernel(pmd, addr);
>>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>>  		get_page(virt_to_page(pte));
>> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>>  		pfn++;
>>  	} while (addr += PAGE_SIZE, addr != end);
>>  }
>> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>>  			}
>>  			pmd_populate_kernel(NULL, pmd, pte);
>>  			get_page(virt_to_page(pmd));
>> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>>  		}
>>  
>>  		next = pmd_addr_end(addr, end);
>> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>>  			}
>>  			pud_populate(NULL, pud, pmd);
>>  			get_page(virt_to_page(pud));
>> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>>  		}
>>  
>>  		next = pud_addr_end(addr, end);
>> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  			}
>>  			pgd_populate(NULL, pgd, pud);
>>  			get_page(virt_to_page(pgd));
>> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>>  		}
>>  
>>  		next = pgd_addr_end(addr, end);
>> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  		pfn += (next - addr) >> PAGE_SHIFT;
>>  	} while (addr = next, addr != end);
>>  out:
>> +	dsb(ishst);
>>  	mutex_unlock(&kvm_hyp_pgd_mutex);
> 
> Why do we need the DSB here? A comment would help.

That's a combination of me being paranoid (we've just dropped a bunch of
CMOs that had DSBs there), and prior art (see dcadda146f4f -- adding
Mark who authored that change).

My understanding is also that we do need this as per K11.5.3 (ARMv8 ARM
rev C.a), which indicates that a DSB ISH "ensures visibility of the
update to translation table walks".

There is also this text:

<quote>

A translation table walk is considered to be a separate observer, and:
-  A write to the translation tables can be observed by that separate
observer at any time after the execution of the instruction that
performed that write, but is only guaranteed to be observable after the
execution of a DSB instruction by the PE that executed the instruction
that performed that write to the translation tables.

</quote>

I'm happy to add comment though.

> If a DMB is sufficient, I think mutex_unlock has release semantics.

I don't think it is, unfortunately.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  2018-05-17 10:35   ` Marc Zyngier
@ 2018-05-24 17:12     ` Mark Rutland
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2018-05-24 17:12 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, kvmarm, kvm

On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
> There is no need to perform cache maintenance operations when
> creating the HYP page tables if we have the multiprocessing
> extensions. ARMv7 mandates them with the virtualization support,
> and ARMv8 just mandates them unconditionally.
> 
> Let's remove these operations.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  virt/kvm/arm/mmu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index ba66bf7ae299..acbfea09578c 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>  		pte = pte_offset_kernel(pmd, addr);
>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>  		get_page(virt_to_page(pte));
> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>  		pfn++;
>  	} while (addr += PAGE_SIZE, addr != end);
>  }
> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>  			}
>  			pmd_populate_kernel(NULL, pmd, pte);
>  			get_page(virt_to_page(pmd));
> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>  		}
>  
>  		next = pmd_addr_end(addr, end);
> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>  			}
>  			pud_populate(NULL, pud, pmd);
>  			get_page(virt_to_page(pud));
> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>  		}
>  
>  		next = pud_addr_end(addr, end);
> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  			}
>  			pgd_populate(NULL, pgd, pud);
>  			get_page(virt_to_page(pgd));
> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>  		}
>  
>  		next = pgd_addr_end(addr, end);
> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  		pfn += (next - addr) >> PAGE_SHIFT;
>  	} while (addr = next, addr != end);
>  out:
> +	dsb(ishst);

I think you need a dsb(ishst) wherever you had a
kvm_flush_dcache_to_poc() previously.

Otherwise, the page table walker could see stale values. e.g. after you
update a bunch of PTE entries and point a PMD entry at those, the PTW
could see the updated PMD entry, but see stale PTE entries, which could
be garbage.

That said, I think for ensuring the *order* those become visible in, you
only need a dmb(ishst), and the ensure they *are visible* you need a
DSB(ISHST) at the end.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
@ 2018-05-24 17:12     ` Mark Rutland
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2018-05-24 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
> There is no need to perform cache maintenance operations when
> creating the HYP page tables if we have the multiprocessing
> extensions. ARMv7 mandates them with the virtualization support,
> and ARMv8 just mandates them unconditionally.
> 
> Let's remove these operations.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  virt/kvm/arm/mmu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index ba66bf7ae299..acbfea09578c 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>  		pte = pte_offset_kernel(pmd, addr);
>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>  		get_page(virt_to_page(pte));
> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>  		pfn++;
>  	} while (addr += PAGE_SIZE, addr != end);
>  }
> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>  			}
>  			pmd_populate_kernel(NULL, pmd, pte);
>  			get_page(virt_to_page(pmd));
> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>  		}
>  
>  		next = pmd_addr_end(addr, end);
> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>  			}
>  			pud_populate(NULL, pud, pmd);
>  			get_page(virt_to_page(pud));
> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>  		}
>  
>  		next = pud_addr_end(addr, end);
> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  			}
>  			pgd_populate(NULL, pgd, pud);
>  			get_page(virt_to_page(pgd));
> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>  		}
>  
>  		next = pgd_addr_end(addr, end);
> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>  		pfn += (next - addr) >> PAGE_SHIFT;
>  	} while (addr = next, addr != end);
>  out:
> +	dsb(ishst);

I think you need a dsb(ishst) wherever you had a
kvm_flush_dcache_to_poc() previously.

Otherwise, the page table walker could see stale values. e.g. after you
update a bunch of PTE entries and point a PMD entry at those, the PTW
could see the updated PMD entry, but see stale PTE entries, which could
be garbage.

That said, I think for ensuring the *order* those become visible in, you
only need a dmb(ishst), and the ensure they *are visible* you need a
DSB(ISHST) at the end.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
  2018-05-24 17:12     ` Mark Rutland
@ 2018-05-25  8:23       ` Marc Zyngier
  -1 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-25  8:23 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, kvmarm, kvm

On 24/05/18 18:12, Mark Rutland wrote:
> On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
>> There is no need to perform cache maintenance operations when
>> creating the HYP page tables if we have the multiprocessing
>> extensions. ARMv7 mandates them with the virtualization support,
>> and ARMv8 just mandates them unconditionally.
>>
>> Let's remove these operations.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index ba66bf7ae299..acbfea09578c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>>  		pte = pte_offset_kernel(pmd, addr);
>>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>>  		get_page(virt_to_page(pte));
>> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>>  		pfn++;
>>  	} while (addr += PAGE_SIZE, addr != end);
>>  }
>> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>>  			}
>>  			pmd_populate_kernel(NULL, pmd, pte);
>>  			get_page(virt_to_page(pmd));
>> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>>  		}
>>  
>>  		next = pmd_addr_end(addr, end);
>> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>>  			}
>>  			pud_populate(NULL, pud, pmd);
>>  			get_page(virt_to_page(pud));
>> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>>  		}
>>  
>>  		next = pud_addr_end(addr, end);
>> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  			}
>>  			pgd_populate(NULL, pgd, pud);
>>  			get_page(virt_to_page(pgd));
>> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>>  		}
>>  
>>  		next = pgd_addr_end(addr, end);
>> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  		pfn += (next - addr) >> PAGE_SHIFT;
>>  	} while (addr = next, addr != end);
>>  out:
>> +	dsb(ishst);
> 
> I think you need a dsb(ishst) wherever you had a
> kvm_flush_dcache_to_poc() previously.
> 
> Otherwise, the page table walker could see stale values. e.g. after you
> update a bunch of PTE entries and point a PMD entry at those, the PTW
> could see the updated PMD entry, but see stale PTE entries, which could
> be garbage.

That's a very good point. I initially only considered the EL2
initialisation (the EL2 MMU is not live yet), but failed to consider the
additional mappings at runtime (each time we instantiate a VM).

> That said, I think for ensuring the *order* those become visible in, you
> only need a dmb(ishst), and the ensure they *are visible* you need a
> DSB(ISHST) at the end.
And we definitely want the latter.

I'll respin this shortly.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
@ 2018-05-25  8:23       ` Marc Zyngier
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Zyngier @ 2018-05-25  8:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/05/18 18:12, Mark Rutland wrote:
> On Thu, May 17, 2018 at 11:35:47AM +0100, Marc Zyngier wrote:
>> There is no need to perform cache maintenance operations when
>> creating the HYP page tables if we have the multiprocessing
>> extensions. ARMv7 mandates them with the virtualization support,
>> and ARMv8 just mandates them unconditionally.
>>
>> Let's remove these operations.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  virt/kvm/arm/mmu.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index ba66bf7ae299..acbfea09578c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -578,7 +578,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
>>  		pte = pte_offset_kernel(pmd, addr);
>>  		kvm_set_pte(pte, pfn_pte(pfn, prot));
>>  		get_page(virt_to_page(pte));
>> -		kvm_flush_dcache_to_poc(pte, sizeof(*pte));
>>  		pfn++;
>>  	} while (addr += PAGE_SIZE, addr != end);
>>  }
>> @@ -605,7 +604,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
>>  			}
>>  			pmd_populate_kernel(NULL, pmd, pte);
>>  			get_page(virt_to_page(pmd));
>> -			kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
>>  		}
>>  
>>  		next = pmd_addr_end(addr, end);
>> @@ -638,7 +636,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
>>  			}
>>  			pud_populate(NULL, pud, pmd);
>>  			get_page(virt_to_page(pud));
>> -			kvm_flush_dcache_to_poc(pud, sizeof(*pud));
>>  		}
>>  
>>  		next = pud_addr_end(addr, end);
>> @@ -675,7 +672,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  			}
>>  			pgd_populate(NULL, pgd, pud);
>>  			get_page(virt_to_page(pgd));
>> -			kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
>>  		}
>>  
>>  		next = pgd_addr_end(addr, end);
>> @@ -685,6 +681,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
>>  		pfn += (next - addr) >> PAGE_SHIFT;
>>  	} while (addr = next, addr != end);
>>  out:
>> +	dsb(ishst);
> 
> I think you need a dsb(ishst) wherever you had a
> kvm_flush_dcache_to_poc() previously.
> 
> Otherwise, the page table walker could see stale values. e.g. after you
> update a bunch of PTE entries and point a PMD entry at those, the PTW
> could see the updated PMD entry, but see stale PTE entries, which could
> be garbage.

That's a very good point. I initially only considered the EL2
initialisation (the EL2 MMU is not live yet), but failed to consider the
additional mappings at runtime (each time we instantiate a VM).

> That said, I think for ensuring the *order* those become visible in, you
> only need a dmb(ishst), and the ensure they *are visible* you need a
> DSB(ISHST) at the end.
And we definitely want the latter.

I'll respin this shortly.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-05-25  8:23 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-17 10:35 [PATCH 0/4] KVM/arm64: Cache maintenance relaxations Marc Zyngier
2018-05-17 10:35 ` Marc Zyngier
2018-05-17 10:35 ` [PATCH 1/4] arm64: KVM: Add support for Stage-2 control of memory types and cacheability Marc Zyngier
2018-05-17 10:35   ` Marc Zyngier
2018-05-24 15:52   ` Catalin Marinas
2018-05-24 15:52     ` Catalin Marinas
2018-05-17 10:35 ` [PATCH 2/4] arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set Marc Zyngier
2018-05-17 10:35   ` Marc Zyngier
2018-05-24 15:52   ` Catalin Marinas
2018-05-24 15:52     ` Catalin Marinas
2018-05-17 10:35 ` [PATCH 3/4] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables Marc Zyngier
2018-05-17 10:35   ` Marc Zyngier
2018-05-24 15:51   ` Catalin Marinas
2018-05-24 15:51     ` Catalin Marinas
2018-05-24 16:36     ` Marc Zyngier
2018-05-24 16:36       ` Marc Zyngier
2018-05-24 17:12   ` Mark Rutland
2018-05-24 17:12     ` Mark Rutland
2018-05-25  8:23     ` Marc Zyngier
2018-05-25  8:23       ` Marc Zyngier
2018-05-17 10:35 ` [PATCH 4/4] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present Marc Zyngier
2018-05-17 10:35   ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.