Re: [PATCH 2/5] KVM: Make locked operations truly atomic

[Avi Kivity <avi@redhat.com>]

On 03/17/2010 09:45 AM, Jan Kiszka wrote:
> Avi Kivity wrote:
>    
>> Once upon a time, locked operations were emulated while holding the mmu mutex.
>> Since mmu pages were write protected, it was safe to emulate the writes in
>> a non-atomic manner, since there could be no other writer, either in the
>> guest or in the kernel.
>>
>> These days emulation takes place without holding the mmu spinlock, so the
>> write could be preempted by an unshadowing event, which exposes the page
>> to writes by the guest.  This may cause corruption of guest page tables.
>>
>> Fix by using an atomic cmpxchg for these operations.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>>   arch/x86/kvm/x86.c |   69 ++++++++++++++++++++++++++++++++++++----------------
>>   1 files changed, 48 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 9d02cc7..d724a52 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3299,41 +3299,68 @@ int emulator_write_emulated(unsigned long addr,
>>   }
>>   EXPORT_SYMBOL_GPL(emulator_write_emulated);
>>
>> +#define CMPXCHG_TYPE(t, ptr, old, new) \
>> +	(cmpxchg((t *)(ptr), *(t *)(old), *(t *)(new)) == *(t *)(old))
>> +
>> +#ifdef CONFIG_X86_64
>> +#  define CMPXCHG64(ptr, old, new) CMPXCHG_TYPE(u64, ptr, old, new)
>> +#else
>> +#  define CMPXCHG64(ptr, old, new) \
>> +	(cmpxchg64((u64 *)(ptr), *(u64 *)(old), *(u *)(new)) == *(u64 *)(old))
>>      
>                                                  ^^^^^^
> This should cause the 32-bit build breakage I see with the current next
> branch.
>    

Also, Marcelo sees autotest breakage, so it's also broken on 64-bit somehow.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.