Re: [patch] paravirt: VDSO page is essential

Zachary Amsden
2007-03-06T01:03:25Z

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

Rusty Russell wrote:
> On Tue, 2007-03-06 at 00:28 +1100, Rusty Russell wrote:
>
>> On Mon, 2007-03-05 at 13:06 +0100, Ingo Molnar wrote:
>>
>>> Subject: [patch] paravirt: VDSO page is essential
>>> From: Ingo Molnar <mingo@elte.hu>
>>>
>>> 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.
>>
>
> And my patch was pretty crack-induced too. Sorry.
>
> I shouldn't have been thinking about using CONFIG options at all: we
> should simply disable the vdso if CONFIG_COMPAT_VDSO=y when we
> *actually* reserve top memory.
>
> This still need some work (doing that now), but do people like the idea?
>
> The current "vdso_disabled" flag merely disabled the ELF note, so it
> needs to be made a little stronger, to not set up the vdso at all.
>

I had just sent this out for internal review...

[-- Attachment #2: compat-vdso-broken --]
[-- Type: text/plain, Size: 3617 bytes --]

COMPAT_VDSO is incompatible with PARAVIRT for most implementations, as they
must relocate the fixmap to make room for a hypervisor. So allow COMPAT_VDSO
kernels to relocate the fixmap as well, just disable the VDSO if they do so.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff -r fad0910252d2 arch/i386/kernel/sysenter.c
--- a/arch/i386/kernel/sysenter.c Mon Mar 05 15:24:04 2007 -0800
+++ b/arch/i386/kernel/sysenter.c Mon Mar 05 15:27:31 2007 -0800
@@ -74,7 +74,12 @@ static struct page *syscall_pages[1];

int __init sysenter_setup(void)
{
- void *syscall_page = (void *)get_zeroed_page(GFP_ATOMIC);
+ void *syscall_page;
+
+ if (!vdso_enabled)
+ return 0;
+
+ syscall_page = (void *)get_zeroed_page(GFP_ATOMIC);
syscall_pages[0] = virt_to_page(syscall_page);

#ifdef CONFIG_COMPAT_VDSO
@@ -106,6 +111,11 @@ int arch_setup_additional_pages(struct l
struct mm_struct *mm = current->mm;
unsigned long addr;
int ret;
+
+ if (!vdso_enabled) {
+ current->mm->context.vdso = (void *)~0UL;
+ return 0;
+ }

down_write(&mm->mmap_sem);
addr = get_unmapped_area(NULL, 0, PAGE_SIZE, 0, 0);
diff -r fad0910252d2 arch/i386/mm/pgtable.c
--- a/arch/i386/mm/pgtable.c Mon Mar 05 15:24:04 2007 -0800
+++ b/arch/i386/mm/pgtable.c Mon Mar 05 16:06:31 2007 -0800
@@ -144,10 +144,8 @@ void set_pmd_pfn(unsigned long vaddr, un
}

static int fixmaps;
-#ifndef CONFIG_COMPAT_VDSO
unsigned long __FIXADDR_TOP = 0xfffff000;
EXPORT_SYMBOL(__FIXADDR_TOP);
-#endif

void __set_fixmap (enum fixed_addresses idx, unsigned long phys, pgprot_t flags)
{
@@ -174,11 +172,13 @@ void reserve_top_address(unsigned long r
printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
(int)-reserve);
#ifdef CONFIG_COMPAT_VDSO
- BUG_ON(reserve != 0);
-#else
+ if (reserve != 0) {
+ printk(KERN_WARNING "Compat VDSO is incompatible with fixmap relocation - disabling VDSO\n");
+ vdso_enabled = 0;
+ }
+#endif
__FIXADDR_TOP = -reserve - PAGE_SIZE;
__VMALLOC_RESERVE += reserve;
-#endif
}

pte_t *pte_alloc_one_kernel(struct mm_struct *mm, unsigned long address)
diff -r fad0910252d2 include/asm-i386/elf.h
--- a/include/asm-i386/elf.h Mon Mar 05 15:24:04 2007 -0800
+++ b/include/asm-i386/elf.h Mon Mar 05 15:44:43 2007 -0800
@@ -137,7 +137,7 @@ extern int dump_task_extended_fpu (struc

#ifdef CONFIG_COMPAT_VDSO
# define VDSO_COMPAT_BASE VDSO_HIGH_BASE
-# define VDSO_PRELINK VDSO_HIGH_BASE
+# define VDSO_PRELINK 0xffffe000UL
#else
# define VDSO_COMPAT_BASE VDSO_BASE
# define VDSO_PRELINK 0
diff -r fad0910252d2 include/asm-i386/fixmap.h
--- a/include/asm-i386/fixmap.h Mon Mar 05 15:24:04 2007 -0800
+++ b/include/asm-i386/fixmap.h Mon Mar 05 15:59:30 2007 -0800
@@ -14,19 +14,6 @@
#define _ASM_FIXMAP_H

-/* used by vmalloc.c, vsyscall.lds.S.
- *
- * Leave one empty page between vmalloc'ed areas and
- * the start of the fixmap.
- */
-#ifndef CONFIG_COMPAT_VDSO
-extern unsigned long __FIXADDR_TOP;
-#else
-#define __FIXADDR_TOP 0xfffff000
-#define FIXADDR_USER_START __fix_to_virt(FIX_VDSO)
-#define FIXADDR_USER_END __fix_to_virt(FIX_VDSO - 1)
-#endif
-
#ifndef __ASSEMBLY__
#include <linux/kernel.h>
#include <asm/acpi.h>
@@ -35,6 +22,15 @@ extern unsigned long __FIXADDR_TOP;
#ifdef CONFIG_HIGHMEM
#include <linux/threads.h>
#include <asm/kmap_types.h>
+#endif
+
+/* used by vmalloc.c, vsyscall.lds.S, elf.h, pgtable.c */
+extern unsigned long __FIXADDR_TOP;
+
+/* used for dumping VDSO to core files */
+#ifdef CONFIG_COMPAT_VDSO
+#define FIXADDR_USER_START __fix_to_virt(FIX_VDSO)
+#define FIXADDR_USER_END __fix_to_virt(FIX_VDSO - 1)
#endif

/*