Skip to contentSkip to conversation

arm64: dbm: Invalidate local TLB when setting TCR_EL1.HD

PATCH Superseded Will Deacon 3 participants 4 messages Oct 1, 2020 arm64

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

PATCH 1/1 arm64: dbm: Invalidate local TLB when setting TCR_EL1.HD

TCR_EL1.HD is permitted to be cached in a TLB, so invalidate the local TLB after setting the bit when detected support for the feature. Although this isn't strictly necessary, since we can happily operate with the bit effectively clear, the current code uses an ISB in a half-hearted attempt to make the change effective, so let's just fix that up.

Cc Catalin Marinas <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +1 -1
index 6424584be01e..29de76046977 100644
@@ -1442,7 +1442,7 @@ static inline void __cpu_enable_hw_dbm(void)
 	u64 tcr = read_sysreg(tcr_el1) | TCR_HD;
 
 	write_sysreg(tcr, tcr_el1);
-	isb();
+	local_flush_tlb_all();
 }
 
 static bool cpu_has_broken_dbm(void)
Signature · 7 lines
2.28.0.709.gb0816b6eb0-goog


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
maintainer
#1 lore raw
Quoted 17 lines · Will Deacon

TCR_EL1.HD is permitted to be cached in a TLB, so invalidate the local TLB after setting the bit when detected support for the feature. Although this isn't strictly necessary, since we can happily operate with the bit effectively clear, the current code uses an ISB in a half-hearted attempt to make the change effective, so let's just fix that up.

Cc Catalin Marinas <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +0 -0
index 6424584be01e..29de76046977 100644

arch/arm64/kernel/cpufeature.c static inline void __cpu_enable_hw_dbm(void) · line 1445

Quoted 5 lines
arch/arm64/kernel/cpufeature.c +1 -1
@@ -1442,7 +1442,7 @@ static inline void __cpu_enable_hw_dbm(void)
 	u64 tcr = read_sysreg(tcr_el1) | TCR_HD;
 
 	write_sysreg(tcr, tcr_el1);
-	isb();
+	local_flush_tlb_all();

Is the TLBI ordered with the TCR_EL1 write or we need the ISB as well?

Signature · 6 lines
Catalin

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author maintainer
#2 lore raw
Quoted 20 lines · Catalin Marinas
Quoted 19 lines · Will Deacon

TCR_EL1.HD is permitted to be cached in a TLB, so invalidate the local TLB after setting the bit when detected support for the feature. Although this isn't strictly necessary, since we can happily operate with the bit effectively clear, the current code uses an ISB in a half-hearted attempt to make the change effective, so let's just fix that up.

Cc Catalin Marinas <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +0 -0
index 6424584be01e..29de76046977 100644
@@ -1442,7 +1442,7 @@ static inline void __cpu_enable_hw_dbm(void)
 	u64 tcr = read_sysreg(tcr_el1) | TCR_HD;
 
Quoted 5 lines

arch/arm64/kernel/cpufeature.c static inline void __cpu_enable_hw_dbm(void) · line 1445

Quoted 3 lines
arch/arm64/kernel/cpufeature.c +1 -1
 	write_sysreg(tcr, tcr_el1);
-	isb();
+	local_flush_tlb_all();

Is the TLBI ordered with the TCR_EL1 write or we need the ISB as well?

Good catch! I think this is direct-write -> indirect-read ordering, which needs the ISB. so I'll keep it and add the flush immediately afterwards.

Will

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

reviewer
#3 lore raw
In reply to Catalin Marinas
Quoted 20 lines · Catalin Marinas
Quoted 19 lines · Will Deacon

TCR_EL1.HD is permitted to be cached in a TLB, so invalidate the local TLB after setting the bit when detected support for the feature. Although this isn't strictly necessary, since we can happily operate with the bit effectively clear, the current code uses an ISB in a half-hearted attempt to make the change effective, so let's just fix that up.

Cc Catalin Marinas <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +0 -0
index 6424584be01e..29de76046977 100644
@@ -1442,7 +1442,7 @@ static inline void __cpu_enable_hw_dbm(void)
 	u64 tcr = read_sysreg(tcr_el1) | TCR_HD;
 
Quoted 5 lines

arch/arm64/kernel/cpufeature.c static inline void __cpu_enable_hw_dbm(void) · line 1445

Quoted 3 lines
arch/arm64/kernel/cpufeature.c +1 -1
 	write_sysreg(tcr, tcr_el1);
-	isb();
+	local_flush_tlb_all();

Is the TLBI ordered with the TCR_EL1 write or we need the ISB as well?

I believe we still need the ISB prior to the TLBI since MSR and TLBI are not usually ordered w.r.t. one another, so after teh TLBI it might still be possible to allcote the stale value into TLBs.

Mark.

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel