Hi Will,
On 07/12/2018 17:53, Will Deacon wrote:
Hi Ard,
Cheers for looking at this.
On Thu, Dec 06, 2018 at 04:57:34PM +0100, Ard Biesheuvel wrote:
This fixes two issues in dcache_by_line_op: patch #4 fixes the logic
that is applied when patching DC CVAP instructions, and patch #5 gets
rid of some unintended undefined symbols resulting from incorrect use
of conditional GAS directives.
Patches #1 to #3 are groundwork for #4.
Cc: Robin Murphy <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Suzuki Poulose <[email protected]>
Cc: Dave Martin <[email protected]>
Ard Biesheuvel (5):
arm64/alternative_cb: move callback reference into replacements
section
arm64/alternative_cb: add nr_alts parameter to callback handlers
arm64/alternative_cb: add support for alternative sequences
arm64/assembler: use callback to 3-way alt-patch DC CVAP instructions
arm64/mm: use string comparisons in dcache_by_line_op
arch/arm64/include/asm/alternative.h | 54 +++++++++++---------
arch/arm64/include/asm/assembler.h | 17 +++---
arch/arm64/include/asm/kvm_mmu.h | 4 +-
arch/arm64/kernel/alternative.c | 22 +++++---
arch/arm64/kernel/cpu_errata.c | 24 ++++++---
arch/arm64/kvm/va_layout.c | 8 +--
6 files changed, 79 insertions(+), 50 deletions(-)
Whilst I can see that this solves the problem, I do wonder whether the
additional infrastructure is really worth it. Couldn't we just do something
really simple like the diff below instead?
Given that there's really only one place we expect CVAP to be used, I
reckon we could factor that out beforehand to make life even simpler, as
below.
Robin.
----->8-----
diff --git a/arch/arm64/include/asm/assembler.h
b/arch/arm64/include/asm/assembler.h
index 6142402c2eb4..8d88d3f1e90e 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -390,11 +390,7 @@ alternative_else
dc civac, \kaddr
alternative_endif
.elseif (\op == cvap)
-alternative_if ARM64_HAS_DCPOP
sys 3, c7, c12, 1, \kaddr // dc cvap
-alternative_else
- dc cvac, \kaddr
-alternative_endif
.else
dc \op, \kaddr
.endif
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 0c22ede52f90..98b17bee4f9d 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -212,6 +212,9 @@ ENDPROC(__dma_clean_area)
* - size - size in question
*/
ENTRY(__clean_dcache_area_pop)
+ alternative_if_not ARM64_HAS_DCPOP
+ b __clean_dcache_area_poc
+ alternative_else_nop_endif
dcache_by_line_op cvap, sy, x0, x1, x2, x3
ret
ENDPIPROC(__clean_dcache_area_pop)
-----8<----
Will
--->8
diff --git a/arch/arm64/include/asm/assembler.h
b/arch/arm64/include/asm/assembler.h
index 953208267252..8dea015b6599 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -390,27 +390,38 @@ alternative_endif
* size: size of the region
* Corrupts: kaddr, size, tmp1, tmp2
*/
+ .macro __dcache_op_workaround_clean_cache, op, kaddr
+alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE
+ dc \op, \kaddr
+alternative_else
+ dc civac, \kaddr
+alternative_endif
+ .endm
+
.macro dcache_by_line_op op, domain, kaddr, size, tmp1, tmp2
dcache_line_size \tmp1, \tmp2
add \size, \kaddr, \size
sub \tmp2, \tmp1, #1
bic \kaddr, \kaddr, \tmp2
9998:
- .if (\op == cvau || \op == cvac)
-alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE
- dc \op, \kaddr
-alternative_else
- dc civac, \kaddr
-alternative_endif
- .elseif (\op == cvap)
+ .ifc \op, cvau
+ __dcache_op_workaround_clean_cache \op, \kaddr
+ .else
+ .ifc \op, cvac
+ __dcache_op_workaround_clean_cache \op, \kaddr
+ .else
+ .ifc \op, cvap
alternative_if ARM64_HAS_DCPOP
sys 3, c7, c12, 1, \kaddr // dc cvap
-alternative_else
- dc cvac, \kaddr
-alternative_endif
+ b 9996f
+alternative_else_nop_endif
+ __dcache_op_workaround_clean_cache cvac, \kaddr
+9996:
.else
dc \op, \kaddr
.endif
+ .endif
+ .endif
add \kaddr, \kaddr, \tmp1
cmp \kaddr, \size
b.lo 9998b
