Re: [PATCH] arm64: Flush the process's mm context TLB entries when switching

[Ding Tianhong]

On 2014/4/14 21:01, Will Deacon wrote:
> Hi Ding,
> 
> On Mon, Apr 14, 2014 at 01:03:12PM +0100, Ding Tianhong wrote:
>> I met a problem when migrating process by following steps:
>>
>> 1) The process was already running on core 0.
>> 2) Set the CPU affinity of the process to 0x02 and move it to core 1,
>>    it could work well.
>> 3) Set the CPU affinity of the process to 0x01 and move it to core 0 again,
>>    the problem occurs and the process was killed.
> 
> [...]
> 
>> It was a very strange problem that the PC and LR are both 0, and the esr is
>> 0x83000006, it means that the used for instruction access generated MMU faults
>> and synchronous external aborts, including synchronous parity errors.
>>
>> I try to fix the problem by invalidating the process's TLB entries when switching,
>> it will make the context stale and pick new one, and then it could work well.
>>
>> So I think in some situation that after the process switching, the modification of
>> the TLB entries in the new core didn't inform all other cores to invalidate the old
>> TLB entries which was in the inner shareable caches, and then if the process schedule
>> to another core, the old TLB entries may occur MMU faults.
> 
> Yes, it sounds like you don't have your TLBs configured correctly. Can you
> confirm that your EL3 firmware is configuring TLB broadcasting correctly
> please?
> 

Hi will:

Do you mean the SCR_EL3.NS?

>> Signed-off-by: Ding Tianhong <dingtianhong@xxxxxxxxxx>
>> ---
>>  arch/arm64/kernel/process.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 6391485..d7d8439 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -283,6 +283,13 @@ static void tls_thread_switch(struct task_struct *next)
>>  	: : "r" (tpidr), "r" (tpidrro));
>>  }
>>  
>> +static void tlb_flush_thread(struct task_struct *prev)
>> +{
>> +	/* Flush the prev task's TLB entries */
>> +	if (prev->mm)
>> +		flush_tlb_mm(prev->mm);
>> +}
>> +
>>  /*
>>   * Thread switching.
>>   */
>> @@ -296,6 +303,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
>>  	hw_breakpoint_thread_switch(next);
>>  	contextidr_thread_switch(next);
>>  
>> +	tlb_flush_thread(prev);
> 
> NAK to the patch -- the architecture certainly doesn't require this, and
> it's a huge hammer for what is more likely a firmware initialisation issue.
> 
> Will
> 

Yep, I am still doubt with this patch, thanks for your suggestion.

Regards
Ding

> .
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/