Thread (29 messages) 29 messages, 3 authors, 2021-05-20

Re: [PATCH v2 03/16] arm64: Do not enable uaccess for invalidate_icache_range

From: Fuad Tabba <hidden>
Date: 2021-05-19 16:28:56

On Tue, May 18, 2021 at 4:36 PM Mark Rutland [off-list ref] wrote:
On Mon, May 17, 2021 at 08:51:11AM +0100, Fuad Tabba wrote:
quoted
invalidate_icache_range() works on the kernel linear map, and
doesn't need uaccess. Remove the code that toggles
uaccess_ttbr0_enable, as well as the code that emits an entry
into the exception table (via the macro
invalidate_icache_by_line).

Changes return type of invalidate_icache_range() from int (which
used to indicate a fault) to void, since it doesn't need uaccess
and won't fault. Note that return value was never checked by any
of the callers.

No functional change intended.
Possible performance impact due to the reduced number of
instructions.

Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Will Deacon <will@kernel.org>
Link: https://lore.kernel.org/linux-arch/20200511110014.lb9PEahJ4hVOYrbwIb_qUHXyNy9KQzNFdb_I3YlzY6A@z/
Signed-off-by: Fuad Tabba <redacted>
---
 arch/arm64/include/asm/cacheflush.h |  2 +-
 arch/arm64/mm/cache.S               | 11 +----------
 2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index 52e5c1623224..a586afa84172 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -57,7 +57,7 @@
  *           - size   - region size
  */
 extern void __flush_icache_range(unsigned long start, unsigned long end);
-extern int  invalidate_icache_range(unsigned long start, unsigned long end);
+extern void invalidate_icache_range(unsigned long start, unsigned long end);
 extern void __flush_dcache_area(void *addr, size_t len);
 extern void __inval_dcache_area(void *addr, size_t len);
 extern void __clean_dcache_area_poc(void *addr, size_t len);
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 092f73acdf9a..6babaaf34f17 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -105,21 +105,12 @@ SYM_FUNC_END(__flush_cache_user_range)
  */
 SYM_FUNC_START(invalidate_icache_range)
 alternative_if ARM64_HAS_CACHE_DIC
-     mov     x0, xzr
      isb
      ret
 alternative_else_nop_endif

-     uaccess_ttbr0_enable x2, x3, x4
-
-     invalidate_icache_by_line x0, x1, x2, x3, 1, 2f
-     mov     x0, xzr
-1:
-     uaccess_ttbr0_disable x1, x2
+     invalidate_icache_by_line x0, x1, x2, x3, 0, 0f
This all looks good to me, but I'd prefer that we didn't have to pass a
fake label. I think if you use the approach I suggested on the prior
patch, this can be:

        invalidate_icache_by_line x0, x1, x2, x3

... which I think makes it clearer that there's no fixup.
Got it!

Thanks,
/fuad
Thanks,
Mark.
quoted
      ret
-2:
-     mov     x0, #-EFAULT
-     b       1b
 SYM_FUNC_END(invalidate_icache_range)

 /*
--
2.31.1.751.gd2f1c929bd-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help