Mathieu Desnoyers <[email protected]> writes:
> * Andi Kleen ([email protected]) wrote:
> >
> > > Ewwwwwwwwwww.... you plan to run this in SMP ? So you actually go byte
> > > by byte changing pieces of instructions non atomically and doing
> > > non-Intel's errata friendly XMC. You are really looking for trouble
> > > there :) Two distinct errors can occur:
> >
> > In this case it is ok because this only happens when transitioning
> > from 1 CPU to 2 CPUs or vice versa and in both cases the other CPUs
> > are essentially stopped.
> >
>
> I agree that it's ok with SMP, but another problem arises: it's not only
> a matter of being protected from SMP access, but also a matter of
> reentrancy wrt interrupt handlers.
>
> i.e.: if, as we are patching nops non atomically, we have a non maskable
> interrupt coming which calls get_cycles_sync() which uses the
Hmm, i didn't think NMI handlers called that. e.g. nmi watchdog just
uses jiffies.
get_cycles_sync patching happens only relatively early at boot, so oprofile
cannot be running yet.
> I see that IRQs are disabled in alternative_instructions(), but it does
> not protect against NMIs, which could come at a very inappropriate
> moment. MCE and SMIs would potentially cause the same kind of trouble.
>
> So unless you can guarantee that any code from NMI handler won't call
> basic things such as get_cycles() (nor MCE, nor SMIs), you can't insure
> it won't execute an illegal instruction. Also, the option of temporarily
> disabling the NMI for the duration of the update simply adds unwanted
> latency to the NMI handler which could be unacceptable in some setups.
Ok it's a fair point. But how would you address it ?
Even if we IPIed the other CPUs NMIs or MCEs could still happen.
BTW Jeremy, have you ever considered that problem with paravirt ops
patching?
>
> Another potential problem comes from preemption: if a thread is
> preempted in the middle of the instructions you are modifying, let's say
> we originally have 4 * 1 byte instructions that we replace with 1 * 4
> byte instruction, a thread could iret in the middle of the new
> instruction when it will be scheduled again, thus executing an illegal
> instruction. This problem could be overcomed by putting the threads in
> the freezer. See the djprobe project for details about this.
This cannot happen for the current code:
- full alternative patching happen only at boot when the other CPUs
are not running
- smp lock patching only ever changes a single byte (lock prefix) of
a single instruction
- kprobes only ever change a single byte
For the immediate value patching it also cannot happen because
you'll never modify multiple instructions and all immediate values
can be changed atomically.
> In the end, I think the safest solution would be to limit ourselves to
> one of these 2 solutions:
> - If the update can be done atomically and leaves the site in a coherent
> state after each atomic modification, while keeping the same
> instruction size, it could be done on the spot.
> - If not, care should be taken to first do a copy of the code to modify
> into a "bypass", making sure that instructions can be relocated, so
> that any context that would try to execute the site during the
> modification would execute a breakpoint which would execute the bypass
> while we modify the instructions.
kprobes does that, but to write the break point it uses text_poke()
with my patch.
> Because of the tricks that must be done to do code modification on a
> live system (as explained above), I don't think it makes sense to
> provide a falsely-safe infrastructure that would allow code modification
> in a non-atomic manner.
Hmm, perhaps it would make sense to add a comment warning about
the limitations of the current text_poke. Can you supply one?
-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
