In message [ref] you wrote:

On Thu, May 19, 2011 at 12:58 AM, Michael Neuling [off-list ref] wrote=
:

quoted

Eric,

quoted

This patch adds save/restore register support for the BlueGene/P
double hummer FPU.

What does this mean? =A0Needs more details here.

Hi Mikey,

any specific details you are looking for here?  AFAIK these patches
are required for the kernel to save/restore the double hummer
properly.

I should have been more specific.  What does double hammer mean?

I description of how double hammer differs from normal and why a change
in the fpu code is needed would be great.

quoted

quoted

+#ifdef CONFIG_BGP
+#define LFPDX(frt, ra, rb) =A0 .long (31<<26)|((frt)<<21)|((ra)<<16)| \
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ((rb)<<11)|(462<<1)

quoted

quoted

+#define STFPDX(frt, ra, rb) =A0.long (31<<26)|((frt)<<21)|((ra)<<16)| \
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ((rb)<<11)|(974<<1)

quoted

quoted

+#endif /* CONFIG_BGP */

Put these in arch/powerpc/include/asm/ppc-opcode.h and reformat to fit
whats there already.

Also, don't need to put these defines inside a #ifdef.

Sure, I'll fix that up.

quoted

quoted

+#ifdef CONFIG_BGP
+#define SAVE_FPR(n, b, base) li b, THREAD_FPR0+(16*(n)); STFPDX(n, base=

, b)

quoted

quoted

+#define REST_FPR(n, b, base) li b, THREAD_FPR0+(16*(n)); LFPDX(n, base,=

 b)

quoted

16*? =A0Are these FP regs 64 or 128 bits wide? =A0If 128 you are doing to
have to play with TS_WIDTH to get the size of the FPs correct in the
thread_struct.

I think there's a bug here.

I actually have three different versions of this code from different
source patches that I'm drawing from - so your help in figuring out
the best way to approach this is appreciated.  The kittyhawk version
of the code has 8* instead of 16*.  According to the docs:
"Each of the two FPU units contains 32 64-bit floating point registers
for a total of 64 FP registers per processor." which would seem to
point to the kittyhawk version - but they have a second SAVE_32SFPRS
for the second hummer.  What wasn't clear to me with this version of
the code was whether or not they were doing something clever like
saving the pair of the 64-bit FPU registers in a single 128-bit slot
(seems plausible).  

Ok, sounds like there is 32*8*2 bytes of data, rather than the normal
32*8 bytes for FP only (ignoring VSX).  If this is the case, then you'll
need make 'fpr' in the thread struct bigger which you can do by setting
TS_FPRWIDTH = 2 like we do for VSX.

If there is some instruction that saves and restores two of these at a
time (which LFPDX/STFPDX might I guess), then we can use that, otherwise
we'll have to do 64 saves/restores.  Double load/stores will be faster
I'm guessing though.  

If two at a time, do we need to increase the index in pairs?

If this is not the way to go, I can certainly
switch the kittyhawk version of the patch with the *, the extra
SAVE32SFPR and the extra double hummer specific storage space in the
thread_struct.  

I'd be tempted to keep it in the 'fpr' part of the struct so you can
then access it with ptrace/signals/core dumps.

If it would help I can post an alternate version of the patch for
discussion with the kittyhawk version.

Sure.

The most useful thing would be to see the instruction definition for
STFPDX/LFPDX.

quoted

quoted

=A0/*

diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms=

/44x/

quoted

Kconfig

quoted

index f485fc5f..24a515e 100644

--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig

@@ -169,6 +169,15 @@ config YOSEMITE

=A0 =A0 =A0 help
=A0 =A0 =A0 =A0 This option enables support for the AMCC PPC440EP evalua=

tion board.

quoted

quoted

+config =A0 =A0 =A0 BGP

Does this FPU feature have a specific name like double hammer? =A0I'd
rather have the BGP defconfig depend on PPC_FPU_DOUBLE_HUMMER, or
something like that...

quoted

+ =A0 =A0 bool "Blue Gene/P"
+ =A0 =A0 depends on 44x
+ =A0 =A0 default n
+ =A0 =A0 select PPC_FPU
+ =A0 =A0 select PPC_DOUBLE_FPU

... in fact, it seem you are doing something like these here but you
don't use PPC_DOUBLE_FPU anywhere?

A fair point.  I'm fine with calling it DOUBLE_HUMMER, but I wasn't sure if
that was "too internal" of a name for the kernel.  Let me know and
I'll fix it up.

What I'm mostly concerned about is disassociating it with a particular
CPU.  

If it has an external name, then all the better.

I'll also change the CONFIG_BGP defines in the FPU code to PPC_DOUBLE_FPU
or PPC_DOUBLE_HUMMER depending on what the community decides.

Mikey