From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Lalancette Subject: 32-on-64 support in xen-unstable? Date: Mon, 06 Jul 2009 16:18:25 +0200 Message-ID: <4A5207B1.3030808@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org Hello, I've been browsing through the preemptible pagetable stuff, and ran across a piece of code that I don't understand or is buggy. Looking at arch/x86/mm.c:new_guest_cr3(), we have this code for 32-on-64 support: if ( is_pv_32on64_domain(d) ) { okay = paging_mode_refcounts(d) ? 0 /* Old code was broken, but what should it be? */ : mod_l4_entry( __va(pagetable_get_paddr(curr->arch.guest_table)), l4e_from_pfn( mfn, (_PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_ACCESSED)), pagetable_get_pfn(curr->arch.guest_table), 0, 0, curr) == 0; if ( unlikely(!okay) ) { MEM_LOG("Error while installing new compat baseptr %lx", mfn); return 0; } However, looking at arch/x86/mm.c:mod_l4_entry(), it returns 0 on success and < 0 on error. It seems to me that booting a 32-bit PV guest is always going to fall into the !okay check on success, when it really shouldn't. Shouldn't this be something like: okay = paging_mode_refcounts(d) ? 0 /* Old code was broken, but what should it be? */ : !mod_l4_entry( __va(pagetable_get_paddr(curr->arch.guest_table)), l4e_from_pfn( mfn, (_PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_ACCESSED)), pagetable_get_pfn(curr->arch.guest_table), 0, 0, curr) == 0; Or am I just missing something? (NOTE: this still leaves open the possibility for failure if mod_l4_entry() returns EINTR or EAGAIN, but maybe we don't have to worry about that here?) Thanks, -- Chris Lalancette