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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
