On 05/04/2011 02:16 PM, Marcelo Tosatti wrote:
On Sun, May 01, 2011 at 02:33:07PM +0900, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa<yoshikawa.takuya@xxxxxxxxxxxxx>
>
> 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().
>
> Note that the unlikely annotations are used to show that the conditions
> are something unusual rather than for performance.
>
> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@xxxxxxxxxxxxx>
> ---
> arch/x86/kvm/paging_tmpl.h | 26 ++++++++++++--------------
> 1 files changed, 12 insertions(+), 14 deletions(-)
Hi Takuya,
>
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 52450a6..f9d9af1 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -79,21 +79,19 @@ static gfn_t gpte_to_gfn_lvl(pt_element_t gpte, int lvl)
> }
>
> 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)
> + npages = get_user_pages_fast((unsigned long)ptep_user, 1, 1,&page);
> + /* Check if the user is doing something meaningless. */
> + if (unlikely(npages != 1))
> return -EFAULT;
>
> - page = gfn_to_page(vcpu->kvm, gpa_to_gfn(gpa));
> -
gfn_to_page is the interface for mapping guest pages inside KVM,
and you're bypassing it for IMO no good reason (i doubt there's any
performance improvement by skipping the translation).
He isn't skipping it - he's using gfn_to_hva() to derive ptep_user,
which is equivalent.
The motivation isn't performance, it's to ensure that cmpxchg_gpte()
operates on the same address as we read it from.
(btw, we're missing a mark_page_dirty() here, no?)
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
