[PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[Marc Zyngier]

ARMv8.2 introduced support for VPIPT i-caches, the V standing for
VMID-tagged. Although this looked like a reasonable idea, no
implementation has ever made it into the wild.

Linux has supported this for over 6 years (amusingly, just as the
architecture was dropping support for AIVIVT i-caches), but we had no
way to even test it, and it is likely that this code was just
bit-rotting.

However, in a recent breakthrough (XML drop 2023-09, tagged as
d55f5af8e09052abe92a02adf820deea2eaed717), the architecture has
finally been purged of this option, making VIPT and PIPT the only two
valid options.

This really means this code is just dead code. Nobody will ever come
up with such an implementation, and we can just get rid of it.

Most of the impact is on KVM, where we drop a few large comment blocks
(and a bit of code), while the core arch code loses the detection code
itself.

* From v2:
  - Fix reserved naming for RESERVED_AIVIVT
  - Collected RBs from Anshuman an Zenghui

Marc Zyngier (3):
  KVM: arm64: Remove VPIPT I-cache handling
  arm64: Kill detection of VPIPT i-cache policy
  arm64: Rename reserved values for CTR_EL0.L1Ip

 arch/arm64/include/asm/cache.h   |  6 ----
 arch/arm64/include/asm/kvm_mmu.h |  7 ----
 arch/arm64/kernel/cpuinfo.c      |  5 ---
 arch/arm64/kvm/hyp/nvhe/pkvm.c   |  2 +-
 arch/arm64/kvm/hyp/nvhe/tlb.c    | 61 --------------------------------
 arch/arm64/kvm/hyp/vhe/tlb.c     | 13 -------
 arch/arm64/tools/sysreg          |  5 +--
 7 files changed, 4 insertions(+), 95 deletions(-)

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/3] KVM: arm64: Remove VPIPT I-cache handling

[Marc Zyngier]

We have some special handling for VPIPT I-cache in critical parts
of the cache and TLB maintenance. Remove it.

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_mmu.h |  7 ----
 arch/arm64/kvm/hyp/nvhe/pkvm.c   |  2 +-
 arch/arm64/kvm/hyp/nvhe/tlb.c    | 61 --------------------------------
 arch/arm64/kvm/hyp/vhe/tlb.c     | 13 -------
 4 files changed, 1 insertion(+), 82 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 49e0d4b36bd0..e3e793d0ec30 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -243,13 +243,6 @@ static inline size_t __invalidate_icache_max_range(void)
 
 static inline void __invalidate_icache_guest_page(void *va, size_t size)
 {
-	/*
-	 * VPIPT I-cache maintenance must be done from EL2. See comment in the
-	 * nVHE flavor of __kvm_tlb_flush_vmid_ipa().
-	 */
-	if (icache_is_vpipt() && read_sysreg(CurrentEL) != CurrentEL_EL2)
-		return;
-
 	/*
 	 * Blow the whole I-cache if it is aliasing (i.e. VIPT) or the
 	 * invalidation range exceeds our arbitrary limit on invadations by
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 9d23a51d7f75..b29f15418c0a 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -12,7 +12,7 @@
 #include <nvhe/pkvm.h>
 #include <nvhe/trap_handler.h>
 
-/* Used by icache_is_vpipt(). */
+/* Used by icache_is_aliasing(). */
 unsigned long __icache_flags;
 
 /* Used by kvm_get_vttbr(). */
diff --git a/arch/arm64/kvm/hyp/nvhe/tlb.c b/arch/arm64/kvm/hyp/nvhe/tlb.c
index 1b265713d6be..a60fb13e2192 100644
--- a/arch/arm64/kvm/hyp/nvhe/tlb.c
+++ b/arch/arm64/kvm/hyp/nvhe/tlb.c
@@ -105,28 +105,6 @@ void __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu,
 	dsb(ish);
 	isb();
 
-	/*
-	 * If the host is running at EL1 and we have a VPIPT I-cache,
-	 * then we must perform I-cache maintenance at EL2 in order for
-	 * it to have an effect on the guest. Since the guest cannot hit
-	 * I-cache lines allocated with a different VMID, we don't need
-	 * to worry about junk out of guest reset (we nuke the I-cache on
-	 * VMID rollover), but we do need to be careful when remapping
-	 * executable pages for the same guest. This can happen when KSM
-	 * takes a CoW fault on an executable page, copies the page into
-	 * a page that was previously mapped in the guest and then needs
-	 * to invalidate the guest view of the I-cache for that page
-	 * from EL1. To solve this, we invalidate the entire I-cache when
-	 * unmapping a page from a guest if we have a VPIPT I-cache but
-	 * the host is running at EL1. As above, we could do better if
-	 * we had the VA.
-	 *
-	 * The moral of this story is: if you have a VPIPT I-cache, then
-	 * you should be running with VHE enabled.
-	 */
-	if (icache_is_vpipt())
-		icache_inval_all_pou();
-
 	__tlb_switch_to_host(&cxt);
 }
 
@@ -157,28 +135,6 @@ void __kvm_tlb_flush_vmid_ipa_nsh(struct kvm_s2_mmu *mmu,
 	dsb(nsh);
 	isb();
 
-	/*
-	 * If the host is running at EL1 and we have a VPIPT I-cache,
-	 * then we must perform I-cache maintenance at EL2 in order for
-	 * it to have an effect on the guest. Since the guest cannot hit
-	 * I-cache lines allocated with a different VMID, we don't need
-	 * to worry about junk out of guest reset (we nuke the I-cache on
-	 * VMID rollover), but we do need to be careful when remapping
-	 * executable pages for the same guest. This can happen when KSM
-	 * takes a CoW fault on an executable page, copies the page into
-	 * a page that was previously mapped in the guest and then needs
-	 * to invalidate the guest view of the I-cache for that page
-	 * from EL1. To solve this, we invalidate the entire I-cache when
-	 * unmapping a page from a guest if we have a VPIPT I-cache but
-	 * the host is running at EL1. As above, we could do better if
-	 * we had the VA.
-	 *
-	 * The moral of this story is: if you have a VPIPT I-cache, then
-	 * you should be running with VHE enabled.
-	 */
-	if (icache_is_vpipt())
-		icache_inval_all_pou();
-
 	__tlb_switch_to_host(&cxt);
 }
 
@@ -205,10 +161,6 @@ void __kvm_tlb_flush_vmid_range(struct kvm_s2_mmu *mmu,
 	dsb(ish);
 	isb();
 
-	/* See the comment in __kvm_tlb_flush_vmid_ipa() */
-	if (icache_is_vpipt())
-		icache_inval_all_pou();
-
 	__tlb_switch_to_host(&cxt);
 }
 
@@ -246,18 +198,5 @@ void __kvm_flush_vm_context(void)
 	/* Same remark as in __tlb_switch_to_guest() */
 	dsb(ish);
 	__tlbi(alle1is);
-
-	/*
-	 * VIPT and PIPT caches are not affected by VMID, so no maintenance
-	 * is necessary across a VMID rollover.
-	 *
-	 * VPIPT caches constrain lookup and maintenance to the active VMID,
-	 * so we need to invalidate lines with a stale VMID to avoid an ABA
-	 * race after multiple rollovers.
-	 *
-	 */
-	if (icache_is_vpipt())
-		asm volatile("ic ialluis");
-
 	dsb(ish);
 }
diff --git a/arch/arm64/kvm/hyp/vhe/tlb.c b/arch/arm64/kvm/hyp/vhe/tlb.c
index b636b4111dbf..b32e2940df7d 100644
--- a/arch/arm64/kvm/hyp/vhe/tlb.c
+++ b/arch/arm64/kvm/hyp/vhe/tlb.c
@@ -216,18 +216,5 @@ void __kvm_flush_vm_context(void)
 {
 	dsb(ishst);
 	__tlbi(alle1is);
-
-	/*
-	 * VIPT and PIPT caches are not affected by VMID, so no maintenance
-	 * is necessary across a VMID rollover.
-	 *
-	 * VPIPT caches constrain lookup and maintenance to the active VMID,
-	 * so we need to invalidate lines with a stale VMID to avoid an ABA
-	 * race after multiple rollovers.
-	 *
-	 */
-	if (icache_is_vpipt())
-		asm volatile("ic ialluis");
-
 	dsb(ish);
 }
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/3] arm64: Kill detection of VPIPT i-cache policy

[Marc Zyngier]

Since the kernel will never run on a system with the VPIPT i-cache
policy, drop the detection code altogether.

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/cache.h | 6 ------
 arch/arm64/kernel/cpuinfo.c    | 5 -----
 2 files changed, 11 deletions(-)

diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index ceb368d33bf4..06a4670bdb0b 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -58,7 +58,6 @@ static inline unsigned int arch_slab_minalign(void)
 #define CTR_L1IP(ctr)		SYS_FIELD_GET(CTR_EL0, L1Ip, ctr)
 
 #define ICACHEF_ALIASING	0
-#define ICACHEF_VPIPT		1
 extern unsigned long __icache_flags;
 
 /*
@@ -70,11 +69,6 @@ static inline int icache_is_aliasing(void)
 	return test_bit(ICACHEF_ALIASING, &__icache_flags);
 }
 
-static __always_inline int icache_is_vpipt(void)
-{
-	return test_bit(ICACHEF_VPIPT, &__icache_flags);
-}
-
 static inline u32 cache_type_cwg(void)
 {
 	return SYS_FIELD_GET(CTR_EL0, CWG, read_cpuid_cachetype());
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index a257da7b56fe..47043c0d95ec 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -36,8 +36,6 @@ static struct cpuinfo_arm64 boot_cpu_data;
 static inline const char *icache_policy_str(int l1ip)
 {
 	switch (l1ip) {
-	case CTR_EL0_L1Ip_VPIPT:
-		return "VPIPT";
 	case CTR_EL0_L1Ip_VIPT:
 		return "VIPT";
 	case CTR_EL0_L1Ip_PIPT:
@@ -388,9 +386,6 @@ static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
 	switch (l1ip) {
 	case CTR_EL0_L1Ip_PIPT:
 		break;
-	case CTR_EL0_L1Ip_VPIPT:
-		set_bit(ICACHEF_VPIPT, &__icache_flags);
-		break;
 	case CTR_EL0_L1Ip_VIPT:
 	default:
 		/* Assume aliasing */
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/3] arm64: Rename reserved values for CTR_EL0.L1Ip

[Marc Zyngier]

We now have *two* values for CTR_EL0.L1Ip that are reserved.
Which makes things a bit awkward. In order to lift the ambiguity,
rename RESERVED (0b01) to RESERVED_AIVIVT, and VPIPT (0b00) to
RESERVED_VPIPT.

This makes it clear which of these meant what, and I'm sure
archeologists will find it useful...

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/tools/sysreg | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 96cbeeab4eec..c5af75b23187 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2004,9 +2004,10 @@ Field	27:24	CWG
 Field	23:20	ERG
 Field	19:16	DminLine
 Enum	15:14	L1Ip
-	0b00	VPIPT
+	# This was named as VPIPT in the ARM but now documented as reserved
+	0b00	RESERVED_VPIPT
 	# This is named as AIVIVT in the ARM but documented as reserved
-	0b01	RESERVED
+	0b01	RESERVED_AIVIVT
 	0b10	VIPT
 	0b11	PIPT
 EndEnum
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[Mark Rutland]

On Mon, Dec 04, 2023 at 02:36:03PM +0000, Marc Zyngier wrote:
> ARMv8.2 introduced support for VPIPT i-caches, the V standing for
> VMID-tagged. Although this looked like a reasonable idea, no
> implementation has ever made it into the wild.
> 
> Linux has supported this for over 6 years (amusingly, just as the
> architecture was dropping support for AIVIVT i-caches), but we had no
> way to even test it, and it is likely that this code was just
> bit-rotting.
> 
> However, in a recent breakthrough (XML drop 2023-09, tagged as
> d55f5af8e09052abe92a02adf820deea2eaed717), the architecture has
> finally been purged of this option, making VIPT and PIPT the only two
> valid options.
> 
> This really means this code is just dead code. Nobody will ever come
> up with such an implementation, and we can just get rid of it.
> 
> Most of the impact is on KVM, where we drop a few large comment blocks
> (and a bit of code), while the core arch code loses the detection code
> itself.
> 
> * From v2:
>   - Fix reserved naming for RESERVED_AIVIVT
>   - Collected RBs from Anshuman an Zenghui
> 
> Marc Zyngier (3):
>   KVM: arm64: Remove VPIPT I-cache handling
>   arm64: Kill detection of VPIPT i-cache policy
>   arm64: Rename reserved values for CTR_EL0.L1Ip

For the series:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Looking forward, we can/should probably replace __icache_flags with a single
ICACHE_NOALIASING or ICACHE_PIPT cpucap, which'd get rid of a bunch of
duplicated logic and make that more sound in the case of races around cpu
onlining.

Mark.

> 
>  arch/arm64/include/asm/cache.h   |  6 ----
>  arch/arm64/include/asm/kvm_mmu.h |  7 ----
>  arch/arm64/kernel/cpuinfo.c      |  5 ---
>  arch/arm64/kvm/hyp/nvhe/pkvm.c   |  2 +-
>  arch/arm64/kvm/hyp/nvhe/tlb.c    | 61 --------------------------------
>  arch/arm64/kvm/hyp/vhe/tlb.c     | 13 -------
>  arch/arm64/tools/sysreg          |  5 +--
>  7 files changed, 4 insertions(+), 95 deletions(-)
> 
> -- 
> 2.39.2
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[Marc Zyngier]

On Mon, 04 Dec 2023 14:44:56 +0000,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Mon, Dec 04, 2023 at 02:36:03PM +0000, Marc Zyngier wrote:
> > ARMv8.2 introduced support for VPIPT i-caches, the V standing for
> > VMID-tagged. Although this looked like a reasonable idea, no
> > implementation has ever made it into the wild.
> > 
> > Linux has supported this for over 6 years (amusingly, just as the
> > architecture was dropping support for AIVIVT i-caches), but we had no
> > way to even test it, and it is likely that this code was just
> > bit-rotting.
> > 
> > However, in a recent breakthrough (XML drop 2023-09, tagged as
> > d55f5af8e09052abe92a02adf820deea2eaed717), the architecture has
> > finally been purged of this option, making VIPT and PIPT the only two
> > valid options.
> > 
> > This really means this code is just dead code. Nobody will ever come
> > up with such an implementation, and we can just get rid of it.
> > 
> > Most of the impact is on KVM, where we drop a few large comment blocks
> > (and a bit of code), while the core arch code loses the detection code
> > itself.
> > 
> > * From v2:
> >   - Fix reserved naming for RESERVED_AIVIVT
> >   - Collected RBs from Anshuman an Zenghui
> > 
> > Marc Zyngier (3):
> >   KVM: arm64: Remove VPIPT I-cache handling
> >   arm64: Kill detection of VPIPT i-cache policy
> >   arm64: Rename reserved values for CTR_EL0.L1Ip
> 
> For the series:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks.

> Looking forward, we can/should probably replace __icache_flags with a single
> ICACHE_NOALIASING or ICACHE_PIPT cpucap, which'd get rid of a bunch of
> duplicated logic and make that more sound in the case of races around cpu
> onlining.

As long as we refuse VIPT CPUs coming up late (i.e. after we have
patched the kernel to set ICACHE_PIPT), it should be doable. I guess
we already have this restriction as userspace is able to probe the
I-cache policy anyway.

How about the patch below (tested in a guest with a bunch of hacks to
expose different L1Ip values)?

Thanks,

	M.

From 8f88afb0b317213dcbf18ea460a508bfccc18568 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Mon, 4 Dec 2023 18:09:40 +0000
Subject: [PATCH] arm64: Make icache detection a cpu capability

Now that we only have two icache policies, we are in a good position
to make the whole detection business more robust.

Let's replace __icache_flags by a single capability (ICACHE_PIPT),
and use this if all CPUs are indeed PIPT. It means we can rely on
existing logic to mandate that a VIPT CPU coming up late will be
denied booting, which is the safe thing to do.

This also leads to some nice cleanups in pKVM, and KVM as a whole
can use ARM64_ICACHE_PIPT as a final cap.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/cache.h   |  9 ++-------
 arch/arm64/include/asm/kvm_hyp.h |  1 -
 arch/arm64/include/asm/kvm_mmu.h |  2 +-
 arch/arm64/kernel/cpufeature.c   |  7 +++++++
 arch/arm64/kernel/cpuinfo.c      | 34 --------------------------------
 arch/arm64/kvm/arm.c             |  1 -
 arch/arm64/kvm/hyp/nvhe/pkvm.c   |  3 ---
 arch/arm64/tools/cpucaps         |  1 +
 arch/arm64/tools/sysreg          |  2 +-
 9 files changed, 12 insertions(+), 48 deletions(-)

diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index 06a4670bdb0b..8ef9522a6151 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -37,9 +37,9 @@
 
 #ifndef __ASSEMBLY__
 
-#include <linux/bitops.h>
 #include <linux/kasan-enabled.h>
 
+#include <asm/cpufeature.h>
 #include <asm/cputype.h>
 #include <asm/mte-def.h>
 #include <asm/sysreg.h>
@@ -55,18 +55,13 @@ static inline unsigned int arch_slab_minalign(void)
 #define arch_slab_minalign() arch_slab_minalign()
 #endif
 
-#define CTR_L1IP(ctr)		SYS_FIELD_GET(CTR_EL0, L1Ip, ctr)
-
-#define ICACHEF_ALIASING	0
-extern unsigned long __icache_flags;
-
 /*
  * Whilst the D-side always behaves as PIPT on AArch64, aliasing is
  * permitted in the I-cache.
  */
 static inline int icache_is_aliasing(void)
 {
-	return test_bit(ICACHEF_ALIASING, &__icache_flags);
+	return !cpus_have_cap(ARM64_ICACHE_PIPT);
 }
 
 static inline u32 cache_type_cwg(void)
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 145ce73fc16c..7ad70f9865fd 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -140,7 +140,6 @@ extern u64 kvm_nvhe_sym(id_aa64mmfr1_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64mmfr2_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64smfr0_el1_sys_val);
 
-extern unsigned long kvm_nvhe_sym(__icache_flags);
 extern unsigned int kvm_nvhe_sym(kvm_arm_vmid_bits);
 
 #endif /* __ARM64_KVM_HYP_H__ */
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index e3e793d0ec30..ab86c2f025cf 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -248,7 +248,7 @@ static inline void __invalidate_icache_guest_page(void *va, size_t size)
 	 * invalidation range exceeds our arbitrary limit on invadations by
 	 * cache line.
 	 */
-	if (icache_is_aliasing() || size > __invalidate_icache_max_range())
+	if (!cpus_have_final_cap(ARM64_ICACHE_PIPT) || size > __invalidate_icache_max_range())
 		icache_inval_all_pou();
 	else
 		icache_inval_pou((unsigned long)va, (unsigned long)va + size);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 646591c67e7a..73a37176676e 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2272,6 +2272,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
 		.matches = has_always,
 	},
+	{
+		.desc = "PIPT I-cache policy",
+		.capability = ARM64_ICACHE_PIPT,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_cpuid_feature,
+		ARM64_CPUID_FIELDS(CTR_EL0, L1Ip, PIPT)
+	},
 	{
 		.desc = "GIC system register CPU interface",
 		.capability = ARM64_HAS_GIC_CPUIF_SYSREGS,
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 47043c0d95ec..a4ea331fb6d4 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -33,20 +33,6 @@
 DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
 static struct cpuinfo_arm64 boot_cpu_data;
 
-static inline const char *icache_policy_str(int l1ip)
-{
-	switch (l1ip) {
-	case CTR_EL0_L1Ip_VIPT:
-		return "VIPT";
-	case CTR_EL0_L1Ip_PIPT:
-		return "PIPT";
-	default:
-		return "RESERVED/UNKNOWN";
-	}
-}
-
-unsigned long __icache_flags;
-
 static const char *const hwcap_str[] = {
 	[KERNEL_HWCAP_FP]		= "fp",
 	[KERNEL_HWCAP_ASIMD]		= "asimd",
@@ -378,24 +364,6 @@ static int __init cpuinfo_regs_init(void)
 }
 device_initcall(cpuinfo_regs_init);
 
-static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
-{
-	unsigned int cpu = smp_processor_id();
-	u32 l1ip = CTR_L1IP(info->reg_ctr);
-
-	switch (l1ip) {
-	case CTR_EL0_L1Ip_PIPT:
-		break;
-	case CTR_EL0_L1Ip_VIPT:
-	default:
-		/* Assume aliasing */
-		set_bit(ICACHEF_ALIASING, &__icache_flags);
-		break;
-	}
-
-	pr_info("Detected %s I-cache on CPU%d\n", icache_policy_str(l1ip), cpu);
-}
-
 static void __cpuinfo_store_cpu_32bit(struct cpuinfo_32bit *info)
 {
 	info->reg_id_dfr0 = read_cpuid(ID_DFR0_EL1);
@@ -457,8 +425,6 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 
 	if (id_aa64pfr0_32bit_el0(info->reg_id_aa64pfr0))
 		__cpuinfo_store_cpu_32bit(&info->aarch32);
-
-	cpuinfo_detect_icache_policy(info);
 }
 
 void cpuinfo_store_cpu(void)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e5f75f1f1085..a5f71165cd5c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -2242,7 +2242,6 @@ static void kvm_hyp_init_symbols(void)
 	kvm_nvhe_sym(id_aa64mmfr1_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
 	kvm_nvhe_sym(id_aa64mmfr2_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64MMFR2_EL1);
 	kvm_nvhe_sym(id_aa64smfr0_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64SMFR0_EL1);
-	kvm_nvhe_sym(__icache_flags) = __icache_flags;
 	kvm_nvhe_sym(kvm_arm_vmid_bits) = kvm_arm_vmid_bits;
 }
 
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index b29f15418c0a..187ce5720697 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -12,9 +12,6 @@
 #include <nvhe/pkvm.h>
 #include <nvhe/trap_handler.h>
 
-/* Used by icache_is_aliasing(). */
-unsigned long __icache_flags;
-
 /* Used by kvm_get_vttbr(). */
 unsigned int kvm_arm_vmid_bits;
 
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index b98c38288a9d..9c6313cd6f5e 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -53,6 +53,7 @@ HAS_TLB_RANGE
 HAS_VIRT_HOST_EXTN
 HAS_WFXT
 HW_DBM
+ICACHE_PIPT
 KVM_HVHE
 KVM_PROTECTED_MODE
 MISMATCHED_CACHE_TYPE
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index c5af75b23187..db8c96841138 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2003,7 +2003,7 @@ Field	28	IDC
 Field	27:24	CWG
 Field	23:20	ERG
 Field	19:16	DminLine
-Enum	15:14	L1Ip
+UnsignedEnum	15:14	L1Ip
 	# This was named as VPIPT in the ARM but now documented as reserved
 	0b00	RESERVED_VPIPT
 	# This is named as AIVIVT in the ARM but documented as reserved
-- 
2.39.2


-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/3] arm64: Rename reserved values for CTR_EL0.L1Ip

[Anshuman Khandual]

On 12/4/23 20:06, Marc Zyngier wrote:
> We now have *two* values for CTR_EL0.L1Ip that are reserved.
> Which makes things a bit awkward. In order to lift the ambiguity,
> rename RESERVED (0b01) to RESERVED_AIVIVT, and VPIPT (0b00) to
> RESERVED_VPIPT.
> 
> This makes it clear which of these meant what, and I'm sure
> archeologists will find it useful...
> 
> Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
>  arch/arm64/tools/sysreg | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 96cbeeab4eec..c5af75b23187 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -2004,9 +2004,10 @@ Field	27:24	CWG
>  Field	23:20	ERG
>  Field	19:16	DminLine
>  Enum	15:14	L1Ip
> -	0b00	VPIPT
> +	# This was named as VPIPT in the ARM but now documented as reserved
> +	0b00	RESERVED_VPIPT
>  	# This is named as AIVIVT in the ARM but documented as reserved
> -	0b01	RESERVED
> +	0b01	RESERVED_AIVIVT
>  	0b10	VIPT
>  	0b11	PIPT
>  EndEnum

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[Mark Rutland]

On Mon, Dec 04, 2023 at 06:26:25PM +0000, Marc Zyngier wrote:
> On Mon, 04 Dec 2023 14:44:56 +0000,
> Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > On Mon, Dec 04, 2023 at 02:36:03PM +0000, Marc Zyngier wrote:
> > > ARMv8.2 introduced support for VPIPT i-caches, the V standing for
> > > VMID-tagged. Although this looked like a reasonable idea, no
> > > implementation has ever made it into the wild.
> > > 
> > > Linux has supported this for over 6 years (amusingly, just as the
> > > architecture was dropping support for AIVIVT i-caches), but we had no
> > > way to even test it, and it is likely that this code was just
> > > bit-rotting.
> > > 
> > > However, in a recent breakthrough (XML drop 2023-09, tagged as
> > > d55f5af8e09052abe92a02adf820deea2eaed717), the architecture has
> > > finally been purged of this option, making VIPT and PIPT the only two
> > > valid options.
> > > 
> > > This really means this code is just dead code. Nobody will ever come
> > > up with such an implementation, and we can just get rid of it.
> > > 
> > > Most of the impact is on KVM, where we drop a few large comment blocks
> > > (and a bit of code), while the core arch code loses the detection code
> > > itself.
> > > 
> > > * From v2:
> > >   - Fix reserved naming for RESERVED_AIVIVT
> > >   - Collected RBs from Anshuman an Zenghui
> > > 
> > > Marc Zyngier (3):
> > >   KVM: arm64: Remove VPIPT I-cache handling
> > >   arm64: Kill detection of VPIPT i-cache policy
> > >   arm64: Rename reserved values for CTR_EL0.L1Ip
> > 
> > For the series:
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> Thanks.
> 
> > Looking forward, we can/should probably replace __icache_flags with a single
> > ICACHE_NOALIASING or ICACHE_PIPT cpucap, which'd get rid of a bunch of
> > duplicated logic and make that more sound in the case of races around cpu
> > onlining.
> 
> As long as we refuse VIPT CPUs coming up late (i.e. after we have
> patched the kernel to set ICACHE_PIPT), it should be doable. I guess
> we already have this restriction as userspace is able to probe the
> I-cache policy anyway.
> 
> How about the patch below (tested in a guest with a bunch of hacks to
> expose different L1Ip values)?

That's roughly what I was thinking; the diff looks good, minor comments below.

> 
> Thanks,
> 
> 	M.
> 
> From 8f88afb0b317213dcbf18ea460a508bfccc18568 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Mon, 4 Dec 2023 18:09:40 +0000
> Subject: [PATCH] arm64: Make icache detection a cpu capability
> 
> Now that we only have two icache policies, we are in a good position
> to make the whole detection business more robust.
> 
> Let's replace __icache_flags by a single capability (ICACHE_PIPT),
> and use this if all CPUs are indeed PIPT. It means we can rely on
> existing logic to mandate that a VIPT CPU coming up late will be
> denied booting, which is the safe thing to do.
> 
> This also leads to some nice cleanups in pKVM, and KVM as a whole
> can use ARM64_ICACHE_PIPT as a final cap.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/include/asm/cache.h   |  9 ++-------
>  arch/arm64/include/asm/kvm_hyp.h |  1 -
>  arch/arm64/include/asm/kvm_mmu.h |  2 +-
>  arch/arm64/kernel/cpufeature.c   |  7 +++++++
>  arch/arm64/kernel/cpuinfo.c      | 34 --------------------------------
>  arch/arm64/kvm/arm.c             |  1 -
>  arch/arm64/kvm/hyp/nvhe/pkvm.c   |  3 ---
>  arch/arm64/tools/cpucaps         |  1 +
>  arch/arm64/tools/sysreg          |  2 +-
>  9 files changed, 12 insertions(+), 48 deletions(-)

Nice diffstat!

[...]

>  /*
>   * Whilst the D-side always behaves as PIPT on AArch64, aliasing is
>   * permitted in the I-cache.
>   */
>  static inline int icache_is_aliasing(void)
>  {
> -	return test_bit(ICACHEF_ALIASING, &__icache_flags);
> +	return !cpus_have_cap(ARM64_ICACHE_PIPT);
>  }

It might be nicer to use alternative_has_cap_{likely,unlikely}(...) for
consistency with other cap checks, though that won't matter for hyp code and I
don't think the likely/unlikely part particularly matters either.

[...]

> -static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
> -{
> -	unsigned int cpu = smp_processor_id();
> -	u32 l1ip = CTR_L1IP(info->reg_ctr);
> -
> -	switch (l1ip) {
> -	case CTR_EL0_L1Ip_PIPT:
> -		break;
> -	case CTR_EL0_L1Ip_VIPT:
> -	default:
> -		/* Assume aliasing */
> -		set_bit(ICACHEF_ALIASING, &__icache_flags);
> -		break;
> -	}
> -
> -	pr_info("Detected %s I-cache on CPU%d\n", icache_policy_str(l1ip), cpu);
> -}

Not printing this for each CPU is a nice bonus!

[...]

> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index c5af75b23187..db8c96841138 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -2003,7 +2003,7 @@ Field	28	IDC
>  Field	27:24	CWG
>  Field	23:20	ERG
>  Field	19:16	DminLine
> -Enum	15:14	L1Ip
> +UnsignedEnum	15:14	L1Ip
>  	# This was named as VPIPT in the ARM but now documented as reserved
>  	0b00	RESERVED_VPIPT
>  	# This is named as AIVIVT in the ARM but documented as reserved

I was initially surprised by the use of UnsignedEnum, but given PIPT is 0b11, I
can see that works. Otherwise, we can keep this as an enum and use a helper
that checks for an exact match.

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[Marc Zyngier]

On Tue, 05 Dec 2023 11:03:16 +0000,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Mon, Dec 04, 2023 at 06:26:25PM +0000, Marc Zyngier wrote:
> > How about the patch below (tested in a guest with a bunch of hacks to
> > expose different L1Ip values)?
> 
> That's roughly what I was thinking; the diff looks good, minor comments below.
> 
> [...]
> 
> >  /*
> >   * Whilst the D-side always behaves as PIPT on AArch64, aliasing is
> >   * permitted in the I-cache.
> >   */
> >  static inline int icache_is_aliasing(void)
> >  {
> > -	return test_bit(ICACHEF_ALIASING, &__icache_flags);
> > +	return !cpus_have_cap(ARM64_ICACHE_PIPT);
> >  }
> 
> It might be nicer to use alternative_has_cap_{likely,unlikely}(...) for
> consistency with other cap checks, though that won't matter for hyp code and I
> don't think the likely/unlikely part particularly matters either.

Right, this should be marginally better. I was initially worried of
the cap not being configured yet, but since we start by assuming VIPT
and only flip to PIPT once we have seen all CPUs, this stays correct
by construction.

> [...]
> 
> > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> > index c5af75b23187..db8c96841138 100644
> > --- a/arch/arm64/tools/sysreg
> > +++ b/arch/arm64/tools/sysreg
> > @@ -2003,7 +2003,7 @@ Field	28	IDC
> >  Field	27:24	CWG
> >  Field	23:20	ERG
> >  Field	19:16	DminLine
> > -Enum	15:14	L1Ip
> > +UnsignedEnum	15:14	L1Ip
> >  	# This was named as VPIPT in the ARM but now documented as reserved
> >  	0b00	RESERVED_VPIPT
> >  	# This is named as AIVIVT in the ARM but documented as reserved
> 
> I was initially surprised by the use of UnsignedEnum, but given PIPT is 0b11, I
> can see that works. Otherwise, we can keep this as an enum and use a helper
> that checks for an exact match.

Yeah, I hesitated either way, but maybe the helper approach is
cleaner.  I'll give it a go and repost this patch.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/3] arm64: Drop support for VPIPT i-cache policy

[Will Deacon]

On Mon, 4 Dec 2023 14:36:03 +0000, Marc Zyngier wrote:
> ARMv8.2 introduced support for VPIPT i-caches, the V standing for
> VMID-tagged. Although this looked like a reasonable idea, no
> implementation has ever made it into the wild.
> 
> Linux has supported this for over 6 years (amusingly, just as the
> architecture was dropping support for AIVIVT i-caches), but we had no
> way to even test it, and it is likely that this code was just
> bit-rotting.
> 
> [...]

Applied to arm64 (for-next/rip-vpipt), thanks!

[1/3] KVM: arm64: Remove VPIPT I-cache handling
      https://git.kernel.org/arm64/c/ced242ba9d7c
[2/3] arm64: Kill detection of VPIPT i-cache policy
      https://git.kernel.org/arm64/c/d8e12a0d3715
[3/3] arm64: Rename reserved values for CTR_EL0.L1Ip
      https://git.kernel.org/arm64/c/f35c32ca6839

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel