From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/5] KVM: Make locked operations truly atomic Date: Wed, 17 Mar 2010 10:05:13 +0200 Message-ID: <4BA08D39.5030005@redhat.com> References: <1268654397-6650-1-git-send-email-avi@redhat.com> <1268654397-6650-3-git-send-email-avi@redhat.com> <4BA088A5.6000201@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33652 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753309Ab0CQIFQ (ORCPT ); Wed, 17 Mar 2010 04:05:16 -0400 In-Reply-To: <4BA088A5.6000201@web.de> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> --- >> 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.