From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM: MMU: Use ptep_user for cmpxchg_gpte() Date: Sat, 30 Apr 2011 22:48:39 +0300 Message-ID: <4DBC6797.3080002@redhat.com> References: <20110430235052.b8289513.takuya.yoshikawa@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, yoshikawa.takuya@oss.ntt.co.jp To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:5823 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751291Ab1D3Tsu (ORCPT ); Sat, 30 Apr 2011 15:48:50 -0400 In-Reply-To: <20110430235052.b8289513.takuya.yoshikawa@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 04/30/2011 05:50 PM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa > > The address of the gpte was already calculated and stored in ptep_user > before entering cmpxchg_gpte(). > > This patch makes cmpxchg_gpte() to use that to make it clear that we > are using the same address during walk_addr_generic(). > > Signed-off-by: Takuya Yoshikawa > --- > Note: I tested this but could not see cmpxchg_gpte() being called. > Is there any good test case? Run with npt or ept disabled. With a normal OS you'll see A and D bits being set when the guest is under memory pressure, or when the guest uses a shared writeable mapping. You can also try kvm-unit-tests.git's x86/access.flat test (also with npt/ept disabled). > static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > - gfn_t table_gfn, unsigned index, > - pt_element_t orig_pte, pt_element_t new_pte) > + pt_element_t __user *ptep_user, unsigned index, > + pt_element_t orig_pte, pt_element_t new_pte) > { > + int npages; > pt_element_t ret; > pt_element_t *table; > struct page *page; > - gpa_t gpa; > > - gpa = mmu->translate_gpa(vcpu, table_gfn<< PAGE_SHIFT, > - PFERR_USER_MASK|PFERR_WRITE_MASK); > - if (gpa == UNMAPPED_GVA) > - return -EFAULT; > - > - page = gfn_to_page(vcpu->kvm, gpa_to_gfn(gpa)); > + npages = get_user_pages_fast((unsigned long)ptep_user, 1, 1,&page); > + BUG_ON(npages != 1); This BUG_ON() is user triggerable - have one thread munmap() guest memory while another thread runs guest code which triggers this path. You need to return an error here. Or maybe you can just return - the user is doing something meaningless and there's no correct action here. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.