[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] x86: Don't crash when mapping a page using EFI runtime page tables
On 05/27/2015 12:59 PM, Jan Beulich wrote:
On 27.05.15 at 12:23, <ross.lagerwall@xxxxxxxxxx> wrote:
On 05/18/2015 03:58 PM, Jan Beulich wrote:
On 15.05.15 at 18:08, <ross.lagerwall@xxxxxxxxxx> wrote:
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -32,20 +32,25 @@ static inline struct vcpu *mapcache_current_vcpu(void)
return NULL;
/*
+ * When using efi runtime page tables, we have the equivalent of the
idle
+ * domain's page tables but current may point at another domain's VCPU.
+ * Return NULL as though current is not properly set up yet.
+ */
+ if ( efi_enabled && read_cr3() == efi_rs_page_table() )
+ return NULL;
I'm okay with the patch in principle; what worries me is the CR3 read
that is now going to be necessary even in non-debug builds. With
this code being the only user of efi_rs_page_table(), I wonder if it
wouldn't make sense to alter that function to return non-zero only
when spin_is_locked(&efi_rs_lock), and then alter the code above
such that the CR3 read would happen only when we got a non-zero
value back.
mapcache_current_vcpu() appears to be called from IRQ-enabled and
IRQ-disabled callers which prevents us from using the spinlock.
I didn't suggest to use any spin lock; I merely suggested checking
whether that particular one is being held by someone (to avoid the
CR3 read if that's not the case).
spin_is_locked() calls check_lock() which causes a BUG_ON() even though
you're not actually using the lock.
--
Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
