Messages in this thread Patch in this message | | | Date | Mon, 15 Apr 2019 13:35:35 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH V5 5/7] x86/alternative: Batch of patch operations |
| |
On Mon, Apr 01, 2019 at 10:58:17AM +0200, Daniel Bristot de Oliveira wrote: > + ip = (void *) regs->ip - sizeof(unsigned char);
That one confused me mightily. Does that want to be:
ip = (void *)regs->ip - LEN_INT3;
or something? Even just a naked 1 would've been less confusing.
> > + /* > + * Skip the binary search if there is a single member in the vector. > + */ > + if (unlikely(bp_patching.nr_entries == 1)) > + goto single_poke; > + > + tp = bsearch(ip, bp_patching.vec, bp_patching.nr_entries, > + sizeof(struct text_patch_loc), > + patch_cmp); > + if (!tp) > + return 0; > + > + /* set up the specified breakpoint detour */ > + regs->ip = (unsigned long) tp->detour; > return 1; > +single_poke: > + if (ip == bp_patching.vec->addr) { > + regs->ip = (unsigned long) bp_patching.vec->detour; > + return 1; > + } > + > + return 0;
if (bp_patching.nr_entries > 1) { tp = bsearch(ip, bp_patching.vec, bp_patching.nr_entries, sizeof(struct text_patch_loc), patch_cmp); if (!tp) return 0; } else { tp = bp_patching.vec; if (tp->addr != ip) return 0; }
regs->ip = (unsigned long)tp->detour; return 1; > } > NOKPROBE_SYMBOL(poke_int3_handler);
> +void text_poke_bp_batch(struct text_patch_loc *tp, unsigned int nr_entries) > { > + int patched_all_but_first = 0; > unsigned char int3 = 0xcc; > + unsigned int i; > > lockdep_assert_held(&text_mutex); > > + bp_patching.vec = tp; > + bp_patching.nr_entries = nr_entries; > + bp_patching.in_progress = true; > /* > * Corresponding read barrier in int3 notifier for making sure the > * in_progress and handler are correctly ordered wrt. patching. > */ > smp_wmb(); > > + /* > + * First step: add a int3 trap to the address that will be patched. > + */ > + for (i = 0; i < nr_entries; i++) > + text_poke(tp[i].addr, &int3, sizeof(int3)); > > on_each_cpu(do_sync_core, NULL, 1); > > + /* > + * Second step: update all but the first byte of the patched range. > + */ > + for (i = 0; i < nr_entries; i++) { > + if (tp[i].len - sizeof(int3) > 0) { > + text_poke((char *)tp[i].addr + sizeof(int3), > + (const char *)tp[i].opcode + sizeof(int3), > + tp[i].len - sizeof(int3)); > + patched_all_but_first++; > + } > + } > + > + if (patched_all_but_first) { > /* > * According to Intel, this core syncing is very likely > * not necessary and we'd be safe even without it. But > @@ -821,15 +874,52 @@ void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler) > on_each_cpu(do_sync_core, NULL, 1); > } > > + /* > + * Third step: replace the first byte (int3) by the first byte of > + * replacing opcode. > + */ > + for (i = 0; i < nr_entries; i++) > + text_poke(tp[i].addr, tp[i].opcode, sizeof(int3)); > > on_each_cpu(do_sync_core, NULL, 1); > /* > * sync_core() implies an smp_mb() and orders this store against > * the writing of the new instruction. > */ > + bp_patching.vec = NULL; > + bp_patching.nr_entries = 0; > + bp_patching.in_progress = false; > +}
| |