linux-kernel - Re: powerpc: Add an inline function to update HID0

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20150805065400.GA14701@in.ibm.com>
Date:	Wed, 5 Aug 2015 12:24:00 +0530
From:	Gautham R Shenoy <ego@...ux.vnet.ibm.com>
To:	Segher Boessenkool <segher@...nel.crashing.org>
Cc:	Michael Ellerman <mpe@...erman.id.au>,
	"Gautham R. Shenoy" <ego@...ux.vnet.ibm.com>,
	Paul Mackerras <paulus@...ba.org>,
	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	mikey@...ling.org
Subject: Re: powerpc: Add an inline function to update HID0

Hi Segher,

Thanks for the suggestions. I will rename the function to
update_power8_hid0() and use asm volatile.


On Tue, Aug 04, 2015 at 09:30:57PM -0500, Segher Boessenkool wrote:
> On Tue, Aug 04, 2015 at 08:08:58PM +1000, Michael Ellerman wrote:
> > > +static inline void update_hid0(unsigned long hid0)
> > > +{
> > > +	/*
> > > +	 *  The HID0 update should at the very least be preceded by a
> > > +	 *  a SYNC instruction followed by an ISYNC instruction
> > > +	 */
> > > +	mb();
> > > +	mtspr(SPRN_HID0, hid0);
> > > +	isync();
> > 
> > That's going to turn into three separate inline asm blocks, which is maybe a
> > bit unfortunate. Have you checked the generated code is what we want, ie. just
> > sync, mtspr, isync ?
> 
> The "mb()" is not such a great name anyway: you don't want a memory
> barrier, you want an actual sync instruction ("sync 0", "hwsync",
> whatever the currently preferred spelling is).
> 
> The function name should also say this is for POWER8 (the required
> sequences are different for some other processors; and some others
> might not even _have_ a HID0, or not at 1008).  power8_write_hid0
> or such?
> 
> For writing it as one asm, why not just
> 
>   asm volatile("sync ; mtspr %0,%1 ; isync" : : "i"(SPRN_HID0), "r"(hid0));
> 
> instead of the stringify stuff?
> 
> 
> Segher
> 

--
Thanks and Regards
gautham.

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