Messages in this thread | | | Date | Fri, 7 Sep 2007 08:59:36 +0200 | From | Andi Kleen <> | Subject | Re: [patch 05/10] Text Edit Lock - Alternative code for i386 and x86_64 |
| |
On Thu, Sep 06, 2007 at 04:01:29PM -0400, Mathieu Desnoyers wrote: > + sync_core(); > + /* Not strictly needed, but can speed CPU recovery up. */
That turned out to break on some VIA CPUs. Should be removed.
> + if (cpu_has_clflush) > + for (faddr = addr; faddr < addr + len; > + faddr += boot_cpu_data.x86_clflush_size) > + asm("clflush (%0) " :: "r" (faddr) : "memory"); > +} > + > +void * text_poke_early(void *addr, const void *opcode, > + size_t len) > +{ > + memcpy(addr, opcode, len);
It would be best to copy __inline_memcpy from x86-64 to i386 and use that here. That avoids the dependency on a patched memcpy and is slightly safer.
> + > + if (len > sizeof(long)) { > + printk(KERN_ERR "text_poke of len %zu too big (max %lu)\n", > + len, sizeof(long)); > + BUG_ON(1);
In general BUG_ON only should be enough because these values can be recovered from the registers.
> + } > + unaligned = (((long)addr + len - 1) & ~(sizeof(long) - 1)) > + - ((long)addr & ~(sizeof(long) - 1)); > + if (unlikely(unaligned)) { > + printk(KERN_ERR "text_poke of at addr %p of len %zu is " > + "unaligned (%d)\n", > + addr, len, unaligned); > + BUG_ON(1); > + }
The common code should be in a common function. In fact they're so similar that the caller could just pass a buffer for the text_set case, couldn't it?
> +#define kernel_wp_save(cr0) \
Is there a real reason this has to be an macro? It could be just a normal function. In fact a shared on in alternative.c. That would also avoid adding more include dependencies.
> + do { \ > + typecheck(unsigned long, cr0); \
typecheck is probably overkill
> + preempt_disable(); \
Should disable interrupts too just to be safer?
-Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |