Re: [Xen-devel] [PATCH v4 04/16] livepatch: Initial ARM64 support.

[Julien Grall <julien.grall@xxxxxxx>]

Hi Konrad,

On 19/09/2016 16:33, Konrad Rzeszutek Wilk wrote:
> > >  void arch_livepatch_revive(void)
> > >  {
> > > +    /*
> > > +     * Nuke the instruction cache. Data cache has been cleaned before in
> > > +     * arch_livepatch_apply_jmp.
> > I think you forgot to clean text region from the payload. Without that, you
> > may receive a crash if you have a separate cache for data and instruction.
> Help me out here please.
>
> Why would we need to call clean_and_invalidate_dcache_va_range on the
> payload .text area (the func->new_addr and func->new_size)?
>
> We don't modify that .text area and after this function is done
> (arch_livepatch_revive) it would be very first time that code would be called.
>
> Hence there would not be any cache remains at all?
Because when you copy the payload to the memory, it will go through the 
data cache (the region is cacheable). So until the data cache is 
cleaned, the data may not reach the memory.
In the case the data and instruction cache are separated, you may read 
invalid instruction from the memory because the data cache have not yet 
synced with the memory.
> Or did you mean the old_addr (the one we just patched?)
We don't care about the previous function. It will never changed except 
for the instruction patched.
> If we are reverting it then we just clear at func->old_addr for one
> instruction? We could drop the dcache for the func->new_addr (so new
> .text code), to explicitly tell the CPU to not waste cache space for
> those instructions? Is that what you meant?
The data cache should not contain any cache line of the previous 
function because it only contains instructions. However the instructions 
cache will contain some of them, but they will be removed by the 
invalidate cache instruction in arch_live_revive.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel