>>> On 27.05.15 at 14:03, <ross.lagerw...@citrix.com> wrote: > On 05/27/2015 12:59 PM, Jan Beulich wrote: >>>>> On 27.05.15 at 12:23, <ross.lagerw...@citrix.com> wrote: >>> On 05/18/2015 03:58 PM, Jan Beulich wrote: >>>>>>> On 15.05.15 at 18:08, <ross.lagerw...@citrix.com> 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.
Then some other mechanism - spin_is_locked() would have produced false positives anyway. E.g. storing the CPU which managed to acquire the lock in efi_rs_enter(), and clearing it in efi_rs_leave() before dropping the lock. efi_rs_page_table() could then return non-zero on only this one CPU. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
