Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755695AbaBTRgs (ORCPT ); Thu, 20 Feb 2014 12:36:48 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:13799 "EHLO SMTP02.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752497AbaBTRgr (ORCPT ); Thu, 20 Feb 2014 12:36:47 -0500 X-IronPort-AV: E=Sophos;i="4.97,513,1389744000"; d="scan'208";a="102714100" Message-ID: <53063D2B.1040502@citrix.com> Date: Thu, 20 Feb 2014 17:36:43 +0000 From: Zoltan Kiss User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Stefano Stabellini CC: Konrad Rzeszutek Wilk , Boris Ostrovsky , David Vrabel , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , , , , =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= , Jan Beulich , Ian Campbell Subject: Re: [PATCH v9] xen/grant-table: Avoid m2p_override during mapping References: <1392238453-26147-1-git-send-email-zoltan.kiss@citrix.com> <5301F74E.3070107@citrix.com> In-Reply-To: Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.80.2.133] X-DLP: MIA2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 20/02/14 17:26, Stefano Stabellini wrote: > On Mon, 17 Feb 2014, Zoltan Kiss wrote: >> On 16/02/14 18:36, Stefano Stabellini wrote: >>> On Wed, 12 Feb 2014, Zoltan Kiss wrote: >>>> diff --git a/arch/arm/include/asm/xen/page.h >>>> b/arch/arm/include/asm/xen/page.h >>>> index e0965ab..4eaeb3f 100644 >>>> --- a/arch/arm/include/asm/xen/page.h >>>> +++ b/arch/arm/include/asm/xen/page.h >>>> @@ -97,16 +97,15 @@ static inline pte_t *lookup_address(unsigned long >>>> address, unsigned int *level) >>>> return NULL; >>>> } >>>> >>>> -static inline int m2p_add_override(unsigned long mfn, struct page *page, >>>> - struct gnttab_map_grant_ref *kmap_op) >>>> -{ >>>> - return 0; >>>> -} >>>> - >>>> -static inline int m2p_remove_override(struct page *page, bool clear_pte) >>>> -{ >>>> - return 0; >>>> -} >>>> +extern int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops, >>>> + struct gnttab_map_grant_ref *kmap_ops, >>>> + struct page **pages, unsigned int count, >>>> + bool m2p_override); >>>> + >>>> +extern int clear_foreign_p2m_mapping(struct gnttab_unmap_grant_ref >>>> *unmap_ops, >>>> + struct gnttab_map_grant_ref *kmap_ops, >>>> + struct page **pages, unsigned int count, >>>> + bool m2p_override); >>> >>> Much much better. >>> The only comment I have is about this m2p_override boolean parameter. >>> m2p_override is now meaningless in this context, what we really want to >>> let the arch specific implementation know is whether the mapping is a >>> kernel only mapping or a userspace mapping. >>> Testing for kmap_ops != NULL might even be enough, but it would not >>> improve the interface. >> gntdev is the only user of this, the kmap_ops parameter there is: >> use_ptemod ? map->kmap_ops + offset : NULL >> where: >> use_ptemod = !xen_feature(XENFEAT_auto_translated_physmap); >> So I think we can't rely on kmap_ops to decide whether we should m2p_override >> or not. >> >>> Is it possible to realize if the mapping is a userspace mapping by >>> checking for GNTMAP_application_map in map_ops? >>> Otherwise I would keep the boolean and rename it to user_mapping. >> Sounds better, but as far as I see gntdev set that flag in find_grant_ptes, >> which is called only >> >> if (use_ptemod) { >> err = apply_to_page_range(vma->vm_mm, vma->vm_start, >> vma->vm_end - vma->vm_start, >> find_grant_ptes, map); >> >> So if xen_feature(XENFEAT_auto_translated_physmap), we don't have kmap_ops, >> and GNTMAP_application_map is not set as well, but I guess we still need >> m2p_override. Or not? I'm a bit confused, maybe because of Monday ... > > If xen_feature(XENFEAT_auto_translated_physmap) we shouldn't need the > m2p_override. > So it's safe to assume that we need m2p_override only if kmap_ops != NULL, and we can avoid the extra bool parameter, is that correct? At least with the current users of grant mapping it seems to be true. In which case we don't need the wrappers for gnttab_[un]map_refs as well. Actually the most of m2p_add/remove_override takes effect only if there is a kmap_op parameter, but what about the rest of the code there? Zoli -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/