Tested-by: Sebastian Macke <sebast...@macke.de>

By flushing the tlb before and after the printk commands I could force the bug in the old version. Testing it with the new patch no kernel panic occurs.

On 13/02/2013 11:25 PM, Jonas Bonn wrote:
The self-modifying code that updates the TLB handler at start-up has
a subtle ordering requirement:  the DTLB handler must be the last thing
changed.

What I was seeing was the following:

i)   The DTLB handler was updated
ii)  The following printk caused a TLB miss and the look-up resulted
      in the page containing itlb_vector (0xc0000a00) being bounced from
      the TLB.
iii) The subsequent access to itlb_vector caused a TLB miss and reload
      of the page containing itlb_vector from the page tables.
iv)  But this reload of the page in iii) was being done by the "new"
      DTLB-miss handler which resulted (correctly) in the page flags being
      set to read-only; the subsequent write-access to itlb_vector thus
      resulted in a page (access) fault.

This is easily remedied if we ensure that the boot-time DTLB-miss handler
continues running until the very last bit of self-modifying code has been
executed.  This patch should ensure that the very last thing updated is the
DTLB-handler itself.

Signed-off-by: Jonas Bonn <jo...@southpole.se>
---
  arch/openrisc/mm/init.c | 17 ++++++++++++++---
  1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
index 79dea97..e7fdc50 100644
--- a/arch/openrisc/mm/init.c
+++ b/arch/openrisc/mm/init.c
@@ -167,15 +167,26 @@ void __init paging_init(void)
                unsigned long *dtlb_vector = __va(0x900);
                unsigned long *itlb_vector = __va(0xa00);
+ printk(KERN_INFO "itlb_miss_handler %p\n", &itlb_miss_handler);
+               *itlb_vector = ((unsigned long)&itlb_miss_handler -
+                               (unsigned long)itlb_vector) >> 2;
+
+               /* Soft ordering constraint to ensure that dtlb_vector is
+                * the last thing updated
+                */
+               barrier();
+
                printk(KERN_INFO "dtlb_miss_handler %p\n", &dtlb_miss_handler);
                *dtlb_vector = ((unsigned long)&dtlb_miss_handler -
                                (unsigned long)dtlb_vector) >> 2;
- printk(KERN_INFO "itlb_miss_handler %p\n", &itlb_miss_handler);
-               *itlb_vector = ((unsigned long)&itlb_miss_handler -
-                               (unsigned long)itlb_vector) >> 2;
        }
+ /* Soft ordering constraint to ensure that cache invalidation and
+        * TLB flush really happen _after_ code has been modified.
+        */
+       barrier();
+
        /* Invalidate instruction caches after code modification */
        mtspr(SPR_ICBIR, 0x900);
        mtspr(SPR_ICBIR, 0xa00);

_______________________________________________
Linux mailing list
Linux@lists.openrisc.net
http://lists.openrisc.net/listinfo/linux

Reply via email to