linux-kernel - Re: [PATCH 9/9] powerpc/pm: support deep sleep feature on T1040

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: <20140312055755.GA17203@pek-khao-d1.corp.ad.wrs.com>
Date:	Wed, 12 Mar 2014 13:57:55 +0800
From:	Kevin Hao <haokexin@...il.com>
To:	Scott Wood <scottwood@...escale.com>
Cc:	Chenhui Zhao <chenhui.zhao@...escale.com>,
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	leoli@...escale.com, Jason.Jin@...escale.com
Subject: Re: [PATCH 9/9] powerpc/pm: support deep sleep feature on T1040

On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
> > +	FSL_DIS_ALL_IRQ
> > +
> > +	/*
> > +	 * Place DDR controller in self refresh mode.
> > +	 * From here on, DDR can't be access any more.
> > +	 */
> > +	lwz	r10, 0(r13)
> > +	oris	r10, r10, CCSR_DDR_SDRAM_CFG_2_FRC_SR@h
> > +	stw	r10, 0(r13)
> > +
> > +	/* can't call udelay() here, so use a macro to delay */
> > +	FSLDELAY(50)
> 
> A timebase loop doesn't require accessing DDR.
> 
> You also probably want to do a "sync, readback, data dependency, isync"
> sequence to make sure that the store has hit CCSR before you begin your
> delay (or is a delay required at all if you do that?).

Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
  To guarantee that the results of any sequence of writes to configuration
  registers are in effect, the final configuration register write should be
  immediately followed by a read of the same register, and that should be
  followed by a SYNC instruction. Then accesses can safely be made to memory
  regions affected by the configuration register write.

> > +
> > +	/* Enable SCU15 to trigger on RCPM Concentrator 0 */
> > +	lwz	r10, 0(r15)
> > +	oris	r10, r10, DCSR_EPU_EPECR15_IC0@h
> > +	stw	r10, 0(r15)
> > +
> > +	/* put Core0 in PH15 mode, trigger EPU FSM */
> > +	lwz	r10, 0(r12)
> > +	ori	r10, r10, CCSR_RCPM_PCPH15SETR_CORE0
> > +	stw	r10, 0(r12)
> 
> Shouldn't there be a sync to ensure that the previous I/O happens before
> the final store to enter PH15?

Do we really need a sync here? According to the PowerISA, the above stores
should be performed in program order.
  If two Store instructions or two Load instructions
  specify storage locations that are both Caching
  Inhibited and Guarded, the corresponding storage
  accesses are performed in program order with
  respect to any processor or mechanism.

Thanks,
Kevin

Content of type "application/pgp-signature" skipped