[8u] RFR: nativeInstruction atomicity and correctness of patching

[Edward Nevill]

Hi,

Was there a conclusion on this?

Are you happy with Anton's rev2 patch?

All the best,
Ed.

On Fri, 2016-04-01 at 10:41 +0000, Anton Kozlov wrote:
> Hi, Andrew,
> 
> On Thu, 2016-03-31 at 09:49 +0100, Andrew Haley wrote:
> > On 30/03/16 20:53, Anton Kozlov wrote:
> > 
> > > On Wed, 2016-03-30 at 16:25 +0100, Andrew Haley wrote:
> > > > On 03/30/2016 04:15 PM, Anton Kozlov wrote:
> > > > > ICache::invalidate_word used here not to provide coherency
> > > > > between
> > > > > Icache and Dcache, but to maintain in-order visibillity of code
> > > > > modification.
> > > > 
> > > > What exactly are you trying to synchronize?
> > 
> > > ICache::invalidate ensures that all CPUs see modifications of code.
> > > Data modification in trampoline should be visible in the same way,
> > > same time as code modification would. For this ICache::invalidate
> > > used, as it by implementation should synchornize dcaches among
> > > CPUs.
> > 
> > I don't think so.  In order for this to work, the *reader* would have
> > to execute some kind of fence too.  I don't think that's possible
> > without an inter-processor interrupt across all cores, and I think
> > that __clear_cache doesn't do that.  I certainly hope not.
> > 
> > > > > This is achieved by implementation of ICache::invalidate_word,
> > > > > which is just call to __clear_cache syscall wrapper.
> > > > 
> > > > I don't think there's any point.  It's not instructions but data,
> > > > so
> > > > ICache::invalidate_word is inappropriate.
> > > 
> > > Agree, ICache is bad name for that. How about direct __clear_cache
> > > call?
> > 
> > > Also, I will try to study implementation of __clear_cache to know,
> > > how exactly it's done.
> > 
> > I think you will find that it does not do what you think it does. 
> >  I'm
> > looking at linux/arch/arm/include/asm/cacheflush.h and I can't see
> > anything like that.  All I see is that do_cache_op calls
> > __flush_cache_user_range, and all that does is ensure that the I and
> > D
> > caches are coherent.