On Mon, 2007-03-05 at 13:06 +0100, Ingo Molnar wrote:
> Subject: [patch] paravirt: VDSO page is essential
> From: Ingo Molnar <[EMAIL PROTECTED]>
> 
> commit 3bbf54725467d604698721384d858b5983b87e8f disables the VDSO for 
> CONFIG_PARAVIRT kernels. This #ifdeffery was a bad change: the VDSO is 
> an essential component of Linux, and this change forces all of them to 
> use int $0x80 - including sane ones like KVM. (If a hypervisor does not 
> handle the VDSO properly then it can work things around via the vdso=0 
> boot option. Or CONFIG_PARAVIRT should not have been merged. But in any 
> case, it is a basic taste issue: we DO NOT #ifdef around core features 
> like this!)

I agree with the criticism, dislike the snarly comments, and disagree
with this patch.

VDSO is only a problem if (1) the hypervisor wants to reserve the top
virtual address space (CONFIG_PARAVIRT=y), and (2) the glibc is old and
can't handle a VDSO mapped anywhere but 0xFFFFE000
(CONFIG_COMPAT_VDSO=y).

Now, KVM wants to use CONFIG_PARAVIRT=y but not reserve_top_address(),
so we should split the config option.  Let's not get too excited because
we kept it simple.  Patch (untested, but fairly simple) below.

BTW, I had a patch to do a runtime test (old glibc causes init to
assert, then disable vdso and try again): everyone hated it.

Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>

diff -r f75715e64a3b arch/i386/Kconfig
--- a/arch/i386/Kconfig Tue Mar 06 00:04:50 2007 +1100
+++ b/arch/i386/Kconfig Tue Mar 06 00:20:44 2007 +1100
@@ -218,9 +218,18 @@ config PARAVIRT
          However, when run without a hypervisor the kernel is
          theoretically slower.  If in doubt, say N.
 
+config RESERVE_TOP
+       bool
+       help
+         Many hypervisors want to reserve some amount of the top of
+         virtual address space.  Unfortunately, old glibc needs the
+         vdso page there, so we must disable vdso if COMPAT_VDSO is
+         enabled as well as this option.
+
 config VMI
        bool "VMI Paravirt-ops support"
        depends on PARAVIRT && !NO_HZ
+       select RESERVE_TOP
        default y
        help
          VMI provides a paravirtualized interface to multiple hypervisors
@@ -893,9 +902,10 @@ config COMPAT_VDSO
 config COMPAT_VDSO
        bool "Compat VDSO support"
        default y
-       depends on !PARAVIRT
-       help
-         Map the VDSO to the predictable old-style address too.
+       help
+         Map the VDSO to the predictable old-style address too, or
+         in the case of a VMI/Xen/lguest virtualized guest, don't create
+         the VDSO at all.
        ---help---
          Say N here if you are running a sufficiently recent glibc
          version (2.3.3 or later), to remove the high-mapped
diff -r f75715e64a3b arch/i386/kernel/sysenter.c
--- a/arch/i386/kernel/sysenter.c       Tue Mar 06 00:04:50 2007 +1100
+++ b/arch/i386/kernel/sysenter.c       Tue Mar 06 00:21:42 2007 +1100
@@ -27,7 +27,7 @@
  * Should the kernel map a VDSO page into processes and pass its
  * address down to glibc upon exec()?
  */
-#ifdef CONFIG_PARAVIRT
+#if defined(CONFIG_COMPAT_VDSO) && defined(CONFIG_RESERVE_TOP)
 unsigned int __read_mostly vdso_enabled = 0;
 #else
 unsigned int __read_mostly vdso_enabled = 1;
diff -r f75715e64a3b arch/i386/mm/pgtable.c
--- a/arch/i386/mm/pgtable.c    Tue Mar 06 00:04:50 2007 +1100
+++ b/arch/i386/mm/pgtable.c    Tue Mar 06 00:06:00 2007 +1100
@@ -173,7 +173,7 @@ void reserve_top_address(unsigned long r
        BUG_ON(fixmaps > 0);
        printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
               (int)-reserve);
-#ifdef CONFIG_COMPAT_VDSO
+#ifndef CONFIG_RESERVE_TOP
        BUG_ON(reserve != 0);
 #else
        __FIXADDR_TOP = -reserve - PAGE_SIZE;


-
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/

Reply via email to