[PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Harald Welte]

This is a cosmetic change, fixing the MODULE_DESCRIPTION() of via-rng.c

Signed-off-by: Harald Welte <HaraldWelte@viatech.com>
---
 drivers/char/hw_random/via-rng.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/char/hw_random/via-rng.c b/drivers/char/hw_random/via-rng.c
index eea0814..824f003 100644
--- a/drivers/char/hw_random/via-rng.c
+++ b/drivers/char/hw_random/via-rng.c
@@ -213,5 +213,5 @@ static void __exit mod_exit(void)
 module_init(mod_init);
 module_exit(mod_exit);
 
-MODULE_DESCRIPTION("H/W RNG driver for VIA chipsets");
+MODULE_DESCRIPTION("H/W RNG driver for VIA CPU with PadLock");
 MODULE_LICENSE("GPL");
-- 
1.5.6.5

-- 
- Harald Welte <HaraldWelte@viatech.com>	    http://linux.via.com.tw/
============================================================================
VIA Open Source Liaison

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Arjan Koers]

Harald Welte wrote:
> This is a cosmetic change, fixing the MODULE_DESCRIPTION() of via-rng.c

Coincidentally, I was trying to make my RNG work for x86_64 today and I
was wondering about this. How can multiple RNGs in current dual-processor
setups and in the future multicore Nano be handled?


The MSR wizardry in via_rng_init doesn't seem to work on my Nano. I'm
simply skipping it with the patch below, because my RNG is enabled
by default. I don't know the proper way to initialize it because of
lacking documentation. Would you happen to know a better way of doing
this?

diff -u linux-2.6.30-rc5.orig/drivers/char/hw_random/Kconfig linux-2.6.30-rc5/drivers/char/hw_random/Kconfig
--- linux-2.6.30-rc5.orig/drivers/char/hw_random/Kconfig	2009-04-23 20:13:55.000000000 +0000
+++ linux-2.6.30-rc5/drivers/char/hw_random/Kconfig	2009-05-10 09:59:59.000000000 +0000
@@ -88,7 +88,7 @@

 config HW_RANDOM_VIA
 	tristate "VIA HW Random Number Generator support"
-	depends on HW_RANDOM && X86_32
+	depends on HW_RANDOM && X86
 	default HW_RANDOM
 	---help---
 	  This driver provides kernel-side support for the Random Number
diff -u linux-2.6.30-rc5.orig/drivers/char/hw_random/via-rng.c linux-2.6.30-rc5/drivers/char/hw_random/via-rng.c
--- linux-2.6.30-rc5.orig/drivers/char/hw_random/via-rng.c	2009-03-23 23:12:14.000000000 +0000
+++ linux-2.6.30-rc5/drivers/char/hw_random/via-rng.c	2009-05-10 15:33:23.000000000 +0000
@@ -132,6 +132,17 @@
 	struct cpuinfo_x86 *c = &cpu_data(0);
 	u32 lo, hi, old_lo;

+	/* Nano */
+	if ((c->x86 == 6) && (c->x86_model >= 0xf)) {
+		if (!cpu_has_xstore_enabled) {
+			printk(KERN_ERR PFX
+			       "don't know how to enable VIA Nano RNG,"
+			       " aborting\n");
+			return -ENODEV;
+		}
+		return 0;
+	}
+
 	/* Control the RNG via MSR.  Tread lightly and pay very close
 	 * close attention to values written, as the reserved fields
 	 * are documented to be "undefined and unpredictable"; but it

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Harald Welte]

Dear Arjan,

On Mon, May 11, 2009 at 02:12:17AM +0200, Arjan Koers wrote:
> Harald Welte wrote:
> > This is a cosmetic change, fixing the MODULE_DESCRIPTION() of via-rng.c
> 
> Coincidentally, I was trying to make my RNG work for x86_64 today and I
> was wondering about this. 

I like those coincidences :)

> How can multiple RNGs in current dual-processor setups and in the future
> multicore Nano be handled?
 
That's actually a good question.  I'll probably forward that to the CPU
division and see what they come up with.  I would assume you just run the
initialization on every CPU, and that's it.

Now the more interesting question is: how can the hw_random framework deal
with it?  As far as I remember, it can only select one out of N available
concurrent providers of hardware randomness.  correct?

> The MSR wizardry in via_rng_init doesn't seem to work on my Nano. 

This is due to the fact that the MSR itself does no longer exist on the Nano.
It only exists on C3 to C7 silicon.

> I'm simply skipping it with the patch below, because my RNG is enabled by
> default. 

yes, this is the correct method.  On Nano you only have to see if it is enabled
in CPUID and forget about

> I don't know the proper way to initialize it because of lacking
> documentation. 

the quick reference for padlock on the nano is public, despite not
at a very well-known location:
ftp://ftp.vtbridge.org/Docs/CPU/Nano/padlock_quick_reference_V095.pdf

-- 
- Harald Welte <HaraldWelte@viatech.com>	    http://linux.via.com.tw/
============================================================================
VIA Open Source Liaison

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Harald Welte]

Dear Arjan,

it seems like it is probably a pragmatic solution to first merge your suggested
changes (Kconfig enable for X86_64 as well as skipping any MSR activity), before
adressing the auto-loading in a potential follow-up patch.

I have sent your modifications (split in two patches, with some additional
comment/explanation) to this list.

The question is: Since according to MAINTAINERS drivers/char/hw_random is
orphaned, which route should patches go?  

Herbert: Do you accept hw_random related patches, or should they go some
different route?

Regards,
-- 
- Harald Welte <HaraldWelte@viatech.com>	    http://linux.via.com.tw/
============================================================================
VIA Open Source Liaison

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Arjan Koers]

Hello Harald,

Harald Welte wrote:
> Dear Arjan,
[...]
>> How can multiple RNGs in current dual-processor setups and in the future
>> multicore Nano be handled?
>  
> That's actually a good question.  I'll probably forward that to the CPU
> division and see what they come up with.  I would assume you just run the
> initialization on every CPU, and that's it.

It would probably have to be setup the same on all CPUs (enabled on all /
disabled on all), because the code might execute on any CPU.


> Now the more interesting question is: how can the hw_random framework deal
> with it?  As far as I remember, it can only select one out of N available
> concurrent providers of hardware randomness.  correct?

I don't know, but it would be nice if it could use PadLock functionality on
multiple CPUs at the same time for increased speed.


>> The MSR wizardry in via_rng_init doesn't seem to work on my Nano. 
> 
> This is due to the fact that the MSR itself does no longer exist on the Nano.
> It only exists on C3 to C7 silicon.

Thanks for the information. Do you happen to know what changed for the other
MSRs that are used for ACE, PowerSaver, ...?


>> I'm simply skipping it with the patch below, because my RNG is enabled by
>> default. 
> 
> yes, this is the correct method.  On Nano you only have to see if it is enabled
> in CPUID and forget about

Since the Nano RNG can't be configured, the patch could be changed to
something like this:

--- linux-2.6.30-rc5/drivers/char/hw_random/via-rng.c.orig	2009-03-23 23:12:14.000000000 +0000
+++ linux-2.6.30-rc5/drivers/char/hw_random/via-rng.c	2009-05-11 10:02:29.000000000 +0000
@@ -132,6 +132,13 @@
 	struct cpuinfo_x86 *c = &cpu_data(0);
 	u32 lo, hi, old_lo;

+	/* VIA Nano CPUs don't have the MSR_VIA_RNG anymore.  The RNG
+	 * is always enabled if CPUID rng_en is set.  There is no
+	 * RNG configuration like it used to be the case in this
+	 * register */
+	if ((c->x86 == 6) && (c->x86_model >= 0x0f))
+		return 0;
+
 	/* Control the RNG via MSR.  Tread lightly and pay very close
 	 * close attention to values written, as the reserved fields
 	 * are documented to be "undefined and unpredictable"; but it
@@ -184,8 +191,17 @@
 {
 	int err;

-	if (!cpu_has_xstore)
+	if (!cpu_has_xstore) {
+		printk(KERN_NOTICE PFX "VIA RNG not detected.\n");
+		return -ENODEV;
+	}
+
+	if (!cpu_has_xstore_enabled) {
+		printk(KERN_NOTICE PFX "VIA RNG detected, but not enabled."
+		       " Hmm, strange...\n");
 		return -ENODEV;
+	}
+
 	printk(KERN_INFO "VIA RNG detected\n");
 	err = hwrng_register(&via_rng);
 	if (err) {

The check for cpu_has_xstore_enabled was moved to mod_init, because
it turns out that arch/x86/kernel/cpu/centaur.c already tries to
enable it in init_c3. This might break things if people have found
ways to disable it again after it got enabled in init_c3.
I've also changed mod_init to look more like
drivers/crypto/padlock-aes.c and drivers/crypto/padlock-sha.c

If it's possible for a Nano to have the cpuid rng flag set and the
rng_en flag unset, init_c3 in arch/x86/kernel/cpu/centaur.c may have
to be changed too. This seems very unlikely to me, because there would
be no way to enable the RNG then.


>> I don't know the proper way to initialize it because of lacking
>> documentation. 
> 
> the quick reference for padlock on the nano is public, despite not
> at a very well-known location:
> ftp://ftp.vtbridge.org/Docs/CPU/Nano/padlock_quick_reference_V095.pdf

Thanks for the link. Together with version 3.1 of the PadLock SDK
from the viaarena website, that's enough information about PadLock.
Unfortunately I've not been able to find documentation about other
features like the MSRs and Powersaver.

Kind regards,

Arjan Koers

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Arjan Koers]

Harald Welte wrote:

> I have sent your modifications (split in two patches, with some additional
> comment/explanation) to this list.

Thanks. I've applied your patches and it works for me.

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Harald Welte]

Hi Arjan,

On Mon, May 11, 2009 at 01:22:01PM +0200, Arjan Koers wrote:
> Harald Welte wrote:
> >> How can multiple RNGs in current dual-processor setups and in the future
> >> multicore Nano be handled?
> >  
> > That's actually a good question.  I'll probably forward that to the CPU
> > division and see what they come up with.  I would assume you just run the
> > initialization on every CPU, and that's it.
> 
> It would probably have to be setup the same on all CPUs (enabled on all /
> disabled on all), because the code might execute on any CPU.

yes, but I don't really consider this a practical problem.  I think there
is no practical case where half of the CPUs want a hardware RNG, and half
of them don't.  If somebody really has that kind of special need, he can
probably patch his kernel himself.

> > Now the more interesting question is: how can the hw_random framework deal
> > with it?  As far as I remember, it can only select one out of N available
> > concurrent providers of hardware randomness.  correct?
> 
> I don't know, but it would be nice if it could use PadLock functionality on
> multiple CPUs at the same time for increased speed.

I'm not so sure if that really makes sense just to increase throughput of
a single workload / work item.  However, for multiple concurrent threads,
of course each thread on each CPU should be able to use the padlock
instructions for AES / PHE / RNG.  And if you enable the feature on all CPU's
during the respective module_init(), it should simply work afterwards.

> > This is due to the fact that the MSR itself does no longer exist on the Nano.
> > It only exists on C3 to C7 silicon.
> 
> Thanks for the information. Do you happen to know what changed for the other
> MSRs that are used for ACE, PowerSaver, ...?

I unfortunately don't [yet] but I'll inquire.

> Since the Nano RNG can't be configured, the patch could be changed to
> something like this:

yes, thanks. looks fine to me.

> If it's possible for a Nano to have the cpuid rng flag set and the
> rng_en flag unset, init_c3 in arch/x86/kernel/cpu/centaur.c may have
> to be changed too. This seems very unlikely to me, because there would
> be no way to enable the RNG then.

I'll re-confirm this with the CPU R&D, too.

Regards,
-- 
- Harald Welte <HaraldWelte@viatech.com>	    http://linux.via.com.tw/
============================================================================
VIA Open Source Liaison

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Herbert Xu]

On Mon, May 11, 2009 at 12:06:51PM +0800, Harald Welte wrote:
>
> Herbert: Do you accept hw_random related patches, or should they go some
> different route?

Yeah I'm taking hw_random patches.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] The VIA Hardware RNG driver is for the CPU, not Chipset

[Herbert Xu]

On Sun, May 10, 2009 at 02:28:44PM +0800, Harald Welte wrote:
> This is a cosmetic change, fixing the MODULE_DESCRIPTION() of via-rng.c
> 
> Signed-off-by: Harald Welte <HaraldWelte@viatech.com>

Patch applied.  Thanks!
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt