From: Jeremy Fitzhardinge Subject: Re: [PATCH]: Fix Xen domU boot with batched mprotect Date: Fri, 17 Oct 2008 08:19:19 -0700 Message-ID: <48F8ACF7.6050905@goop.org> References: <48F5CE10.3060403@redhat.com> <48F6274D.76E4.0078.0@novell.com> <48F61919.2050005@goop.org> <48F72C7A.76E4.0078.0@novell.com> <48F76773.2090700@goop.org> <48F85711.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <48F85711.76E4.0078.0@novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: Ingo Molnar , "xen-devel@lists.xensource.com" , Chris Lalancette , linux-kernel@vger.kernel.org List-Id: xen-devel@lists.xenproject.org Jan Beulich wrote: >>>> Jeremy Fitzhardinge 16.10.08 18:10 >>> >>>> >> 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; >> > > This, imo, is still broken (i.e. the name of the function still isn't matched > by the implementation): KERNEL_IMAGE_SIZE is a constant and doesn't > account for the fact that only the real kernel image can be relied upon > to be mapped. > Perhaps, but I don't think it matters too much. Unless you have a tiny amount of physical memory, locations in the kernel mapping beyond the actual kernel will still resolve to proper locations in the linear map. >> and 32-bit is similar (but simpler, since it doesn't need to worry about a separate kernel mapping). >> > > This continues to be broken, but not as badly as it used to be - while it > now covers user space and the vmalloc area (I'm unclear why this is > excluded only after booting completed, though), hypervisor space > continues to not be considered here. > > But as mentioned before - excluding the vmalloc area seems bogus wrt > the name of the function, but as I take it the confusion here is intended. > I think a strictly correct name for the function would be can_i_use___pa_on_this_address(vaddr). It isn't is_this_really_an_addressable_location(vaddr). J