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

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

[Fabrice Gasnier]

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

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

[Russell King - ARM Linux]

On Fri, Jan 31, 2014 at 04:59:41PM +0100, Fabrice Gasnier wrote:
> Is it a desired change in recent kernels ?

I think it was an unnoticed behavioural change.

> 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?

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 172ee18ff124..b0ff06f49cd0 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -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


-- 
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".

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

[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
On 01/31/2014 06:08 PM, Russell King - ARM Linux wrote:
>> 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?
>
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 172ee18ff124..b0ff06f49cd0 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -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

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

[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) :

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index dc894ab..e22b109 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -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
          */
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 4636d56..a9567bb 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -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

On 02/03/2014 10:12 AM, Fabrice Gasnier wrote:
> 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
> On 01/31/2014 06:08 PM, Russell King - ARM Linux wrote:
>>> 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?
>>
>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>> index 172ee18ff124..b0ff06f49cd0 100644
>> --- a/arch/arm/kernel/traps.c
>> +++ b/arch/arm/kernel/traps.c
>> @@ -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
>

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

[Fabrice Gasnier]

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

On 02/03/2014 05:43 PM, Fabrice Gasnier wrote:
> 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) :
>
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index dc894ab..e22b109 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -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
>          */
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 4636d56..a9567bb 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -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
>
> On 02/03/2014 10:12 AM, Fabrice Gasnier wrote:
>> 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
>> On 01/31/2014 06:08 PM, Russell King - ARM Linux wrote:
>>>> 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?
>>>
>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>>> index 172ee18ff124..b0ff06f49cd0 100644
>>> --- a/arch/arm/kernel/traps.c
>>> +++ b/arch/arm/kernel/traps.c
>>> @@ -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
>>
>