On Mon, 2009-09-07 at 20:43 +0100, Richard Kettlewell wrote:
> I saw the text_poke_early crash too, on a Soekris net4501, which uses an 
> AMD ElanSC520.  The problem may or may not apply to other 486-class 
> hardware (some reports of the bug say it does).
> 
> The workaround is to specify noreplace-paravirt on the kernel command line.
> 
> If the bug does indeed affect all 486-class CPUs then it would be a good 
> idea to compile CONFIG_PARAVIRT out of the -486 kernels entirely, at 
> least until such time as the bug is fixed.

I'm not sure we can do that because the -486 kernel is used for
installation.  Let's fix this properly.

Comparing the original code with the oops, we have:

void *text_poke_early(void *addr, const void *opcode, size_t len)
{
c010793c:       55                      push   %ebp
c010793d:       89 c5                   mov    %eax,%ebp
c010793f:       57                      push   %edi
c0107940:       56                      push   %esi
c0107941:       89 d6                   mov    %edx,%esi
c0107943:       53                      push   %ebx
c0107944:       89 cb                   mov    %ecx,%ebx

	unsigned long flags;
c0107946:       83 ec 04                sub    $0x4,%esp

	local_irq_save(flags);
[original]
c0107949:       51                      push   %ecx
c010794a:       52                      push   %edx
c010794b:       ff 15 dc 1d 34 c0       call   *0xc0341ddc
c0107951:       5a                      pop    %edx
c0107952:       59                      pop    %ecx
c0107953:       89 c2                   mov    %eax,%edx
[patched code not visible]
[original]
c0107955:       51                      push   %ecx
c0107956:       52                      push   %edx
c0107957:       ff 15 e4 1d 34 c0       call   *0xc0341de4
c010795d:       5a                      pop    %edx
c010795e:       59                      pop    %ecx
[patched]
c0107955:	fa                   	cli    
c0107956:	90                   	nop
c0107958:	8d b4 26 00 00 00 00 	lea    0x0(%esi,%eiz,1),%esi
c010795e:	90                   	nop

	memcpy(addr, opcode, len);
c010795f:       89 c8                   mov    %ecx,%eax
c0107961:       89 ef                   mov    %ebp,%edi
c0107963:       c1 e8 02                shr    $0x2,%eax
c0107966:       89 c1                   mov    %eax,%ecx
c0107968:       f3 a5                   rep movsl %ds:(%esi),%es:(%edi)
c010796a:       89 d9                   mov    %ebx,%ecx
c010796c:       83 e1 03                and    $0x3,%ecx
c010796f:       74 02                   je     0xc0107973
c0107971:       f3 a4                   rep movsb %ds:(%esi),%es:(%edi)
c0107973:       89 d0                   mov    %edx,%eax

	local_irq_restore(flags);
[original]
c0107975:       51                      push   %ecx
c0107976:       52                      push   %edx
c0107977:       ff 15 e0 1d 34 c0       call   *0xc0341de0
c010797d:       5a                      pop    %edx
c010797e:       59                      pop    %ecx
[patched]
c0107975:	50                   	push   %eax
c0107976:	9d                   	popf   
c0107977:	90                   	nop
c0107878:	8d b4 26 00 00 00 00 	lea    0x0(%esi,%eiz,1),%esi

	sync_core();
c010797f:       b8 01 00 00 00          mov    $0x1,%eax
c0107984:       0f a2                   cpuid  

}
c0107986:       5a                      pop    %edx
c0107987:       89 e8                   mov    %ebp,%eax
c0107989:       5b                      pop    %ebx
c010798a:       5e                      pop    %esi
c010798b:       5f                      pop    %edi
c010798c:       5d                      pop    %ebp
c010798d:       c3                      ret    

The crash occurs at c010797d.  Presumably the original code is
prefetched and executed up to and including the call instruction, and
then the patched code is fetched after the local_irq_restore()
implementation returns.

I think we need to flush the prefetch buffer between memcpy() and
local_irq_restore().  According to Intel documentation, we can do this
on the i486 with a jmp instruction, and later processors flush the
prefetch buffer automatically when the corresponding memory is modified.

There is a further problem in that sync_core() uses "cpuid" which isn't
implemented by most 486-class processors, but that is easy to fix.

I don't have any 486-class systems to test this on, so perhaps you could
try this patch:

--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -488,6 +488,9 @@
 	unsigned long flags;
 	local_irq_save(flags);
 	memcpy(addr, opcode, len);
+	/* Force 486-class processors to flush prefetched instructions,
+	   since we may have just patched local_irq_restore(). */
+	asm volatile("jmp 1f\n1:\n" ::: "memory");
 	local_irq_restore(flags);
 	sync_core();
 	/* Could also do a CLFLUSH here to speed up CPU recovery; but
--- a/include/asm-x86/processor.h
+++ b/include/asm-x86/processor.h
@@ -698,6 +698,12 @@
 {
 	int tmp;
 
+#if defined(CONFIG_M386) || defined(CONFIG_M486)
+	/* This is unnecessary on 386- and 486-class processors, most of
+	   which don't even implement CPUID. */
+	if (boot_cpu_data.x86 < 5)
+		return;
+#endif
 	asm volatile("cpuid" : "=a" (tmp) : "0" (1)
 		     : "ebx", "ecx", "edx", "memory");
 }
--- END ---

Ben.

-- 
Ben Hutchings
Life is what happens to you while you're busy making other plans.
                                                               - John Lennon