[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48F76773.2090700@goop.org>
Date: Thu, 16 Oct 2008 09:10:27 -0700
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Jan Beulich <jbeulich@...ell.com>
CC: "xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
Chris Lalancette <clalance@...hat.com>,
linux-kernel@...r.kernel.org
Subject: Re: [Xen-devel] [PATCH]: Fix Xen domU boot with batched mprotect
Jan Beulich wrote:
>>>> Jeremy Fitzhardinge <jeremy@...p.org> 15.10.08 18:23 >>>
>>>>
>> virt_addr_valid() is supposed to be usable in this circumstace. The
>> comment says "virt_to_page(kaddr) returns a valid pointer if and only if
>> virt_addr_valid(kaddr) returns true", which implies that
>> virt_addr_valid() returns a meaningful result on all addresses - and if
>> not, it should be fixed.
>>
>
> Where did you find this comment? I had no luck grep-ing for it...
>
It's in tip.git, which has quite a few changes in this area.
http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=include/asm-x86/page.h;h=d4f1d5791fc186f29a9a60d4fe182d80f05038e4;hb=HEAD
http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=arch/x86/mm/ioremap.c;h=ae71e11eb3e5e4ddeceadc9128d3afea564f27e0;hb=HEAD
> In any case, if that's the expectation, then on i386 virt_addr_valid()
> must be implemented as something like
>
> #define virt_addr_valid(kaddr) ((kaddr) >= PAGE_OFFSET && (kaddr) < high_memory && pfn_valid(__pa(kaddr) >> PAGE_SHIFT))
>
> x86-64 would need something similar, except that high_memory obviously
> must be replaced (or that part could perhaps be left out altogether), and
> the un-mapped addresses above the kernel mapping would need to be
> filtered out.
>
> Btw., if you look at other architectures, you'll see that most of them use
> the same (as you say broken) construct.
>
> Otoh, if that cited statement really holds, then virt_addr_valid() isn't
> really expected to do what its name implies: In particular, there are
> valid address ranges in kernel space which it wouldn't be permitted to
> return true on without significantly complicating the virt_to_page()
> implementation (e.g. x86-64's vmalloc and modules areas).
>
The current x86-64 implementation is:
bool __virt_addr_valid(unsigned long x)
{
if (x >= __START_KERNEL_map) {
x -= __START_KERNEL_map;
if (x >= KERNEL_IMAGE_SIZE)
return false;
x += phys_base;
} else {
if (x < PAGE_OFFSET)
return false;
x -= PAGE_OFFSET;
if (system_state == SYSTEM_BOOTING ?
x > MAXMEM : !phys_addr_valid(x)) {
return false;
}
}
return pfn_valid(x >> PAGE_SHIFT);
}
and 32-bit is similar (but simpler, since it doesn't need to worry about a separate kernel mapping).
>
> yields a positive indication from virt_addr_valid() on all tested addresses:
>
> <4>null: 00000000 00040000 1:1
> <4>half: 7fffffff 000bffff 1:1
> <4>hm-p: ed7ff000 0002d7ff 1:1
> <4>hm-1: ed7fffff 0002d7ff 1:1
> <4>hm: ed800000 0002d800 1:1
> <4>hm+1: ed800001 0002d800 1:1
> <4>hm+p: ed801000 0002d801 1:1
> <4>km: f56fa000 000356fa 1:1
> <4>hv: f5800000 00035800 1:1
>
It would be interesting to try that with tip.git's version of
__virt_addr_valid(). In the Xen case, all we need is a guarantee that
virt_addr_valid() returns true iff __pa(addr) returns a proper result,
so that we can use the resulting pfn as an index into pfn->mfn. I
believe this is what the current implementation does.
J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists