From: Tim Deegan <tim@xen.org>
To: Mukesh Rathor <mukesh.rathor@oracle.com>
Cc: Keir Fraser <keir.xen@gmail.com>,
"Xen-devel@lists.xensource.com" <Xen-devel@lists.xensource.com>,
Ian Campbell <Ian.Campbell@citrix.com>,
"stefano.stabellini@eu.citrix.com"
<stefano.stabellini@eu.citrix.com>
Subject: Re: [hybrid]: code review for function mapping pfn to foreign mfn
Date: Tue, 24 Apr 2012 10:36:26 +0100 [thread overview]
Message-ID: <20120424093626.GC34721@ocelot.phlegethon.org> (raw)
In-Reply-To: <20120423183709.5636656f@mantra.us.oracle.com>
At 18:37 -0700 on 23 Apr (1335206229), Mukesh Rathor wrote:
> On Thu, 19 Apr 2012 15:15:27 +0100
> Tim Deegan <tim@xen.org> wrote:
>
> > At 18:29 -0700 on 13 Apr (1334341792), Mukesh Rathor wrote:
> > - AFAICT you're using set_mmio_p2m_entry and adding a new unmap
> > operation just to avoid having the m2p updated. Since you can't
> > rely on the unmap always happening through the new call (and you don't
> > enforce it anywhere), it would be better to add a new p2m_type
> > just for non-grant foreign mappings. Then you can gate the m2p
> > updates in the existing code on the map being normal RAM, as is
> > already done for p2m_is_grant().
>
> Hi Tim,
>
> The variants of get_page* are confusing me, so wanna double check with
> you. I should be able to do something like following, right?
>
[...]
> if ( (rc=get_page_and_type_from_pagenr(mfn, PGT_writable_page,fdom,0,0)) ) {
> put_pg_owner(fdom);
> return rc;
> }
Yes, but:
- You should use get_page_from_pagenr() if fdom is paging_mode_external()
and reference/copy the comment in get_page_from_l1e() to explain why:
/* Foreign mappings into guests in shadow external mode don't
* contribute to writeable mapping refcounts. (This allows the
* qemu-dm helper process in dom0 to map the domain's memory without
* messing up the count of "real" writable mappings.) */
- You should drop the refcount (and typecount, if you took one) if the
mapping fails.
- You need to make sure that _any_ path that removes the mapping drops
the ref/type (_after_ any TLB flushes have happened, please!) Maybe
the best way to do that is in ept_set_entry() for EPT and
paging_write_p2m_entry() for NPT/shadow.
- You need to handle the p2m teardown path as well. I think the best
way to do that is to hoist the relinquish_shared_pages() loop
up into a new function in p2m.c, and add your put_page[_and_type]
calls in there.
Cheers,
Tim.
next prev parent reply other threads:[~2012-04-24 9:36 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-14 1:29 [hybrid]: code review for function mapping pfn to foreign mfn Mukesh Rathor
2012-04-16 13:53 ` Ian Campbell
2012-04-16 14:02 ` Tim Deegan
2012-04-17 1:53 ` Mukesh Rathor
2012-04-17 9:05 ` Ian Campbell
2012-04-18 23:29 ` Mukesh Rathor
2012-04-19 7:22 ` Ian Campbell
2012-04-19 14:15 ` Tim Deegan
2012-04-24 1:37 ` Mukesh Rathor
2012-04-24 9:36 ` Tim Deegan [this message]
2012-04-24 23:06 ` Mukesh Rathor
2012-04-26 9:08 ` Tim Deegan
2012-04-26 18:18 ` Mukesh Rathor
2012-04-26 19:57 ` Tim Deegan
2012-04-27 1:56 ` Mukesh Rathor
2012-04-27 8:51 ` Tim Deegan
[not found] <mailman.2710.1334825330.1399.xen-devel@lists.xen.org>
2012-04-19 14:40 ` Andres Lagar-Cavilla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120424093626.GC34721@ocelot.phlegethon.org \
--to=tim@xen.org \
--cc=Ian.Campbell@citrix.com \
--cc=Xen-devel@lists.xensource.com \
--cc=keir.xen@gmail.com \
--cc=mukesh.rathor@oracle.com \
--cc=stefano.stabellini@eu.citrix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.