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

[Ard Biesheuvel]

On Mon, 24 May 2021 at 11:20, Fuad Tabba <tabba at google.com> wrote:
>
> Hi Ard,
>
> > > 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.