Tested-by: Sebastian Macke <[email protected]>
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 <[email protected]> --- 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 [email protected] http://lists.openrisc.net/listinfo/linux
