[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v9] xen/grant-table: Avoid m2p_override during mapping

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
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
+                                    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
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

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?


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.