linux-kernel - Re: [PATCH 06/10] MIPS: pm-cps: Use MIPS standard lightweight ordering barrier

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] [day] [month] [year] [list]

Date:   Wed, 31 Aug 2016 16:49:24 +0100
From:   Matt Redfearn <matt.redfearn@...tec.com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     Ralf Baechle <ralf@...ux-mips.org>, <linux-mips@...ux-mips.org>,
        Adam Buchbinder <adam.buchbinder@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        <linux-kernel@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Markos Chandras <markos.chandras@...tec.com>,
        Paul Burton <paul.burton@...tec.com>
Subject: Re: [PATCH 06/10] MIPS: pm-cps: Use MIPS standard lightweight
 ordering barrier



On 31/08/16 15:28, Peter Zijlstra wrote:
> On Wed, Aug 31, 2016 at 02:36:26PM +0100, Matt Redfearn wrote:
>> The code previously had 0x10 as a magic number, this patch just replaces
>> that with a #defined name. The value is documented in the MIPS64 instruction
>> set manual, https://imgtec.com/?do-download=4302, table 6.5.
>>
>> This sync type has been standard since MIPSr2. That document also states
>> that "If an implementation does not use one of these non-zero values to
>> define a different synchronization behavior, then that non-zero value of
>> stype must act the same as stype zero completion barrier." As such,
>> stype_ordering can always be set to this sync type rather than setting it
>> only for certain CPUs.

Hi Peter,

> Right. We all had a bunch of fun trying to decode that manual a while
> back, and IIRC were left with a bunch of questions on what it all meant
> in 3+ CPU scenarios.

Yes, I remember that fun....

>
> In anycase, not sure why I was Cc'ed to this patch, but in general I

Patman decided to CC you as you've touched arch/mips/include/barrier.h I 
suppose.

> have low confidence in barrier patches that lack lots of detail. And the
> code in question has woefully inadequate comments:
>
>                  /* Ordering barrier */
>                  uasm_i_sync(&p, stype_ordering);
>
> Order what against what and why? Is my first question. A comment really
> should explain.

Fair enough - we'll put something together to improve the comments.

>
> In any case, you've removed the only (runtime) assignment to the
> variable, it can become 'const'.

True enough.

Thanks,
Matt