On Fri, 2006-10-27 at 21:50 -0700, Andrew Morton wrote: > On Fri, 27 Oct 2006 21:33:08 -0700 > Jeremy Fitzhardinge <jeremy at goop.org> wrote: > > > Andrew Morton wrote: > > > It'd be better to use include/linux/uaccess.h:probe_kernel_address() for > > > this operation. > > > > > Ah, yes, that was the precedent I was thinking of, > > We've done open-coded __get_user() in various places in the past. The difference with > probe_kernel_address() is that it doesn't get deadlocked on mmap_sem(). > > > but I guess it would > > be better to just use it directly. It's a relatively new interface, > > isn't it? > > Yeah. New enough that nobody's tried using it on non-x86 ;) It needs > to do set_fs(KERNEL_DS). And the function name is misleading: it really does get a value, not merely probe an address. And the arguments are reversed from __get_user, just to add fun. Andrew, please replace prep-for-paravirt-be-careful-about-touching-bios-warning-fix.patch Subject: Be careful about touching BIOS address space BIOS ROM areas may not be mapped into the guest address space, so be careful when touching those addresses to make sure they appear to be mapped. At Andrew's request, fix probe_kernel_address for non-x86. Signed-off-by: Jeremy Fitzhardinge <jeremy at xensource.com> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> (modified) diff -r 9a6c8ceba677 arch/i386/kernel/setup.c --- a/arch/i386/kernel/setup.c Mon Oct 30 11:34:30 2006 +1100 +++ b/arch/i386/kernel/setup.c Mon Oct 30 13:15:33 2006 +1100 @@ -47,6 +47,7 @@ #include <linux/crash_dump.h> #include <linux/dmi.h> #include <linux/pfn.h> +#include <linux/uaccess.h> #include <video/edid.h> @@ -270,7 +271,14 @@ static struct resource standard_io_resou .flags = IORESOURCE_BUSY | IORESOURCE_IO } }; -#define romsignature(x) (*(unsigned short *)(x) == 0xaa55) +static inline int romsignature(const unsigned char *x) +{ + unsigned short sig; + int ret = 0; + if (probe_kernel_address((const unsigned short *)x, sig) == 0) + ret = (sig == 0xaa55); + return ret; +} static int __init romchecksum(unsigned char *rom, unsigned long length) { diff -r 9a6c8ceba677 arch/i386/pci/pcbios.c --- a/arch/i386/pci/pcbios.c Mon Oct 30 11:34:30 2006 +1100 +++ b/arch/i386/pci/pcbios.c Mon Oct 30 13:15:02 2006 +1100 @@ -5,6 +5,7 @@ #include <linux/pci.h> #include <linux/init.h> #include <linux/module.h> +#include <linux/uaccess.h> #include "pci.h" #include "pci-functions.h" @@ -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 (probe_kernel_address(&check->fields.signature, sig)) + continue; + if (check->fields.signature != BIOS32_SIGNATURE) continue; length = check->fields.length * 16; diff -r 9a6c8ceba677 include/linux/uaccess.h --- a/include/linux/uaccess.h Mon Oct 30 11:34:30 2006 +1100 +++ b/include/linux/uaccess.h Mon Oct 30 13:10:39 2006 +1100 @@ -34,10 +34,13 @@ static inline unsigned long __copy_from_ #define probe_kernel_address(addr, retval) \ ({ \ long ret; \ + mm_segment_t old_fs = get_fs(); \ \ + set_fs(KERNEL_DS); \ inc_preempt_count(); \ ret = __get_user(retval, addr); \ dec_preempt_count(); \ + set_fs(old_fs); \ ret; \ })