Skip to contentSkip to conversation

Handling PCI/ROM space

PATCH Jeremy Fitzhardinge 2 participants 3 messages Oct 18, 2006 virtio core

lore
Share & tools
lore
#0 lore raw

I'm seeing oopses in probe_roms() and pci_find_bios(), apparently because those pages are not mapped under Xen. I'm not sure why I'm seeing this now and not before, but I suspect its because I enabled CONFIG_DEBUG_PAGEALLOC. Anyway, I've got these patches to deal with these cases:

arch/i386/kernel/setup.c +8 -1
@@ -276,7 +276,14 @@ static struct resource standard_io_resou
 #define STANDARD_IO_RESOURCES \
 	(sizeof standard_io_resources / sizeof standard_io_resources[0])
 
-#define romsignature(x) (*(unsigned short *)(x) == 0xaa55)
+static inline int romsignature(const unsigned char *x)
+{
+     unsigned short sig;
+     int ret = 0;
+     if (__get_user(sig, (const unsigned short *)x) == 0)
+	  ret = (sig == 0xaa55);
+     return ret;
+}
 
 static int __init romchecksum(unsigned char *rom, unsigned long length)
 {
arch/i386/pci/pcbios.c +6 -1
@@ -5,6 +5,7 @@
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/module.h>
+#include <asm/uaccess.h>
 #include "pci.h"
 #include "pci-functions.h"
 
@@ -301,7 +302,7 @@ static struct pci_raw_ops pci_bios_acces
 
 static struct pci_raw_ops * __devinit pci_find_bios(void)
 {
-	union bios32 *check;
+	union bios32 *check, sig;
 	unsigned char sum;
 	int i, length;
 
@@ -314,6 +315,10 @@ static struct pci_raw_ops * __devinit pc
 	for (check = (union bios32 *) __va(0xe0000);
 	     check <= (union bios32 *) __va(0xffff0);
 	     ++check) {
+		long sig;
+		if (__get_user(sig, &check->fields.signature))
+			continue;
+
 		if (check->fields.signature != BIOS32_SIGNATURE)
 			continue;
 		length = check->fields.length * 16;

Does this seem reasonable, or should there be some other fix?

J

#1 lore raw
Quoted 5 lines · Jeremy Fitzhardinge

I'm seeing oopses in probe_roms() and pci_find_bios(), apparently because those pages are not mapped under Xen. I'm not sure why I'm seeing this now and not before, but I suspect its because I enabled CONFIG_DEBUG_PAGEALLOC. Anyway, I've got these patches to deal with these cases:

Probably a config file difference - GOBIOS vs GODIRECT.

These patches look fine, although is there a more general solution? Like bypassing the whole PCI ROM probing entirely? We don't really want to use the PCI ROM here either, although we have a slightly worse problem - the pages are mapped and do have a PCI ROM. Perhaps paravirt-ops should be able to flip a global switch to disable this (perhaps already there?).

Zach

#2 lore raw
Quoted 8 lines · Zachary Amsden

Probably a config file difference - GOBIOS vs GODIRECT.

These patches look fine, although is there a more general solution? Like bypassing the whole PCI ROM probing entirely? We don't really want to use the PCI ROM here either, although we have a slightly worse problem - the pages are mapped and do have a PCI ROM. Perhaps paravirt-ops should be able to flip a global switch to disable this (perhaps already there?).

There's a pci_probe variable, which has a bitmask of which PCI access methods to use; if you mask out PCI_PROBE_BIOS then it won't bother. I considered doing this, but it wasn't clear to me where this actually get set up (it's a bit diffuse), and whether to add a new pv_op hook intercept this.

I also thought that we don't necessarily want to unconditionally disable PCI access for paravirt guests since even an unprivileged guest may have a raw device exported to it, and we don't want to add too many special cases when dom0 is migrated to operate in the pv_op world. So it seemed more correct to actually allow the probe but deal with unmapped pci ROM space.

J