On Wed, Dec 14, 2016 at 11:16:07AM +0000, James Morse wrote:
> Hi Pratyush,
> On 14/12/16 09:38, Pratyush Anand wrote:
> > On Saturday 26 November 2016 12:00 AM, James Morse wrote:
> >> On 22/11/16 04:32, Pratyush Anand wrote:
> >>> +/*
> >>> + * disable_dcache: Disable D-cache and flush RAM locations
> >>> + * ram_start - Start address of RAM
> >>> + * ram_end - End address of RAM
> >>> + */
> >>> +void disable_dcache(uint64_t ram_start, uint64_t ram_end)
> >>> +{
> >>> + switch(get_current_el()) {
> >>> + case 2:
> >>> + reset_sctlr_el2();
> >>> + break;
> >>> + case 1:
> >>> + reset_sctlr_el1();
> >>
> >> You have C code running between disabling the MMU and cleaning the cache. The
> >> compiler is allowed to move data on and off the stack in here, but after
> >> disabling the MMU it will see whatever was on the stack before we turned the MMU
> >> on. Any data written at the beginning of this function is left in the caches.
> >>
> >> I'm afraid this sort of stuff needs to be done in assembly!
> >
> > All these routines are self coded in assembly even though they are called
> > from C, so should be safe I think. Anyway, I can keep all of them in
> > assembly as well.
>
> You can't tell the compiler that the stack data is inaccessible until the dcache
> clean call completes. Some future version may do really crazy things in here.
> You can decompile what your compiler version produces to check it doesn't
> load/store to the stack, but that doesn't mean my compiler version does the
> same. This is the kind of thing that is extremely difficult to debug, its best
> not to take the risk.
FWIW, I completely agree.
We've been bitten in the past; see commit 5e051531447259e5 ("arm64:
convert part of soft_restart() to assembly") for an example.
Thanks,
Mark.
