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@lists.xen.org
https://lists.xen.org/xen-devel

Reply via email to