Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
AI-Spam-Status: Not processed
Subject: Re: [PATCH] MIPS: KASLR: Fix sync_icache() trapped in loop when
 synci_step is zero
To:     Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Jinyang He <hejinyang@loongson.cn>
Cc:     linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org
References: <1606878005-11427-1-git-send-email-hejinyang@loongson.cn>
 <20201202103943.GA9065@alpha.franken.de>
From:   Jiaxun Yang <jiaxun.yang@flygoat.com>
Message-ID: <b6e97d2f-6d5a-1dbb-3701-b238709b7345@flygoat.com>
Date:   Thu, 3 Dec 2020 12:02:10 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.3
MIME-Version: 1.0
In-Reply-To: <20201202103943.GA9065@alpha.franken.de>
Content-Type: text/plain; charset=gbk; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


?? 2020/12/2 ????6:39, Thomas Bogendoerfer д??:
> On Wed, Dec 02, 2020 at 11:00:05AM +0800, Jinyang He wrote:
>> Reading synci_step by using rdhwr instruction may return zero if no cache
>> need be synchronized. On the one hand, to make sure all load operation and
>> store operation finished we do __sync() for every platform. On the other
>> hand, some platform need operate synci one time although step is zero.
> Should this be someting like: Avoid endless loop, if no synci is needed ?
>
>> diff --git a/arch/mips/kernel/relocate.c b/arch/mips/kernel/relocate.c
>> index 57bdd276..47aeb33 100644
>> --- a/arch/mips/kernel/relocate.c
>> +++ b/arch/mips/kernel/relocate.c
>> @@ -64,7 +64,7 @@ static void __init sync_icache(void *kbase, unsigned long kernel_length)
>>   			: "r" (kbase));
>>   
>>   		kbase += step;
>> -	} while (kbase < kend);
>> +	} while (step && kbase < kend);
> why not do a
>
> 	if (step == 0)
> 		return;
>
> before entering the loop ? According to MIPS32PRA no synci is needed,
> if stepi value is zero.
>
> Thomas.
>
> PS: Does anybody know a reason, why this code doesn't use an old fashioned
> dache/icache flushing, which might be slower but would work also on
> legecy cores ?

I thought that's because legacy flush requires much more cares.
You'll have to probe cache ways sets and line size to do so.
However relocation happens very early, even before cache probe.

Thanks.

- Jiaxun