[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH]: Fix Xen domU boot with batched mprotect



Jan Beulich wrote:
Jeremy Fitzhardinge <jeremy@xxxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.