Skip to contentSkip to conversation

KVM: PPC: elide struct thread_struct instances from stack

PATCH Applied Andreas Schwab 6 participants 7 messages May 31, 2010 – Jun 2, 2010 3 messages not in the archive

Subsystems (3)
KVM PowerPC KVM PowerPC
lore
Share & tools
lore

Andreas Schwab

author
#0 lore raw

PATCH 1/1 KVM: PPC: elide struct thread_struct instances from stack

Instead of instantiating a whole thread_struct on the stack use only the required parts of it.

Signed-off-by Andreas Schwab <[email protected]>
Tested-by Alexander Graf <[email protected]>
---
 arch/powerpc/include/asm/kvm_fpu.h       |   27 +++++----
 arch/powerpc/kernel/ppc_ksyms.c          |    4 -
 arch/powerpc/kvm/book3s.c                |   49 +++++++++-------
 arch/powerpc/kvm/book3s_paired_singles.c |   94 ++++++++++++------------------
 arch/powerpc/kvm/fpu.S                   |   18 ++++++
 5 files changed, 97 insertions(+), 95 deletions(-)
arch/powerpc/include/asm/kvm_fpu.h +15 -12
index 94f05de..c3d4f05 100644
@@ -22,24 +22,24 @@
 
 #include <linux/types.h>
 
-extern void fps_fres(struct thread_struct *t, u32 *dst, u32 *src1);
-extern void fps_frsqrte(struct thread_struct *t, u32 *dst, u32 *src1);
-extern void fps_fsqrts(struct thread_struct *t, u32 *dst, u32 *src1);
+extern void fps_fres(u64 *fpscr, u32 *dst, u32 *src1);
+extern void fps_frsqrte(u64 *fpscr, u32 *dst, u32 *src1);
+extern void fps_fsqrts(u64 *fpscr, u32 *dst, u32 *src1);
 
-extern void fps_fadds(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2);
-extern void fps_fdivs(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2);
-extern void fps_fmuls(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2);
-extern void fps_fsubs(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2);
+extern void fps_fadds(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2);
+extern void fps_fdivs(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2);
+extern void fps_fmuls(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2);
+extern void fps_fsubs(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2);
 
-extern void fps_fmadds(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2,
+extern void fps_fmadds(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2,
 		       u32 *src3);
-extern void fps_fmsubs(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2,
+extern void fps_fmsubs(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2,
 		       u32 *src3);
-extern void fps_fnmadds(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2,
+extern void fps_fnmadds(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2,
 		        u32 *src3);
-extern void fps_fnmsubs(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2,
+extern void fps_fnmsubs(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2,
 		        u32 *src3);
-extern void fps_fsel(struct thread_struct *t, u32 *dst, u32 *src1, u32 *src2,
+extern void fps_fsel(u64 *fpscr, u32 *dst, u32 *src1, u32 *src2,
 		     u32 *src3);
 
 #define FPD_ONE_IN(name) extern void fpd_ ## name(u64 *fpscr, u32 *cr, \

@@ -82,4 +82,7 @@ FPD_THREE_IN(fmadd)
 FPD_THREE_IN(fnmsub)
 FPD_THREE_IN(fnmadd)
 
+extern void kvm_cvt_fd(u32 *from, u64 *to, u64 *fpscr);
+extern void kvm_cvt_df(u64 *from, u32 *to, u64 *fpscr);
+
 #endif
arch/powerpc/kernel/ppc_ksyms.c +0 -4
index bc9f39d..ab3e392 100644
@@ -101,10 +101,6 @@ EXPORT_SYMBOL(pci_dram_offset);
 EXPORT_SYMBOL(start_thread);
 EXPORT_SYMBOL(kernel_thread);
 
-#ifndef CONFIG_BOOKE
-EXPORT_SYMBOL_GPL(cvt_df);
-EXPORT_SYMBOL_GPL(cvt_fd);
-#endif
 EXPORT_SYMBOL(giveup_fpu);
 #ifdef CONFIG_ALTIVEC
 EXPORT_SYMBOL(giveup_altivec);
arch/powerpc/kvm/book3s.c +27 -22

88 lines not on this page · raw message

arch/powerpc/kvm/book3s_paired_singles.c +37 -57

285 lines not on this page · raw message

arch/powerpc/kvm/fpu.S +18 -0
index 2b340a3..cb34bbe 100644
@@ -271,3 +271,21 @@ FPD_THREE_IN(fmsub)
 FPD_THREE_IN(fmadd)
 FPD_THREE_IN(fnmsub)
 FPD_THREE_IN(fnmadd)
+
+_GLOBAL(kvm_cvt_fd)
+	lfd	0,0(r5)			/* load up fpscr value */
+	MTFSF_L(0)
+	lfs	0,0(r3)
+	stfd	0,0(r4)
+	mffs	0
+	stfd	0,0(r5)			/* save new fpscr value */
+	blr
+
+_GLOBAL(kvm_cvt_df)
+	lfd	0,0(r5)			/* load up fpscr value */
+	MTFSF_L(0)
+	lfd	0,0(r3)
+	stfs	0,0(r4)
+	mffs	0
+	stfd	0,0(r5)			/* save new fpscr value */
+	blr
Signature · 9 lines
1.7.1


Andreas.

-- 
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
In reply to a message not in the archive
Quoted 2 lines · Andreas Schwab

Instead of instantiating a whole thread_struct on the stack use only the required parts of it.

...

Quoted 7 lines
arch/powerpc/kvm/fpu.S +7 -0
+_GLOBAL(kvm_cvt_fd)
+	lfd	0,0(r5)			/* load up fpscr value */
+	MTFSF_L(0)
+	lfs	0,0(r3)
+	stfd	0,0(r4)
+	mffs	0
+	stfd	0,0(r5)			/* save new fpscr value */

arch/powerpc/kvm/fpu.S FPD_THREE_IN(fmsub) · line 291

Quoted 10 lines
arch/powerpc/kvm/fpu.S +10 -0
+	blr
+
+_GLOBAL(kvm_cvt_df)
+	lfd	0,0(r5)			/* load up fpscr value */
+	MTFSF_L(0)
+	lfd	0,0(r3)
+	stfs	0,0(r4)
+	mffs	0
+	stfd	0,0(r5)			/* save new fpscr value */
+	blr

I re-read the relevant part of the PowerPC architecture spec yesterday, and it seems pretty clear that the FPSCR doesn't affect the behaviour of lfs and stfs, and is not affected by them. So in fact 4 out of the 7 instructions in each of those procedures are unnecessary (and similarly for the cvt_fd/df used in the alignment fixup code).

Paul.

#4 lore raw
Quoted 22 lines · Paul Mackerras
Quoted 2 lines · Andreas Schwab

Instead of instantiating a whole thread_struct on the stack use only the required parts of it.

...

Quoted 15 lines
arch/powerpc/kvm/fpu.S +15 -0
+_GLOBAL(kvm_cvt_fd)
+	lfd	0,0(r5)			/* load up fpscr value */
+	MTFSF_L(0)
+	lfs	0,0(r3)
+	stfd	0,0(r4)
+	mffs	0
+	stfd	0,0(r5)			/* save new fpscr value */
+	blr
+
+_GLOBAL(kvm_cvt_df)
+	lfd	0,0(r5)			/* load up fpscr value */
+	MTFSF_L(0)
+	lfd	0,0(r3)
+	stfs	0,0(r4)
+	mffs	0
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	stfd	0,0(r5)			/* save new fpscr value */
+	blr

I re-read the relevant part of the PowerPC architecture spec yesterday, and it seems pretty clear that the FPSCR doesn't affect the behaviour of lfs and stfs, and is not affected by them. So in fact 4 out of the 7 instructions in each of those procedures are unnecessary (and similarly for the cvt_fd/df used in the alignment fixup code).

So the rounding control field is not used on lfs? Interesting. I couldn't find a reference to it being used or modified either though.

Alex

A message that is not in the archive (sent off-list or not yet received): 20100531224040.GA22927-oklZEfemRj05kJ7NmlRacFaTQe2KTcn/@public.gmane.org

#6 lore raw
Quoted 5 lines · Paul Mackerras

I re-read the relevant part of the PowerPC architecture spec yesterday, and it seems pretty clear that the FPSCR doesn't affect the behaviour of lfs and stfs, and is not affected by them. So in fact 4 out of the 7 instructions in each of those procedures are unnecessary (and similarly for the cvt_fd/df used in the alignment fixup code).

I'd prefer to have this deferred to a separate patch.

Andreas.

Signature · 3 lines
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

A message that is not in the archive (sent off-list or not yet received): [email protected]

#8 lore raw
Quoted 9 lines · Andreas Schwab
Quoted 5 lines · Paul Mackerras

I re-read the relevant part of the PowerPC architecture spec yesterday, and it seems pretty clear that the FPSCR doesn't affect the behaviour of lfs and stfs, and is not affected by them. So in fact 4 out of the 7 instructions in each of those procedures are unnecessary (and similarly for the cvt_fd/df used in the alignment fixup code).

I'd prefer to have this deferred to a separate patch.

I agree. Andreas' patch takes the current logic and moves it to be KVM contained, so we don't clutter the stack. The fact that the old code was inefficient is a separate story.

Avi / Marcelo, please apply the patch nevertheless.

Alex

Applied
#9 lore raw
In reply to Andreas Schwab
Quoted 5 lines · Andreas Schwab

Instead of instantiating a whole thread_struct on the stack use only the required parts of it.

Signed-off-by Andreas Schwab <[email protected]>
Tested-by Alexander Graf <[email protected]>
Quoted 7 lines
---
 arch/powerpc/include/asm/kvm_fpu.h       |   27 +++++----
 arch/powerpc/kernel/ppc_ksyms.c          |    4 -
 arch/powerpc/kvm/book3s.c                |   49 +++++++++-------
 arch/powerpc/kvm/book3s_paired_singles.c |   94 ++++++++++++------------------
 arch/powerpc/kvm/fpu.S                   |   18 ++++++
 5 files changed, 97 insertions(+), 95 deletions(-)

Applied, thanks.