Re: [PATCH v4 04/18] arm64: Do not enable uaccess for flush_icache_range

On Mon, May 24, 2021 at 11:41:53AM +0200, Ard Biesheuvel wrote:

On Mon, 24 May 2021 at 11:20, Fuad Tabba [off-list ref] wrote:

quoted

Hi Ard,

quoted

quoted

diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 2d881f34dd9d..7c54bcbf5a36 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S

@@ -14,6 +14,34 @@
 #include <asm/alternative.h>
 #include <asm/asm-uaccess.h>

+/*
+ *     __flush_cache_range(start,end) [fixup]
+ *
+ *     Ensure that the I and D caches are coherent within specified region.
+ *     This is typically used when code has been written to a memory region,
+ *     and will be executed.
+ *
+ *     - start   - virtual start address of region
+ *     - end     - virtual end address of region
+ *     - fixup   - optional label to branch to on user fault
+ */
+.macro __flush_cache_range, fixup
+alternative_if ARM64_HAS_CACHE_IDC
+       dsb     ishst

Should this perhaps be dsb ish? IIUC, ishst does not synchronize on
completion of cache maintenance, and while that is implicit on this
code path, I'd still assume it needs to complete before carrying on.
Or does IDC not require this?

I'm not sure; ishst in this patch is unchanged (just moved to the
macro). Reading the Arm ARM (B2-143) I think that ishst is correct:

"""
CTR_EL0.{DIC, IDC} == {0, 1}

The write is complete for the shareability domain. Subsequently the
location has been invalidated to the Point of unification (PoU) from
the instruction cache, and that invalidation is complete for the
shareability domain.

CTR_EL0.{DIC, IDC} == {1, 1}

The write is complete for the shareability domain.
"""

Does my interpretation sound right to you?

Thanks for digging that up.

So IDC does guarantee that completing the store is sufficient for the
I-side to observe it, so I think your interpretation is correct.

FWIW, that's muy understanding too.

Ths idea with IDC is that when instructions fetches look in data/unified
caches these lookups are made coherently (and the results may then be
allocated into I-caches), so there's no architectural requirement for
data cache maintenance, but we need to ensure completion for ordering
against instruction fetches.

The requirement is similar to that for translation table updates, where
until ARMv7's multiprocessing extension we required data cache
maintenance, but these days the fetches are coherent on the data side,
and for similar ordering reasons we complete writes with DSB ISHST.

Thanks,
Mark.

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