Why are imprecise external aborts masked on recent kernel while booting ?

DISCUSSION Fabrice Gasnier 2 participants 5 messages Jan 31, 2014 – Feb 7, 2014 Arm (32-bit)

Share & tools

Fabrice Gasnier

author

2014-01-31 15:59 UTC #0 lore raw

Hi all,

I'm working on a PCIe driver that uses an "imprecise external abort" handler to detect when a port is behind a switch.
This mechanism is used when enumerating PCIe bus upon kernel boot.

In prior kernel (3.4), I noticed it's possible to use an abort handler registered by using hook_fault_code(16+6, ...);
These aborts are detected and the relevant handler is called as long as it is registered, when probing the bus.

In more recent kernel (3.10), abort handler is no longer triggered during kernel boot. At PCIe bus enumeration, imprecise aborts are not enabled. It seems unmasked later when starting userland init process (e.g. when CPSR.A bit on arm is cleared). Aborts are deferred until then.

I found another thread that looks like similar :
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/208641.html

I looked for where imprecise aborts were enabled on 3.4. I found out that it was enabled when the first schedule() happens. More precisely in
kernel_thread_helper() when doing: "msr cpsr_c, r7".
There has been changes beetween 3.6 or 3.7, like in commit 9e14f828ee4a7a4a98703e380d180717a579fb35 (and others) that i believe changes the behavior of unmasking CPSR.A bit. kernel_thread_helper has been removed in that patch.

Is it a desired change in recent kernels ?

Is it possible to unmask imprecise data aborts earlier in the boot process (e.g. before PCIe bus enumeration, when drivers are being probed) ?

Best regards,
Fabrice

Russell King - ARM Linux

2014-01-31 17:08 UTC #1 lore raw

Quoted 1 line · Fabrice Gasnier

Is it a desired change in recent kernels ?

I think it was an unnoticed behavioural change.

Quoted 3 lines

Is it possible to unmask imprecise data aborts earlier in the boot process (e.g. before PCIe bus enumeration, when drivers are being probed) ?

How about this patch?

arch/arm/kernel/traps.c +9 -0

index 172ee18ff124..b0ff06f49cd0 100644
@@ -900,6 +900,15 @@ void __init early_trap_init(void *vectors_base)
 
 	flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
 	modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
+
+	/* Enable imprecise aborts */
+	asm volatile(
+		"mrs	%0, cpsr\n"
+	"	bic	%0, %0, %1\n"
+	"	msr	cpsr_c, %0"
+		: "=&r" (i)
+		: "r" (PSR_A_BIT));
+
 #else /* ifndef CONFIG_CPU_V7M */
 	/*
 	 * on V7-M there is no need to copy the vector table to a dedicated

Signature · 3 lines

FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

Fabrice Gasnier

author

2014-02-03 09:12 UTC #2 lore raw

Hi Russell,

Thank you for your help.
I just tried following patch on both 3.10 and above vanilla 3.13.1. Unfortunately, these instructions have no effect on the arm cpsr. I dumped regs right after msr instruction have been executed. It remains untouched :

Here is assembly from gdb:

    0xc064a400 <+128>:    mov    r3, #256    ; 0x100
    0xc064a404 <+132>:    mrs    r2, CPSR
    0xc064a408 <+136>:    bic    r2, r2, r3
    0xc064a40c <+140>:    msr    CPSR_c, r2

CPSR.A bit is still set after these instructions : 0x600001d3 Although, I see it has been cleared in r2: 0x600000d3

Please advise.
Thanks,
BR,
Fabrice

Quoted 25 lines · Russell King - ARM Linux

Quoted 3 lines

Is it possible to unmask imprecise data aborts earlier in the boot
Quoted 2 lines
process (e.g. before PCIe bus enumeration, when drivers are being probed) ?

How about this patch?

arch/arm/kernel/traps.c +9 -0

index 172ee18ff124..b0ff06f49cd0 100644
@@ -900,6 +900,15 @@ void __init early_trap_init(void *vectors_base)
 
  	flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
  	modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
+
+	/* Enable imprecise aborts */
+	asm volatile(
+		"mrs	%0, cpsr\n"
+	"	bic	%0, %0, %1\n"
+	"	msr	cpsr_c, %0"
+		: "=&r" (i)
+		: "r" (PSR_A_BIT));
+
  #else /* ifndef CONFIG_CPU_V7M */
  	/*
  	 * on V7-M there is no need to copy the vector table to a dedicated

Fabrice Gasnier

author

2014-02-03 16:43 UTC #3 lore raw

Hi,

I had no success with msr instruction to set CPSR.A bit.

I re-checked on a 3.4 kernel, msr instruction present in former "kernel_thread_helper()" routine was not responsible for clearing CPSR.A bit.
'A' bit was cleared because SPSR was altered before executing following instruction in arch/arm/kernel/entry-header.S :
movs pc, lr @ return & move spsr_svc into cpsr

Sorry for the confusion in my first email: that movs instruction was responsible for clearing 'A' bit on 3.4 kernel. But on recent kernel, "restore_user_regs" macro seems no longer called for a kernel thread.

So, I tried the 'cps'instruction that does it! I re-worked slightly your previous patch.
I also noticed that secondary needs to be set separately.

Please, could you comment on the following patch ? (I can resend correctly formated patch if you wish) :

arch/arm/kernel/smp.c +3 -0

index dc894ab..e22b109 100644
@@ -378,6 +378,9 @@ asmlinkage void secondary_start_kernel(void)
         local_irq_enable();
         local_fiq_enable();
 
+       /* Enable imprecise aborts */
+       asm volatile("cpsie     a");
+
         /*
          * OK, it's off to the idle thread for us
          */

arch/arm/kernel/traps.c +4 -0

index 4636d56..a9567bb 100644
@@ -900,6 +900,10 @@ void __init early_trap_init(void *vectors_base)
 
         flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
         modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
+
+       /* Enable imprecise aborts */
+       asm volatile("cpsie     a");
+
  #else /* ifndef CONFIG_CPU_V7M */
         /*
          * on V7-M there is no need to copy the vector table to a dedicated

Thanks,
BR,
Fabrice

Quoted 50 lines · Fabrice Gasnier

Hi Russell,
Thank you for your help.
I just tried following patch on both 3.10 and above vanilla 3.13.1. Unfortunately, these instructions have no effect on the arm cpsr. I dumped regs right after msr instruction have been executed. It remains untouched :
Here is assembly from gdb:
   0xc064a400 <+128>:    mov    r3, #256    ; 0x100
   0xc064a404 <+132>:    mrs    r2, CPSR
   0xc064a408 <+136>:    bic    r2, r2, r3
   0xc064a40c <+140>:    msr    CPSR_c, r2
CPSR.A bit is still set after these instructions : 0x600001d3 Although, I see it has been cleared in r2: 0x600000d3
Please advise.
Thanks,
BR,
Fabrice
Quoted 26 lines · Russell King - ARM Linux
Quoted 4 lines
Is it possible to unmask imprecise data aborts earlier in the boot
Quoted 1 line
process (e.g. before PCIe bus enumeration, when drivers are being
probed)
Quoted 1 line
?
How about this patch?
arch/arm/kernel/traps.c +9 -0
index 172ee18ff124..b0ff06f49cd0 100644
@@ -900,6 +900,15 @@ void __init early_trap_init(void *vectors_base)
        flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
      modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
+
+    /* Enable imprecise aborts */
+    asm volatile(
+        "mrs    %0, cpsr\n"
+    "    bic    %0, %0, %1\n"
+    "    msr    cpsr_c, %0"
+        : "=&r" (i)
+        : "r" (PSR_A_BIT));
+
  #else /* ifndef CONFIG_CPU_V7M */
      /*
       * on V7-M there is no need to copy the vector table to a 
dedicated

Fabrice Gasnier

author

2014-02-07 16:20 UTC #4 lore raw

Hi all,

I think there should be differences depending on ARM version.
I'm using armv7, "cpsie a" is ok,
On other arm version, it might be necessary to use different instruction as Russel advised previously.

Does it makes sense to add an abort enable/disable macro or function ?

I'm sending a separate patch to illustrate.

Thanks,
BR,
Fabrice

Quoted 109 lines · Fabrice Gasnier

Hi,
I had no success with msr instruction to set CPSR.A bit.
I re-checked on a 3.4 kernel, msr instruction present in former "kernel_thread_helper()" routine was not responsible for clearing CPSR.A bit.
'A' bit was cleared because SPSR was altered before executing following instruction in arch/arm/kernel/entry-header.S :
movs pc, lr @ return & move spsr_svc into cpsr
Sorry for the confusion in my first email: that movs instruction was responsible for clearing 'A' bit on 3.4 kernel. But on recent kernel, "restore_user_regs" macro seems no longer called for a kernel thread.
So, I tried the 'cps'instruction that does it! I re-worked slightly your previous patch.
I also noticed that secondary needs to be set separately.
Please, could you comment on the following patch ? (I can resend correctly formated patch if you wish) :
arch/arm/kernel/smp.c +3 -0
index dc894ab..e22b109 100644
@@ -378,6 +378,9 @@ asmlinkage void secondary_start_kernel(void)
        local_irq_enable();
        local_fiq_enable();
 
+       /* Enable imprecise aborts */
+       asm volatile("cpsie     a");
+
        /*
         * OK, it's off to the idle thread for us
         */
arch/arm/kernel/traps.c +4 -0
index 4636d56..a9567bb 100644
@@ -900,6 +900,10 @@ void __init early_trap_init(void *vectors_base)
 
        flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
        modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
+
+       /* Enable imprecise aborts */
+       asm volatile("cpsie     a");
+
 #else /* ifndef CONFIG_CPU_V7M */
        /*
         * on V7-M there is no need to copy the vector table to a 
dedicated
Thanks,
BR,
Fabrice
Quoted 50 lines · Fabrice Gasnier
Hi Russell,
Thank you for your help.
I just tried following patch on both 3.10 and above vanilla 3.13.1. Unfortunately, these instructions have no effect on the arm cpsr. I dumped regs right after msr instruction have been executed. It remains untouched :
Here is assembly from gdb:
   0xc064a400 <+128>:    mov    r3, #256    ; 0x100
   0xc064a404 <+132>:    mrs    r2, CPSR
   0xc064a408 <+136>:    bic    r2, r2, r3
   0xc064a40c <+140>:    msr    CPSR_c, r2
CPSR.A bit is still set after these instructions : 0x600001d3 Although, I see it has been cleared in r2: 0x600000d3
Please advise.
Thanks,
BR,
Fabrice
Quoted 26 lines · Russell King - ARM Linux
Quoted 4 lines
Is it possible to unmask imprecise data aborts earlier in the boot
Quoted 1 line
process (e.g. before PCIe bus enumeration, when drivers are being
probed)
Quoted 1 line
?
How about this patch?
arch/arm/kernel/traps.c +9 -0
index 172ee18ff124..b0ff06f49cd0 100644
@@ -900,6 +900,15 @@ void __init early_trap_init(void *vectors_base)
        flush_icache_range(vectors, vectors + PAGE_SIZE * 2);
      modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
+
+    /* Enable imprecise aborts */
+    asm volatile(
+        "mrs    %0, cpsr\n"
+    "    bic    %0, %0, %1\n"
+    "    msr    cpsr_c, %0"
+        : "=&r" (i)
+        : "r" (PSR_A_BIT));
+
  #else /* ifndef CONFIG_CPU_V7M */
      /*
       * on V7-M there is no need to copy the vector table to a 
dedicated