Re: [Xen-devel] [PATCH] add locking around certain calls to map_pages_to_xen()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.07.13 at 13:37, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 11/07/13 12:30, Jan Beulich wrote:
>> While boot time calls don't need this, run time uses of the function
>> which may result in L2 page tables to get populated need to be
>> serialized to avoid two CPUs populating the same L2 (or L3) entry,
>> overwriting each other's results.
>>
>> This fixes what would seem to be a regression from commit b0581b92
>> ("x86: make map_domain_page_global() a simple wrapper around vmap()"),
>> albeit that change only made more readily visible the already existing
>> issue.
>>
>> The __init addition to memguard_init(), while seemingly unrelated,
>> helps making obvious that this function's use of map_pages_to_xen() is
>> a boot time only one.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -5757,13 +5757,33 @@ void destroy_xen_mappings(unsigned long 
>>  void __set_fixmap(
>>      enum fixed_addresses idx, unsigned long mfn, unsigned long flags)
>>  {
>> -    BUG_ON(idx >= __end_of_fixed_addresses);
>> +    /*
>> +     * As the fixmap area requires more than a single L2 entry, we need a 
> lock
>> +     * to avoid races of map_pages_to_xen() populating those. There's no 
> need
>> +     * for this at boot time, and not doing so avoids needing to disable 
> IRQs
>> +     * while holding the lock.
>> +     */
>> +    static DEFINE_SPINLOCK(lock);
>> +
>> +    if ( system_state > SYS_STATE_boot )
>> +       spin_lock(&lock);
>>      map_pages_to_xen(fix_to_virt(idx), mfn, 1, flags);
>> +    if ( system_state > SYS_STATE_boot )
>> +        spin_unlock(&lock);
> 
> Perhaps read system_state to a local variable?
> 
> While I really hope this cant happen, a race condition with system_state
> changing would be a very bad thing here.

That really can't happen, and hence I don't see the need to use
a local variable for latching the state.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel