Re: 2.6.37-rc7: Regression: b43: crashes in hwrng_register()

From: Mario 'BitKoenig' Holbe
Date: Thu Jan 06 2011 - 08:15:46 EST


Hello Herbert,

On Thu, Jan 06, 2011 at 05:12:41PM +1100, Herbert Xu wrote:
> On Wed, Jan 05, 2011 at 02:16:22PM +0100, Mario 'BitKoenig' Holbe wrote:
> > attached. I hope I got the gcc call right. However, I prefer the objdump
> > output anyways, so this one is attached as well.
> I see what you mean now.
> Please let me know if this patch (still against vanilla) helps.

The patch helps. No crashes, meaningful random data - perfect.
I still have 2 small annotations...

1. Having ECX on the clobber-list is not really necessary.
XSTORE doesn't touch ECX at all.
REP XSTORE would touch it, but for this ECX would be an input anyways.

2. Would you mind doing the same for EDX as you did for EDI?
This doesn't really change anything currently, but will probably help
avoiding a debug-session like ours somewhere in the future :)

A patch that does both is attached - to apply on top of your patch, if
you like. I tested this patch - it passed all my tests: no crashes,
meaningful random data.


Thanks for your help & regards
Mario
--
It is a capital mistake to theorize before one has data.
Insensibly one begins to twist facts to suit theories instead of theories
to suit facts. -- Sherlock Holmes by Arthur Conan Doyle
--- linux-source-2.6.37-rc7/drivers/char/hw_random/via-rng.c.hxu4 2011-01-06 09:40:44.879334924 +0100
+++ linux-source-2.6.37-rc7/drivers/char/hw_random/via-rng.c 2011-01-06 13:07:25.031181409 +0100
@@ -81,13 +81,7 @@
ts_state = irq_ts_save();

asm(".byte 0x0F,0xA7,0xC0 /* xstore %%edi (addr=%0) */"
- : "=m" (*addr), "=a" (eax_out), "+D" (addr)
- : "d" (edx_in)
-#ifdef CONFIG_64BIT
- : "%rcx");
-#else
- : "%ecx");
-#endif
+ : "=m" (*addr), "=a" (eax_out), "+D" (addr), "+d" (edx_in));

irq_ts_restore(ts_state);
return eax_out;

Attachment: signature.asc
Description: Digital signature