2006-11-01 05:50:00

by Chris Wright

[permalink] [raw]
Subject: [PATCH 46/61] fix Intel RNG detection

-stable review patch. If anyone has any objections, please let us know.
------------------

From: Jan Beulich <[email protected]>

[PATCH] fix Intel RNG detection

Previously, since determination whether there was an Intel random number
generator was based on a single bit, on systems with a matching bridge
device but without a firmware hub, there was a 50% chance that the code
would incorrectly decide that the system had an RNG. This patch adds
detection of the firmware hub to better qualify the existence of an RNG.

There is one issue with the patch: I was unable to determine the LPC
equivalent for the PCI bridge 8086:2430 (since the old code didn't care
about which of the many devices provided by the ICH/ESB it was chose to use
the PCI bridge device, but the FWH settings live in the LPC device, so the
device list needed to be changed).

Signed-off-by: Jan Beulich <[email protected]>
Signed-off-by: Michael Buesch <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Chris Wright <[email protected]>

---
drivers/char/hw_random/intel-rng.c | 186 +++++++++++++++++++++++++++++++++++--
1 file changed, 177 insertions(+), 9 deletions(-)

--- linux-2.6.18.1.orig/drivers/char/hw_random/intel-rng.c
+++ linux-2.6.18.1/drivers/char/hw_random/intel-rng.c
@@ -50,6 +50,43 @@
#define INTEL_RNG_ADDR_LEN 3

/*
+ * LPC bridge PCI config space registers
+ */
+#define FWH_DEC_EN1_REG_OLD 0xe3
+#define FWH_DEC_EN1_REG_NEW 0xd9 /* high byte of 16-bit register */
+#define FWH_F8_EN_MASK 0x80
+
+#define BIOS_CNTL_REG_OLD 0x4e
+#define BIOS_CNTL_REG_NEW 0xdc
+#define BIOS_CNTL_WRITE_ENABLE_MASK 0x01
+#define BIOS_CNTL_LOCK_ENABLE_MASK 0x02
+
+/*
+ * Magic address at which Intel Firmware Hubs get accessed
+ */
+#define INTEL_FWH_ADDR 0xffff0000
+#define INTEL_FWH_ADDR_LEN 2
+
+/*
+ * Intel Firmware Hub command codes (write to any address inside the device)
+ */
+#define INTEL_FWH_RESET_CMD 0xff /* aka READ_ARRAY */
+#define INTEL_FWH_READ_ID_CMD 0x90
+
+/*
+ * Intel Firmware Hub Read ID command result addresses
+ */
+#define INTEL_FWH_MANUFACTURER_CODE_ADDRESS 0x000000
+#define INTEL_FWH_DEVICE_CODE_ADDRESS 0x000001
+
+/*
+ * Intel Firmware Hub Read ID command result values
+ */
+#define INTEL_FWH_MANUFACTURER_CODE 0x89
+#define INTEL_FWH_DEVICE_CODE_8M 0xac
+#define INTEL_FWH_DEVICE_CODE_4M 0xad
+
+/*
* Data for PCI driver interface
*
* This data only exists for exporting the supported
@@ -58,12 +95,50 @@
* want to register another driver on the same PCI id.
*/
static const struct pci_device_id pci_tbl[] = {
- { 0x8086, 0x2418, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, },
- { 0x8086, 0x2428, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, },
- { 0x8086, 0x2430, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, },
- { 0x8086, 0x2448, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, },
- { 0x8086, 0x244e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, },
- { 0x8086, 0x245e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, },
+/* AA
+ { 0x8086, 0x2418, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+ { 0x8086, 0x2410, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* AA */
+/* AB
+ { 0x8086, 0x2428, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+ { 0x8086, 0x2420, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* AB */
+/* ??
+ { 0x8086, 0x2430, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+/* BAM, CAM, DBM, FBM, GxM
+ { 0x8086, 0x2448, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+ { 0x8086, 0x244c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* BAM */
+ { 0x8086, 0x248c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* CAM */
+ { 0x8086, 0x24cc, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* DBM */
+ { 0x8086, 0x2641, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* FBM */
+ { 0x8086, 0x27b9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* GxM */
+ { 0x8086, 0x27bd, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* GxM DH */
+/* BA, CA, DB, Ex, 6300, Fx, 631x/632x, Gx
+ { 0x8086, 0x244e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+ { 0x8086, 0x2440, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* BA */
+ { 0x8086, 0x2480, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* CA */
+ { 0x8086, 0x24c0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* DB */
+ { 0x8086, 0x24d0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Ex */
+ { 0x8086, 0x25a1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 6300 */
+ { 0x8086, 0x2640, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Fx */
+ { 0x8086, 0x2670, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2671, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2672, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2673, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2674, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2675, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2676, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2677, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2678, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x2679, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x267a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x267b, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x267c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x267d, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x267e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x267f, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
+ { 0x8086, 0x27b8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Gx */
+/* E
+ { 0x8086, 0x245e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+ { 0x8086, 0x2450, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* E */
{ 0, }, /* terminate list */
};
MODULE_DEVICE_TABLE(pci, pci_tbl);
@@ -138,22 +213,115 @@ static struct hwrng intel_rng = {
};


+#ifdef CONFIG_SMP
+static char __initdata waitflag;
+
+static void __init intel_init_wait(void *unused)
+{
+ while (waitflag)
+ cpu_relax();
+}
+#endif
+
static int __init mod_init(void)
{
int err = -ENODEV;
+ unsigned i;
+ struct pci_dev *dev = NULL;
void __iomem *mem;
- u8 hw_status;
+ unsigned long flags;
+ u8 bios_cntl_off, fwh_dec_en1_off;
+ u8 bios_cntl_val = 0xff, fwh_dec_en1_val = 0xff;
+ u8 hw_status, mfc, dvc;
+
+ for (i = 0; !dev && pci_tbl[i].vendor; ++i)
+ dev = pci_get_device(pci_tbl[i].vendor, pci_tbl[i].device, NULL);

- if (!pci_dev_present(pci_tbl))
+ if (!dev)
goto out; /* Device not found. */

+ /* Check for Intel 82802 */
+ if (dev->device < 0x2640) {
+ fwh_dec_en1_off = FWH_DEC_EN1_REG_OLD;
+ bios_cntl_off = BIOS_CNTL_REG_OLD;
+ } else {
+ fwh_dec_en1_off = FWH_DEC_EN1_REG_NEW;
+ bios_cntl_off = BIOS_CNTL_REG_NEW;
+ }
+
+ pci_read_config_byte(dev, fwh_dec_en1_off, &fwh_dec_en1_val);
+ pci_read_config_byte(dev, bios_cntl_off, &bios_cntl_val);
+
+ mem = ioremap_nocache(INTEL_FWH_ADDR, INTEL_FWH_ADDR_LEN);
+ if (mem == NULL) {
+ pci_dev_put(dev);
+ err = -EBUSY;
+ goto out;
+ }
+
+ /*
+ * Since the BIOS code/data is going to disappear from its normal
+ * location with the Read ID command, all activity on the system
+ * must be stopped until the state is back to normal.
+ */
+#ifdef CONFIG_SMP
+ set_mb(waitflag, 1);
+ if (smp_call_function(intel_init_wait, NULL, 1, 0) != 0) {
+ set_mb(waitflag, 0);
+ pci_dev_put(dev);
+ printk(KERN_ERR PFX "cannot run on all processors\n");
+ err = -EAGAIN;
+ goto err_unmap;
+ }
+#endif
+ local_irq_save(flags);
+
+ if (!(fwh_dec_en1_val & FWH_F8_EN_MASK))
+ pci_write_config_byte(dev,
+ fwh_dec_en1_off,
+ fwh_dec_en1_val | FWH_F8_EN_MASK);
+ if (!(bios_cntl_val &
+ (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK)))
+ pci_write_config_byte(dev,
+ bios_cntl_off,
+ bios_cntl_val | BIOS_CNTL_WRITE_ENABLE_MASK);
+
+ writeb(INTEL_FWH_RESET_CMD, mem);
+ writeb(INTEL_FWH_READ_ID_CMD, mem);
+ mfc = readb(mem + INTEL_FWH_MANUFACTURER_CODE_ADDRESS);
+ dvc = readb(mem + INTEL_FWH_DEVICE_CODE_ADDRESS);
+ writeb(INTEL_FWH_RESET_CMD, mem);
+
+ if (!(bios_cntl_val &
+ (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK)))
+ pci_write_config_byte(dev, bios_cntl_off, bios_cntl_val);
+ if (!(fwh_dec_en1_val & FWH_F8_EN_MASK))
+ pci_write_config_byte(dev, fwh_dec_en1_off, fwh_dec_en1_val);
+
+ local_irq_restore(flags);
+#ifdef CONFIG_SMP
+ /* Tell other CPUs to resume. */
+ set_mb(waitflag, 0);
+#endif
+
+ iounmap(mem);
+ pci_dev_put(dev);
+
+ if (mfc != INTEL_FWH_MANUFACTURER_CODE ||
+ (dvc != INTEL_FWH_DEVICE_CODE_8M &&
+ dvc != INTEL_FWH_DEVICE_CODE_4M)) {
+ printk(KERN_ERR PFX "FWH not detected\n");
+ err = -ENODEV;
+ goto out;
+ }
+
err = -ENOMEM;
mem = ioremap(INTEL_RNG_ADDR, INTEL_RNG_ADDR_LEN);
if (!mem)
goto out;
intel_rng.priv = (unsigned long)mem;

- /* Check for Intel 82802 */
+ /* Check for Random Number Generator */
err = -ENODEV;
hw_status = hwstatus_get(mem);
if ((hw_status & INTEL_RNG_PRESENT) == 0)

--


2006-11-20 23:48:12

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH 46/61] fix Intel RNG detection

On Tue, Oct 31, 2006 at 09:34:26PM -0800, Chris Wright wrote:

> From: Jan Beulich <[email protected]>
>
> [PATCH] fix Intel RNG detection
>
> Previously, since determination whether there was an Intel random number
> generator was based on a single bit, on systems with a matching bridge
> device but without a firmware hub, there was a 50% chance that the code
> would incorrectly decide that the system had an RNG. This patch adds
> detection of the firmware hub to better qualify the existence of an RNG.
>
> There is one issue with the patch: I was unable to determine the LPC
> equivalent for the PCI bridge 8086:2430 (since the old code didn't care
> about which of the many devices provided by the ICH/ESB it was chose to use
> the PCI bridge device, but the FWH settings live in the LPC device, so the
> device list needed to be changed).
>
> Signed-off-by: Jan Beulich <[email protected]>
> Signed-off-by: Michael Buesch <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>


Since I pushed an update to our Fedora users based on 2.6.18.2, a few people
have reported they no longer have their RNG's detected.
Here's one report: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215144

Dave

--
http://www.codemonkey.org.uk

2006-11-21 02:21:41

by Chris Wright

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

* Dave Jones ([email protected]) wrote:
> Since I pushed an update to our Fedora users based on 2.6.18.2, a few people
> have reported they no longer have their RNG's detected.
> Here's one report: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215144

Hmm, I wonder if the report is valid? Jan's patch would have the correct
side effect of disabling false positives (for RNG identification).
Be good to check that it actually used to work.

Having said that, Jan the datasheet recommendation is looser than your
implementation. It only recommends checking for manufacturer code,
you check device code as well. Do you know of any scenarios where that
would matter (I can't conceive of any)?

thanks,
-chris

2006-11-21 09:07:38

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 46/61] fix Intel RNG detection

On Tuesday 21 November 2006 00:45, Dave Jones wrote:
> On Tue, Oct 31, 2006 at 09:34:26PM -0800, Chris Wright wrote:
>
> > From: Jan Beulich <[email protected]>
> >
> > [PATCH] fix Intel RNG detection
> >
> > Previously, since determination whether there was an Intel random number
> > generator was based on a single bit, on systems with a matching bridge
> > device but without a firmware hub, there was a 50% chance that the code
> > would incorrectly decide that the system had an RNG. This patch adds
> > detection of the firmware hub to better qualify the existence of an RNG.
> >
> > There is one issue with the patch: I was unable to determine the LPC
> > equivalent for the PCI bridge 8086:2430 (since the old code didn't care
> > about which of the many devices provided by the ICH/ESB it was chose to use
> > the PCI bridge device, but the FWH settings live in the LPC device, so the
> > device list needed to be changed).
> >
> > Signed-off-by: Jan Beulich <[email protected]>
> > Signed-off-by: Michael Buesch <[email protected]>
> > Signed-off-by: Andrew Morton <[email protected]>
> > Signed-off-by: Linus Torvalds <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > Signed-off-by: Chris Wright <[email protected]>
>
>
> Since I pushed an update to our Fedora users based on 2.6.18.2, a few people
> have reported they no longer have their RNG's detected.
> Here's one report: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215144

Well, this patch should acutally fix false detections.
Did these people actually have a _working_ rng before?
I saw a report of a mac user, for which the rng disappeared. But
the previously "detected" rng didn't work either. (Work as in produces
sane random numbers). So there actually wasn't a rng available all the time.

--
Greetings Michael.

2006-11-21 09:31:25

by Jan Beulich

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

>>> Chris Wright <[email protected]> 21.11.06 03:21 >>>
>* Dave Jones ([email protected]) wrote:
>> Since I pushed an update to our Fedora users based on 2.6.18.2, a few people
>> have reported they no longer have their RNG's detected.
>> Here's one report: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215144
>
>Hmm, I wonder if the report is valid? Jan's patch would have the correct
>side effect of disabling false positives (for RNG identification).
>Be good to check that it actually used to work.

Indeed, that is quite significant to know here.

>Having said that, Jan the datasheet recommendation is looser than your
>implementation. It only recommends checking for manufacturer code,
>you check device code as well. Do you know of any scenarios where that
>would matter (I can't conceive of any)?

Since Intel doesn't list any other device codes, I suppose there are none.
But of course, it's not entirely impossible that there are others, but I
wouldn't want to relax the already weak check; I'd rather want to add
other device codes if we have proof that these are valid.

Jan

2006-11-21 11:22:15

by Vassilios Kotoulas

[permalink] [raw]
Subject: Re: [PATCH 46/61] fix Intel RNG detection

>> Dave Jones (davej@xxxxxxxxxx) wrote:
>> Since I pushed an update to our Fedora users based on 2.6.18.2, a few people
>> have reported they no longer have their RNG's detected.
>> Here's one report: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=215144

> Hmm, I wonder if the report is valid? Jan's patch would have the correct
> side effect of disabling false positives (for RNG identification).
> Be good to check that it actually used to work.
>
> Having said that, Jan the datasheet recommendation is looser than your
> implementation. It only recommends checking for manufacturer code,
> you check device code as well. Do you know of any scenarios where that
> would matter (I can't conceive of any)?

Hi,

sorry for the broken threading, I read this in the archives.

My RNG worked with the older kernel 2.6.18-1.2200.fc5
I booted this kernel and made the following tests:

hexdump -v /dev/hw_random

here is the beginning of the output:

0000000 a7ff 1dff b312 0be0 64c9 8f83 9082 7d94
0000010 ac03 c513 ac1b b502 b93e 8f34 1a05 d417
0000020 b9cc c5d5 345f e6f6 d84b 333c c156 9007
0000030 0539 b145 7fdc 071b ea10 9145 c395 5536
0000040 7942 f8a2 f07d a5ea 2c76 a934 742c 1288
0000050 925a f710 67e6 8f68 ece2 bb23 536d bebe
0000060 1ad3 ab94 ab7b 54c9 8ce3 adaa 7f79 edd7
0000070 4c5b 0a28 66ee bc8d 90ae 0515 353e dcf5

These Numbers seem random to me :)

rngtest produces the following output:

[root@kra:~] rngtest -t1 </dev/hw_random
rngtest 2
Copyright (c) 2004 by Henrique de Moraes Holschuh
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

rngtest: starting FIPS tests...
rngtest: bits received from input: 60032
rngtest: FIPS 140-2 successes: 3
rngtest: FIPS 140-2 failures: 0
rngtest: FIPS 140-2(2001-10-10) Monobit: 0
rngtest: FIPS 140-2(2001-10-10) Poker: 0
rngtest: FIPS 140-2(2001-10-10) Runs: 0
rngtest: FIPS 140-2(2001-10-10) Long run: 0
rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
rngtest: input channel speed: (min=44.617; avg=48.397; max=51.079)Kibits/s
rngtest: FIPS tests speed: (min=62.129; avg=62.536; max=62.949)Mibits/s
rngtest: Program run time: 1211852 microseconds
rngtest: bits received from input: 120032
rngtest: FIPS 140-2 successes: 6
rngtest: FIPS 140-2 failures: 0
rngtest: FIPS 140-2(2001-10-10) Monobit: 0
rngtest: FIPS 140-2(2001-10-10) Poker: 0
rngtest: FIPS 140-2(2001-10-10) Runs: 0
rngtest: FIPS 140-2(2001-10-10) Long run: 0
rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
rngtest: input channel speed: (min=44.307; avg=46.440; max=51.079)Kibits/s
rngtest: FIPS tests speed: (min=59.980; avg=61.627; max=62.949)Mibits/s
rngtest: Program run time: 2525610 microseconds

I also updated the bugzilla report with these tests.

2006-11-22 01:51:06

by Chris Wright

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

* Jan Beulich ([email protected]) wrote:
> >>> Chris Wright <[email protected]> 21.11.06 03:21 >>>
> >Hmm, I wonder if the report is valid? Jan's patch would have the correct
> >side effect of disabling false positives (for RNG identification).
> >Be good to check that it actually used to work.
>
> Indeed, that is quite significant to know here.

It does appear to work w/out the patch. I've asked for a small bit
of diagnostics (below), perhaps you've got something you'd rather see?
I expect this to be a 24C0 LPC Bridge.


diff --git a/drivers/char/hw_random/intel-rng.c b/drivers/char/hw_random/intel-rng.c
index 8efbc9c..c655f57 100644
--- a/drivers/char/hw_random/intel-rng.c
+++ b/drivers/char/hw_random/intel-rng.c
@@ -304,6 +304,10 @@ #ifdef CONFIG_SMP
set_mb(waitflag, 0);
#endif

+ printk(PFX "pci vendor:device %hx:%hx fwh_dec_en1 %x bios_cntl_val %x "
+ "mfc %x dvc %x\n", dev->vendor, dev->device,
+ fwh_dec_en1_val, bios_cntl_val, mfc, dvc);
+
iounmap(mem);
pci_dev_put(dev);

2006-11-22 07:51:19

by Jan Beulich

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

>It does appear to work w/out the patch. I've asked for a small bit
>of diagnostics (below), perhaps you've got something you'd rather see?
>I expect this to be a 24C0 LPC Bridge.

Yes, that's what I'd have asked for. If it works, I expect the device
code to be different, or both manufacturer and device code to be
invalid. Depending on the outcome, perhaps we'll need an override
option so that this test can be partially (i.e. just the device code
part) or entirely (all the FWH detection) skipped.
The base problem is the vague documentation of the whole
detection mechanism - a lot of this I had to read between the lines.

Jan

2006-11-24 20:32:35

by Dave Jones

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

On Wed, Nov 22, 2006 at 08:53:08AM +0100, Jan Beulich wrote:
> >It does appear to work w/out the patch. I've asked for a small bit
> >of diagnostics (below), perhaps you've got something you'd rather see?
> >I expect this to be a 24C0 LPC Bridge.
>
> Yes, that's what I'd have asked for. If it works, I expect the device
> code to be different, or both manufacturer and device code to be
> invalid. Depending on the outcome, perhaps we'll need an override
> option so that this test can be partially (i.e. just the device code
> part) or entirely (all the FWH detection) skipped.
> The base problem is the vague documentation of the whole
> detection mechanism - a lot of this I had to read between the lines.

The bug report I referenced came back with this from that debug patch..

intel_rng: no version for "struct_module" found: kernel tainted.
intel_rng: pci vendor:device 8086:24c0 fwh_dec_en1 80 bios_cntl_val 2 mfc cb dvc 88
intel_rng: FWH not detected

Dave

--
http://www.codemonkey.org.uk

2006-11-27 08:29:23

by Jan Beulich

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

>>> Dave Jones <[email protected]> 24.11.06 21:27 >>>
>On Wed, Nov 22, 2006 at 08:53:08AM +0100, Jan Beulich wrote:
> > >It does appear to work w/out the patch. I've asked for a small bit
> > >of diagnostics (below), perhaps you've got something you'd rather see?
> > >I expect this to be a 24C0 LPC Bridge.
> >
> > Yes, that's what I'd have asked for. If it works, I expect the device
> > code to be different, or both manufacturer and device code to be
> > invalid. Depending on the outcome, perhaps we'll need an override
> > option so that this test can be partially (i.e. just the device code
> > part) or entirely (all the FWH detection) skipped.
> > The base problem is the vague documentation of the whole
> > detection mechanism - a lot of this I had to read between the lines.
>
>The bug report I referenced came back with this from that debug patch..
>
>intel_rng: no version for "struct_module" found: kernel tainted.
>intel_rng: pci vendor:device 8086:24c0 fwh_dec_en1 80 bios_cntl_val 2 mfc cb dvc 88
>intel_rng: FWH not detected

Okay, this means the lock is being set by the BIOS, which disallows
disabling BIOS (and thus accessing the FWH). By default, I think it
is correct to consider the RNG not present in this case, however as
previously indicated I think we should provide a way to force
skipping the FWH test (with three levels - carry out, skip always, or
skip if BIOS locked).
I'll prepare a patch as soon as I can, but it might take a few days
until I get to it.

Jan

2006-11-29 08:44:43

by Jan Beulich

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

>>> Dave Jones <[email protected]> 24.11.06 21:27 >>>
>On Wed, Nov 22, 2006 at 08:53:08AM +0100, Jan Beulich wrote:
> > >It does appear to work w/out the patch. I've asked for a small bit
> > >of diagnostics (below), perhaps you've got something you'd rather see?
> > >I expect this to be a 24C0 LPC Bridge.
> >
> > Yes, that's what I'd have asked for. If it works, I expect the device
> > code to be different, or both manufacturer and device code to be
> > invalid. Depending on the outcome, perhaps we'll need an override
> > option so that this test can be partially (i.e. just the device code
> > part) or entirely (all the FWH detection) skipped.
> > The base problem is the vague documentation of the whole
> > detection mechanism - a lot of this I had to read between the lines.
>
>The bug report I referenced came back with this from that debug patch..
>
>intel_rng: no version for "struct_module" found: kernel tainted.
>intel_rng: pci vendor:device 8086:24c0 fwh_dec_en1 80 bios_cntl_val 2 mfc cb dvc 88
>intel_rng: FWH not detected

Any chance you could have them test below patch (perhaps before I
actually submit it)? They should see the warning message added when
not using any options, and they should then be able to use the
no_fwh_detect option to get the thing to work again.

I'll meanwhile ask Intel about how they suppose to follow the RNG
detection sequence when the BIOS locks out write access to the
FWH interface.

Jan

Index: head-2006-11-21/drivers/char/hw_random/intel-rng.c
===================================================================
--- head-2006-11-21.orig/drivers/char/hw_random/intel-rng.c 2006-11-21 10:36:15.000000000 +0100
+++ head-2006-11-21/drivers/char/hw_random/intel-rng.c 2006-11-29 09:09:21.000000000 +0100
@@ -143,6 +143,8 @@ static const struct pci_device_id pci_tb
};
MODULE_DEVICE_TABLE(pci, pci_tbl);

+static __initdata int no_fwh_detect;
+module_param(no_fwh_detect, int, 0);

static inline u8 hwstatus_get(void __iomem *mem)
{
@@ -240,6 +242,11 @@ static int __init mod_init(void)
if (!dev)
goto out; /* Device not found. */

+ if (no_fwh_detect < 0) {
+ pci_dev_put(dev);
+ goto fwh_done;
+ }
+
/* Check for Intel 82802 */
if (dev->device < 0x2640) {
fwh_dec_en1_off = FWH_DEC_EN1_REG_OLD;
@@ -252,6 +259,23 @@ static int __init mod_init(void)
pci_read_config_byte(dev, fwh_dec_en1_off, &fwh_dec_en1_val);
pci_read_config_byte(dev, bios_cntl_off, &bios_cntl_val);

+ if ((bios_cntl_val &
+ (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK))
+ == BIOS_CNTL_LOCK_ENABLE_MASK) {
+ static __initdata /*const*/ char warning[] =
+ KERN_WARNING PFX "Firmware space is locked read-only. If you can't or\n"
+ KERN_WARNING PFX "don't want to disable this in firmware setup, and if\n"
+ KERN_WARNING PFX "you are certain that your system has a functional\n"
+ KERN_WARNING PFX "RNG, try using the 'no_fwh_detect' option.\n";
+
+ pci_dev_put(dev);
+ if (no_fwh_detect)
+ goto fwh_done;
+ printk(warning);
+ err = -EBUSY;
+ goto out;
+ }
+
mem = ioremap_nocache(INTEL_FWH_ADDR, INTEL_FWH_ADDR_LEN);
if (mem == NULL) {
pci_dev_put(dev);
@@ -280,8 +304,7 @@ static int __init mod_init(void)
pci_write_config_byte(dev,
fwh_dec_en1_off,
fwh_dec_en1_val | FWH_F8_EN_MASK);
- if (!(bios_cntl_val &
- (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK)))
+ if (!(bios_cntl_val & BIOS_CNTL_WRITE_ENABLE_MASK))
pci_write_config_byte(dev,
bios_cntl_off,
bios_cntl_val | BIOS_CNTL_WRITE_ENABLE_MASK);
@@ -315,6 +338,8 @@ static int __init mod_init(void)
goto out;
}

+fwh_done:
+
err = -ENOMEM;
mem = ioremap(INTEL_RNG_ADDR, INTEL_RNG_ADDR_LEN);
if (!mem)

2006-12-13 19:58:15

by dean gaudet

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

On Wed, 29 Nov 2006, Jan Beulich wrote:

> >>> Dave Jones <[email protected]> 24.11.06 21:27 >>>
> >On Wed, Nov 22, 2006 at 08:53:08AM +0100, Jan Beulich wrote:
> > > >It does appear to work w/out the patch. I've asked for a small bit
> > > >of diagnostics (below), perhaps you've got something you'd rather see?
> > > >I expect this to be a 24C0 LPC Bridge.
> > >
> > > Yes, that's what I'd have asked for. If it works, I expect the device
> > > code to be different, or both manufacturer and device code to be
> > > invalid. Depending on the outcome, perhaps we'll need an override
> > > option so that this test can be partially (i.e. just the device code
> > > part) or entirely (all the FWH detection) skipped.
> > > The base problem is the vague documentation of the whole
> > > detection mechanism - a lot of this I had to read between the lines.
> >
> >The bug report I referenced came back with this from that debug patch..
> >
> >intel_rng: no version for "struct_module" found: kernel tainted.
> >intel_rng: pci vendor:device 8086:24c0 fwh_dec_en1 80 bios_cntl_val 2 mfc cb dvc 88
> >intel_rng: FWH not detected
>
> Any chance you could have them test below patch (perhaps before I
> actually submit it)? They should see the warning message added when
> not using any options, and they should then be able to use the
> no_fwh_detect option to get the thing to work again.
>
> I'll meanwhile ask Intel about how they suppose to follow the RNG
> detection sequence when the BIOS locks out write access to the
> FWH interface.

just for the public record (i already communicated with Jan in private
mail on this one)... i have a box which hangs hard starting at 2.6.18.2
and 2.6.19 -- hangs hard during the intel hw rng tests (no sysrq
response). and the hang occurs prior to the printk so it took some
digging to figure out which module was taking out the system.

Jan's patch gets the box past the hang... it seems like this should be in
at least the next 2.6.19.x stable (and if there's going to be another
2.6.18.x stable then it should be included there as well).

there is apparently no hw rng on this box (returns all 0xff).

thanks
-dean

>
> Jan
>
> Index: head-2006-11-21/drivers/char/hw_random/intel-rng.c
> ===================================================================
> --- head-2006-11-21.orig/drivers/char/hw_random/intel-rng.c 2006-11-21 10:36:15.000000000 +0100
> +++ head-2006-11-21/drivers/char/hw_random/intel-rng.c 2006-11-29 09:09:21.000000000 +0100
> @@ -143,6 +143,8 @@ static const struct pci_device_id pci_tb
> };
> MODULE_DEVICE_TABLE(pci, pci_tbl);
>
> +static __initdata int no_fwh_detect;
> +module_param(no_fwh_detect, int, 0);
>
> static inline u8 hwstatus_get(void __iomem *mem)
> {
> @@ -240,6 +242,11 @@ static int __init mod_init(void)
> if (!dev)
> goto out; /* Device not found. */
>
> + if (no_fwh_detect < 0) {
> + pci_dev_put(dev);
> + goto fwh_done;
> + }
> +
> /* Check for Intel 82802 */
> if (dev->device < 0x2640) {
> fwh_dec_en1_off = FWH_DEC_EN1_REG_OLD;
> @@ -252,6 +259,23 @@ static int __init mod_init(void)
> pci_read_config_byte(dev, fwh_dec_en1_off, &fwh_dec_en1_val);
> pci_read_config_byte(dev, bios_cntl_off, &bios_cntl_val);
>
> + if ((bios_cntl_val &
> + (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK))
> + == BIOS_CNTL_LOCK_ENABLE_MASK) {
> + static __initdata /*const*/ char warning[] =
> + KERN_WARNING PFX "Firmware space is locked read-only. If you can't or\n"
> + KERN_WARNING PFX "don't want to disable this in firmware setup, and if\n"
> + KERN_WARNING PFX "you are certain that your system has a functional\n"
> + KERN_WARNING PFX "RNG, try using the 'no_fwh_detect' option.\n";
> +
> + pci_dev_put(dev);
> + if (no_fwh_detect)
> + goto fwh_done;
> + printk(warning);
> + err = -EBUSY;
> + goto out;
> + }
> +
> mem = ioremap_nocache(INTEL_FWH_ADDR, INTEL_FWH_ADDR_LEN);
> if (mem == NULL) {
> pci_dev_put(dev);
> @@ -280,8 +304,7 @@ static int __init mod_init(void)
> pci_write_config_byte(dev,
> fwh_dec_en1_off,
> fwh_dec_en1_val | FWH_F8_EN_MASK);
> - if (!(bios_cntl_val &
> - (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK)))
> + if (!(bios_cntl_val & BIOS_CNTL_WRITE_ENABLE_MASK))
> pci_write_config_byte(dev,
> bios_cntl_off,
> bios_cntl_val | BIOS_CNTL_WRITE_ENABLE_MASK);
> @@ -315,6 +338,8 @@ static int __init mod_init(void)
> goto out;
> }
>
> +fwh_done:
> +
> err = -ENOMEM;
> mem = ioremap(INTEL_RNG_ADDR, INTEL_RNG_ADDR_LEN);
> if (!mem)
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2006-12-13 21:24:07

by Chris Wright

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

* dean gaudet ([email protected]) wrote:
> just for the public record (i already communicated with Jan in private
> mail on this one)... i have a box which hangs hard starting at 2.6.18.2
> and 2.6.19 -- hangs hard during the intel hw rng tests (no sysrq
> response). and the hang occurs prior to the printk so it took some
> digging to figure out which module was taking out the system.
>
> Jan's patch gets the box past the hang... it seems like this should be in
> at least the next 2.6.19.x stable (and if there's going to be another
> 2.6.18.x stable then it should be included there as well).

Thanks for the data point. I wonder if you get SMI and never come back.
Do you boot with no_fwh_detect=1 or -1?

thanks,
-chris

2006-12-13 23:00:06

by dean gaudet

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

On Wed, 13 Dec 2006, Chris Wright wrote:

> * dean gaudet ([email protected]) wrote:
> > just for the public record (i already communicated with Jan in private
> > mail on this one)... i have a box which hangs hard starting at 2.6.18.2
> > and 2.6.19 -- hangs hard during the intel hw rng tests (no sysrq
> > response). and the hang occurs prior to the printk so it took some
> > digging to figure out which module was taking out the system.
> >
> > Jan's patch gets the box past the hang... it seems like this should be in
> > at least the next 2.6.19.x stable (and if there's going to be another
> > 2.6.18.x stable then it should be included there as well).
>
> Thanks for the data point. I wonder if you get SMI and never come back.
> Do you boot with no_fwh_detect=1 or -1?

with the patch it boots perfectly without any command-line args.

without the patch it crashes after the "4" and before the "5" in this
hacked up segment of the code:

if (!(fwh_dec_en1_val & FWH_F8_EN_MASK))
pci_write_config_byte(dev,
fwh_dec_en1_off,
fwh_dec_en1_val | FWH_F8_EN_MASK);
if (!(bios_cntl_val &
(BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK)))
pci_write_config_byte(dev,
bios_cntl_off,
bios_cntl_val | BIOS_CNTL_WRITE_ENABLE_MASK);

printk(KERN_INFO "intel-rng: 4\n");
writeb(INTEL_FWH_RESET_CMD, mem);
printk(KERN_INFO "intel-rng: 5\n");

-dean

2006-12-14 08:14:24

by Jan Beulich

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

>with the patch it boots perfectly without any command-line args.

Are you getting the 'Firmware space is locked read-only' message then?

Thanks, Jan

2006-12-14 08:40:20

by dean gaudet

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

On Thu, 14 Dec 2006, Jan Beulich wrote:

> >with the patch it boots perfectly without any command-line args.
>
> Are you getting the 'Firmware space is locked read-only' message then?

yep...

so let me ask a naive question... don't we want the firmware locked
read-only because that protects the bios from viruses? honestly i'm naive
in this area of pc hardware, but i'm kind of confused why we'd want
unlocked firmware just so we can detect a RNG.

-dean

2006-12-14 10:11:21

by Jan Beulich

[permalink] [raw]
Subject: Re: [stable] [PATCH 46/61] fix Intel RNG detection

>>> dean gaudet <[email protected]> 14.12.06 09:40 >>>
>On Thu, 14 Dec 2006, Jan Beulich wrote:
>
>> >with the patch it boots perfectly without any command-line args.
>>
>> Are you getting the 'Firmware space is locked read-only' message then?
>
>yep...
>
>so let me ask a naive question... don't we want the firmware locked
>read-only because that protects the bios from viruses? honestly i'm naive
>in this area of pc hardware, but i'm kind of confused why we'd want
>unlocked firmware just so we can detect a RNG.

Indeed, these are contradicting requirements. The RNG detection, as
outlined by Intel documentation, requires being able to write to firmware
hub space (which in turn is hidden behind BIOS space). But I agree that
this is not a good solution (and even without that, it is not good to
require temporarily making invisible the entire BIOS code/data in order
to detect a non-essential device like an RNG).

Jan