[PATCH]: powerpc: clarify use of twi/isync in io macros

Paul, please apply.

Clarify why twi appears in the i/o macros.

Signed-off-by: Linas Vepstas <redacted>
Cc: Segher Boessenkool <redacted>

----
 include/asm-powerpc/io.h |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-2.6.18-rc7-git1/include/asm-powerpc/io.h
===================================================================

--- linux-2.6.18-rc7-git1.orig/include/asm-powerpc/io.h	2006-09-14 13:38:48.000000000 -0500
+++ linux-2.6.18-rc7-git1/include/asm-powerpc/io.h	2006-09-19 14:26:26.000000000 -0500

@@ -278,6 +278,17 @@ static inline void iosync(void)
  * These routines do not perform EEH-related I/O address translation,
  * and should not be used directly by device drivers.  Use inb/readb
  * instead.
+ *
+ * Read operations have additional twi & isync to make sure the read
+ * is actually performed (i.e. the data has come back) before we start
+ * executing any following instructions.
+ *
+ * A data-dependent branch followed by an isync ensures that no
+ * instructions after the isync in program order will be
+ * (speculatively) executed, so the load that the twi depends
+ * on has to complete before anything else is executed; in
+ * particular, it's a barrier to keep MMIO reads ordered before
+ * main-storage accesses.
  */
 static inline int in_8(const volatile unsigned char __iomem *addr)
 {

+ *
+ * Read operations have additional twi & isync to make sure the read
+ * is actually performed (i.e. the data has come back) before we  
start
+ * executing any following instructions.
+ *
+ * A data-dependent branch followed by an isync ensures that no
+ * instructions after the isync in program order will be
+ * (speculatively) executed

..."before the isync has completed, and it won't complete until
the branch is resolved;"...

so the load that the twi depends
+ * on has to complete before anything else is executed; in
+ * particular, it's a barrier to keep MMIO reads ordered before
+ * main-storage accesses.
  */

You shouldn't post my test and put your signed-off on it, esp.
if I forgot to type half a sentence :-)  Here, have one though:

Signed-off-by: Segher Boessenkool <redacted>



Segher

On Tue, Sep 19, 2006 at 09:57:11PM +0200, Segher Boessenkool wrote:

..."before the isync has completed, and it won't complete until
the branch is resolved;"...

New improved patch attched.

You shouldn't post my test and put your signed-off on it, esp.
if I forgot to type half a sentence :-)  Here, have one though:

My moral fiber did twinge before I hit the send button,
but I confused it with a muscle spasm.

Signed-off-by: Segher Boessenkool <redacted>

--------

Clarify why twi appears in the i/o macros.

Signed-off-by: Linas Vepstas <redacted>
Signed-off-by: Segher Boessenkool <redacted>

----
 include/asm-powerpc/io.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Index: linux-2.6.18-rc7-git1/include/asm-powerpc/io.h
===================================================================

--- linux-2.6.18-rc7-git1.orig/include/asm-powerpc/io.h	2006-09-14 13:38:48.000000000 -0500
+++ linux-2.6.18-rc7-git1/include/asm-powerpc/io.h	2006-09-19 15:04:30.000000000 -0500

@@ -278,6 +278,18 @@ static inline void iosync(void)
  * These routines do not perform EEH-related I/O address translation,
  * and should not be used directly by device drivers.  Use inb/readb
  * instead.
+ *
+ * Read operations have additional twi & isync to make sure the read
+ * is actually performed (i.e. the data has come back) before we start
+ * executing any following instructions.
+ *
+ * A data-dependent branch followed by an isync ensures that no
+ * instructions after the isync in program order will be
+ * (speculatively) executed before the isync has completed, and
+ * the isync won't complete until the branch is resolved. The
+ * the load that the twi depends on has to complete before
+ * anything else is executed; in particular, it's a barrier to
+ * keep MMIO reads ordered before main-storage accesses.
  */
 static inline int in_8(const volatile unsigned char __iomem *addr)
 {

My moral fiber did twinge before I hit the send button,
but I confused it with a muscle spasm.

Heh.

Signed-off-by: Linas Vepstas <redacted>
Signed-off-by: Segher Boessenkool <redacted>

The other way around, or change mine to a From: -- you took my
stuff, not the other way around.  Oh well, doesn't matter as
far as I'm concerned.  Some people think that keeping the chain
in the correct order is very important though.  Let's just say
it was a cooperative work and fool them all :-)


Segher

Linas Vepstas writes:

+ * A data-dependent branch followed by an isync ensures that no
+ * instructions after the isync in program order will be
+ * (speculatively) executed, so the load that the twi depends
+ * on has to complete before anything else is executed; in

While the statement about branches is true, it's not especially
relevant when it comes to twi.  What's happening is that isync has to
prevent any following instructions from starting execution until all
previous instructions have completed, meaning that they have got to
the point of knowing whether they will generate an exception or not.
In general the processor doesn't know whether twi will generate an
exception until the data it depends on is available, and that's why
the isync has to wait for the previous load to have returned the
data.

+ * particular, it's a barrier to keep MMIO reads ordered before
+ * main-storage accesses.

Yes.

Paul.

quoted

+ * A data-dependent branch followed by an isync ensures that no
+ * instructions after the isync in program order will be
+ * (speculatively) executed, so the load that the twi depends
+ * on has to complete before anything else is executed; in

While the statement about branches is true, it's not especially
relevant when it comes to twi.

twi is a data-dependent branch -- all exceptions are branches (well
they do change the execution flow!), and this one is generated
directly by an insn even.  Traps and "normal" branches are neighbours
in the opcode maps, too, FWIW ;-)

What's happening is that isync has to
prevent any following instructions from starting execution until all
previous instructions have completed, meaning that they have got to
the point of knowing whether they will generate an exception or not.

It also has to know whether the isync will be executed at all, in
the sequential model.

In general the processor doesn't know whether twi will generate an
exception until the data it depends on is available, and that's why
the isync has to wait for the previous load to have returned the
data.

Yeah, the main point is that the load insn can complete _before_
the data has come back ("the storage access has been performed"),
so an isync without another insn depending on the load result
won't do.

Care to write some better explanatory text than the rubbish I
came up with?  :-)

Btw, you say "in general the CPU doesn't know", but it _can_ know
in this code, as the TO field is 0 (so it can never trap).  Is
there anything preventing/forbidding a CPU from optimising this
case?  I see it spelled out for "conditional branch" insns, but
it would be interesting to know if the arch guarantees no insn
will complete before its input regs are ready.