Skip to contentSkip to conversation

powerpc: Don't export cvt_fd & _df when CONFIG_PPC_FPU is not set

PATCH In review Benjamin Herrenschmidt 4 participants 14 messages May 31, 2010 – Jun 2, 2010 4 messages not in the archive

Subsystems (3)
PowerPC KVM PowerPC Linux for power macintosh
lore
Share & tools
lore

Benjamin Herrenschmidt

author
#0 lore raw

PATCH 1/1 powerpc: Don't export cvt_fd & _df when CONFIG_PPC_FPU is not set

Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
 arch/powerpc/kernel/ppc_ksyms.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/powerpc/kernel/ppc_ksyms.c +1 -1
index bc9f39d..3b4dcc8 100644
@@ -101,7 +101,7 @@ EXPORT_SYMBOL(pci_dram_offset);
 EXPORT_SYMBOL(start_thread);
 EXPORT_SYMBOL(kernel_thread);
 
-#ifndef CONFIG_BOOKE
+#ifdef CONFIG_PPC_FPU
 EXPORT_SYMBOL_GPL(cvt_df);
 EXPORT_SYMBOL_GPL(cvt_fd);
 #endif
author
#1 lore raw
Quoted 4 lines · Benjamin Herrenschmidt
Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
 arch/powerpc/kernel/ppc_ksyms.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Alex, this is just a temporary fix to remove the build breakage for 40x etc... but please, update KVM to just build-in its own.

Don't we have .o's that are always built into modules nowadays ?

Cheers,
Ben.

#2 lore raw
Quoted 8 lines · Benjamin Herrenschmidt
Quoted 4 lines · Benjamin Herrenschmidt
Signed-off-by Benjamin Herrenschmidt <[email protected]>

---
arch/powerpc/kernel/ppc_ksyms.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

Alex, this is just a temporary fix to remove the build breakage for 40x etc... but please, update KVM to just build-in its own.

What's the bad part in reusing the existing code?

Quoted 1 line

Don't we have .o's that are always built into modules nowadays ?

I don't think I understand what you mean.

Alex

#3 lore raw
Quoted 8 lines · Alexander Graf
Quoted 6 lines · Benjamin Herrenschmidt
Quoted 4 lines · Benjamin Herrenschmidt
Signed-off-by Benjamin Herrenschmidt <[email protected]>

---
arch/powerpc/kernel/ppc_ksyms.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 4 lines
Quoted 2 lines

Alex, this is just a temporary fix to remove the build breakage for 40x etc... but please, update KVM to just build-in its own.

What's the bad part in reusing the existing code?

The bad part is that KVM is wasting a ridiculous amount of stack space by allocating multiple instances of struct task_struct on it. I have a patch removing all of it, but I haven't tested it yet.

Andreas.

Signature · 3 lines
Andreas Schwab, [email protected]
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."
#4 lore raw
Quoted 16 lines · Andreas Schwab
Quoted 13 lines · Alexander Graf
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 5 lines · Benjamin Herrenschmidt
Signed-off-by Benjamin Herrenschmidt <[email protected]>

---
arch/powerpc/kernel/ppc_ksyms.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

Alex, this is just a temporary fix to remove the build breakage for 40x etc... but please, update KVM to just build-in its own.

Quoted 7 lines
Quoted 2 lines

What's the bad part in reusing the existing code?

The bad part is that KVM is wasting a ridiculous amount of stack space by allocating multiple instances of struct task_struct on it. I have a patch removing all of it, but I haven't tested it yet.

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Alex

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

#6 lore raw
Quoted 2 lines · Alexander Graf

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Still untested, because rc1 does not boot.

Andreas.

---
 arch/powerpc/include/asm/kvm_fpu.h       |   29 +++++----
 arch/powerpc/kvm/book3s.c                |   49 ++++++++--------
 arch/powerpc/kvm/book3s_paired_singles.c |   94 ++++++++++++-------------------
 arch/powerpc/kvm/fpu.S                   |   18 +++++
 4 files changed, 98 insertions(+), 92 deletions(-)
arch/powerpc/include/asm/kvm_fpu.h +16 -13
@@ -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_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_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(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/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
@@ -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 · 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."
#7 lore raw
Quoted 9 lines · Andreas Schwab
Quoted 3 lines · Alexander Graf

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Still untested, because rc1 does not boot.

Normal guests work great with the patch and from a quick look over it, it seems sane. I couldn't test the paired single implementation yet, because my userspace app for that is in a flux right now.

Either way, thanks a lot for the patch. Mind to wrap it up and submit it? I'll fix breakages later on. Please CC [email protected] and [email protected], so Avi or Marcelo can pick it up on the KVM tree.

Alex

author
#8 lore raw
In reply to Andreas Schwab
Quoted 6 lines · Andreas Schwab
Quoted 2 lines · Alexander Graf

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Still untested, because rc1 does not boot.

Ouch ! What machine ?

Cheers,
Ben.

A message that is not in the archive (sent off-list or not yet received): 1275384716.1931.711.camel__10281.3212587082$1275384770$gmane$org@pasglop

#10 lore raw
Quoted 9 lines · Benjamin Herrenschmidt
Quoted 6 lines · Andreas Schwab
Quoted 2 lines · Alexander Graf

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Still untested, because rc1 does not boot.

Ouch ! What machine ?

It's crashing in macio_pci_add_devices.

Andreas.

Signature · 3 lines
Andreas Schwab, [email protected]
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."
author
#11 lore raw
Quoted 5 lines · Andreas Schwab
Quoted 3 lines · Benjamin Herrenschmidt
Quoted 2 lines · Andreas Schwab

Alexander Graf <[email protected]> writes:

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Still untested, because rc1 does not boot.

Ouch ! What machine ?

It's crashing in macio_pci_add_devices.

Ok, so a G5 ? I'll have a look tomorrow.

Cheers,
Ben.

A message that is not in the archive (sent off-list or not yet received): 1275385607.1931.712.camel__30394.2679115154$1275385651$gmane$org@pasglop

#13 lore raw

PATCH 1/1 macio: don't dereference pointer before null check

Signed-off-by Andreas Schwab <[email protected]>
---
 drivers/macintosh/macio_asic.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
drivers/macintosh/macio_asic.c +4 -3
index 9714780..40390ac 100644
@@ -492,8 +492,8 @@ static void macio_pci_add_devices(struct macio_chip *chip)
 	}
 
 	/* Add media bay devices if any */
-	pnode = mbdev->ofdev.dev.of_node;
-	if (mbdev)
+	if (mbdev) {
+		pnode = mbdev->ofdev.dev.of_node;
 		for (np = NULL; (np = of_get_next_child(pnode, np)) != NULL;) {
 			if (macio_skip_device(np))
 				continue;

@@ -502,10 +502,11 @@ static void macio_pci_add_devices(struct macio_chip *chip)
 						 mbdev,  root_res) == NULL)
 				of_node_put(np);
 		}
+	}
 
 	/* Add serial ports if any */
-	pnode = sdev->ofdev.dev.of_node;
 	if (sdev) {
+		pnode = sdev->ofdev.dev.of_node;
 		for (np = NULL; (np = of_get_next_child(pnode, np)) != NULL;) {
 			if (macio_skip_device(np))
 				continue;
Signature · 6 lines
1.7.1

-- 
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]

#15 lore raw
Quoted 5 lines · Andreas Schwab
Quoted 3 lines · Benjamin Herrenschmidt
Quoted 2 lines · Andreas Schwab

Alexander Graf <[email protected]> writes:

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Mind to send it over so I can take a look at it :)? Getting rid of the task_struct structs lying in the stack is certainly a good idea.

Still untested, because rc1 does not boot.

Ouch ! What machine ?

It's crashing in macio_pci_add_devices.

And snd-aoa is broken, too.

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."
author
#16 lore raw
Quoted 4 lines
Quoted 2 lines

It's crashing in macio_pci_add_devices.

And snd-aoa is broken, too.

Right, all macio_driver's are. I have a fix, testing now.

Cheers,
Ben.

author
#17 lore raw
In reply to Alexander Graf
Quoted 5 lines · Alexander Graf
Quoted 1 line

Alex, this is just a temporary fix to remove the build breakage for

40x

Quoted 1 line

etc... but please, update KVM to just build-in its own.

What's the bad part in reusing the existing code?

I'm not saying we should dup the code, but I'm sure we can find a better way for that sort of stuff, along with other base support bits like you normally find in libgcc, to be linked directly with all modules.

calling out of modules symbols sucks for perf too anyways.

Quoted 3 lines
Quoted 1 line

Don't we have .o's that are always built into modules nowadays ?

I don't think I understand what you mean.

Ben.