linux-kernel - Re: [PATCH] [bugfix] replace unnessary ldax with common ldr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <57C6DC00.2050709@arm.com>
Date:   Wed, 31 Aug 2016 14:30:40 +0100
From:   Vladimir Murzin <vladimir.murzin@....com>
To:     Catalin Marinas <catalin.marinas@....com>,
        Kenneth Lee <liguozhu@...ilicon.com>
Cc:     Will Deacon <will.deacon@....com>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] [bugfix] replace unnessary ldax with common ldr

On 30/08/16 10:07, Catalin Marinas wrote:
> On Tue, Aug 30, 2016 at 02:35:31PM +0800, Kenneth Lee wrote:
>> (add comment for the previous mail, sorry for the duplication)
>>
>> There is no store_ex pairing with this load_ex. It is not necessary and
>> gave wrong hint to the cache system.
>>
>> Signed-off-by: Kenneth Lee <liguozhu@...ilicon.com>
>> ---
>>  arch/arm64/include/asm/spinlock.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/spinlock.h b/arch/arm64/include/asm/spinlock.h
>> index c85e96d..3334c4f 100644
>> --- a/arch/arm64/include/asm/spinlock.h
>> +++ b/arch/arm64/include/asm/spinlock.h
>> @@ -63,7 +63,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
>>  	 */
>>  "	sevl\n"
>>  "2:	wfe\n"
>> -"	ldaxrh	%w2, %4\n"
>> +"	ldrh	%w2, %4\n"
>>  "	eor	%w1, %w2, %w0, lsr #16\n"
>>  "	cbnz	%w1, 2b\n"
>>  	/* We got the lock. Critical section starts here. */
> 
> This is needed because the arch_spin_unlock() code only uses an STLR
> without an explicit SEV (like we have on AArch32). An event is
> automatically generated when the exclusive monitor is cleared by STLR.
> But without setting it with a load exclusive in arch_spin_lock() (even
> though it does not acquire the lock), there won't be anything to clear,
> hence no event to be generated. In this case, the WFE would wait
> indefinitely.
> 

Maybe worth to add this as a comment, no?

Cheers
Vladimir