lkml.org 
[lkml]   [2018]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 0/5] arm64: assorted fixes for dcache_by_line_op
From
Date
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 <robin.murphy@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Suzuki Poulose <suzuki.poulose@arm.com>
>> Cc: Dave Martin <Dave.Martin@arm.com>
>>
>> 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
>

\
 
 \ /
  Last update: 2018-12-10 15:50    [W:0.096 / U:8.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site