Thanks for the review, Song.
On 29/09/23 2:38 am, Song Liu wrote:
On Thu, Sep 28, 2023 at 12:48 PM Hari Bathini <hbathini@xxxxxxxxxxxxx> wrote:
patch_instruction() entails setting up pte, patching the instruction,
clearing the pte and flushing the tlb. If multiple instructions need
to be patched, every instruction would have to go through the above
drill unnecessarily. Instead, introduce function patch_instructions()
that sets up the pte, clears the pte and flushes the tlb only once per
page range of instructions to be patched. This adds a slight overhead
to patch_instruction() call while improving the patching time for
scenarios where more than one instruction needs to be patched.
Signed-off-by: Hari Bathini <hbathini@xxxxxxxxxxxxx>
Acked-by: Song Liu <song@xxxxxxxxxx>
With a nit below.
[...]
+/*
+ * A page is mapped and instructions that fit the page are patched.
+ * Assumes 'len' to be (PAGE_SIZE - offset_in_page(addr)) or below.
+ */
+static int __do_patch_instructions_mm(u32 *addr, void *code, size_t len, bool repeat_instr)
{
int err;
u32 *patch_addr;
@@ -307,11 +336,15 @@ static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
orig_mm = start_using_temp_mm(patching_mm);
- err = __patch_instruction(addr, instr, patch_addr);
+ /* Single instruction case. */
+ if (len == 0) {
+ err = __patch_instruction(addr, *(ppc_inst_t *)code, patch_addr);
len == 0 for single instruction is a little weird to me. How about we just use
len == 4 or 8 depending on the instruction to patch?
Yeah. Looks a bit weird but it avoids the need to call ppc_inst_read()
& ppc_inst_len(). A comment explaining why this weird check could
have been better though..
Thanks
Hari
