Patchwork [0/8] Fix 8xx MMU/TLB

login
register
mail settings
Submitter Joakim Tjernlund
Date Oct. 16, 2009, 8:16 a.m.
Message ID <OFF665748B.AAF26319-ONC1257651.00296F23-C1257651.002D6BF7@transmode.se>
Download mbox | patch
Permalink /patch/36180/
State Superseded
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Joakim Tjernlund - Oct. 16, 2009, 8:16 a.m.
Rex Feany <RFeany@mrv.com> wrote on 15/10/2009 18:56:50:
>
> arch/powerpc/kernel/head_8xx.o: In function `FixupDAR':
> /home/rfeany/src/lnxnm/linux-dev/arch/powerpc/kernel/head_8xx.S:576: undefined
> reference to `DARfix'
>
> With all of your patches applied I have this problem:
>
> open("/proc/mounts", O_RDONLY)          = 3
> fstat64(0x3, 0x7fc6ad58)                = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x3001f000
> read(3, 0x3001f000, 1024)               = -1 EFAULT (Bad address)
> exit_group(0)                           = ?
>
> but it works fine with /dev/zero:
>
> open("/dev/zero", O_RDONLY)             = 3
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x30001000
> read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0
> \0"..., 1024) = 1024
>
> If I revert "8xx: start using dcbX instructions in various copy
> routines" then it works again. I think it is the cache instructions
> added to __copy_tofrom_user: reading from /dev/zero is OK (it uses
> __clear_user, no dcbX), but copy_to_user() fails.

Yes, only copy_tofrom_user will actually case a TLBError with
the dcbX insn.

>
> It seems stable with all but the dcbX patch applied. I haven't been able
> to crash it yet, anyway :)

Right, it is the pte table walk that is blowing up.
I just noted that 2.6 lacks a tophys() call in its table walk
so I removed that one(there is one more tophys call but I don't think
it should be removed).
Try this addon patch:
Rex Feany - Oct. 16, 2009, 8:25 p.m.
Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):

> Right, it is the pte table walk that is blowing up.
> I just noted that 2.6 lacks a tophys() call in its table walk
> so I removed that one(there is one more tophys call but I don't think
> it should be removed).
> Try this addon patch:

no difference

take care!
/rex.
Joakim Tjernlund - Oct. 17, 2009, 11:24 a.m.
Rex Feany <RFeany@mrv.com> wrote on 16/10/2009 22:25:41:
>
> Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
>
> > Right, it is the pte table walk that is blowing up.
> > I just noted that 2.6 lacks a tophys() call in its table walk
> > so I removed that one(there is one more tophys call but I don't think
> > it should be removed).
> > Try this addon patch:
>
> no difference

OK, thinking a bit more, this part should not be executed as
copy_tofrom_user executes in kernel space.

Any chance you can stick a HW breakpoint on FixupDAR?
Perhaps there is something different with kernel
virtual address to phys address?
A simple topys() works in 2.4, but perhaps not in 2.6?
this is the part of interest:
FixupDAR:	/* Entry point for dcbx workaround. */
      /* fetch instruction from memory. */
      mfspr r10, SPRN_SRR0
      andis.      r11, r10, 0x8000
      tophys  (r11, r10)
      beq-  139b		/* Branch if user space address */
140:  lwz   r11,0(r11)

If not kernel dcbX works, you could see if user space does.
Here is a start:

#include <errno.h>
#include <sys/mman.h>
#include <stdio.h>

dcbz(void const *ptr)
{
      __asm__ ("dcbz 0, %0" : : "r" (ptr) : "memory");
}

dcbf(void const *ptr)
{
      __asm__ ("dcbf 0, %0" : : "r" (ptr) : "memory");
}

dcbi(void const *ptr)
{
      __asm__ ("dcbi 0, %0" : : "r" (ptr) : "memory");
}

dcbst(void const *ptr)
{
      __asm__ ("dcbst 0, %0" : : "r" (ptr) : "memory");
}

icbi(void const *ptr)
{
      __asm__ ("icbi 0, %0" : : "r" (ptr) : "memory");
}

dcbt(void const *ptr) /* no TLB Miss */
{
      __asm__ ("dcbt 0, %0" : : "r" (ptr) : "memory");
}

dcbtst(void const *ptr) /* no TLB Miss */
{
      __asm__ ("dcbtst 0, %0" : : "r" (ptr) : "memory");
}

static const unsigned long const a[16*4094] ;
main()
{
      volatile unsigned long b, *ptr = &a[10*4096], *mptr;
      b = *ptr;

      mptr = mmap(NULL, 16*4094, PROT_WRITE | PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
      if (mptr == MAP_FAILED)
            printf("mmap failed:%s\n", strerror(errno));

      ptr = mptr;
#if 0
      dcbst(&a[2*4096]);
      b = *ptr;
      dcbf(&a[6*4096]);
      b = *ptr;
      dcbz(&a[4*4096]);
      b = *ptr;
      dcbst(&a[8*4096]);
      dcbst(&a[8*4096]);
#endif
      *ptr = 17;
      printf("dcbst(ptr)\n"); fflush(stdout);
      dcbst(ptr);
      *(ptr+3+1024) = 18;

      printf("dcbst(ptr+3*1024)\n"); fflush(stdout);
      dcbst(ptr+3*1024);

      printf("dcbt(ptr+5*1024)\n"); fflush(stdout);
      dcbt(ptr+5*1024);

      printf("dcbz(ptr+2*1024)\n"); fflush(stdout);
      dcbf(&a[6*4096]);

      printf("dcbz(&a[4*4096])\n"); fflush(stdout);
      dcbz(&a[4*4096]); fflush(stdout);

      printf("dcbf(&a[6*4096])\n"); fflush(stdout);
      dcbf(&a[6*4096]);

      //dcbz(ptr+2*1024);
      //dcbi(&a[6*4096]);
      //icbi(&a[8*4096]);
}

Patch

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 3df4a17..0e91da4 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -540,7 +540,6 @@  DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR
      mtspr SPRN_MD_EPN, r10
      mfspr r11, SPRN_M_TWB	/* Get level 1 table entry address */
      lwz   r11, 0(r11)	/* Get the level 1 entry */
-     tophys  (r11, r11)
      DO_8xx_CPU6(0x3b80, r3)
      mtspr SPRN_MD_TWC, r11	/* Load pte table base address */
      mfspr r11, SPRN_MD_TWC	/* ....and get the pte address */