From: "Jan Beulich" <jbeulich@novell.com>
To: "Jeremy Fitzhardinge" <jeremy@goop.org>,
"Chris Lalancette" <clalance@redhat.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
<linux-kernel@vger.kernel.org>
Subject: Re: [Xen-devel] [PATCH]: Fix Xen domU boot with batched mprotect
Date: Wed, 15 Oct 2008 16:24:29 +0100 [thread overview]
Message-ID: <48F6274D.76E4.0078.0@novell.com> (raw)
In-Reply-To: <48F5CE10.3060403@redhat.com>
>>> Chris Lalancette <clalance@redhat.com> 15.10.08 13:03 >>>
>The right thing to do is to use arbitrary_virt_to_machine, so that we can be
>sure we are modifying the right pfn. This unfortunately introduces a
>performance penalty because of a full page-table-walk, but we can avoid that
>penalty for pages in the p2m list by checking if virt_addr_valid is true, and if
>so, just doing the lookup in the p2m table.
Could you explain how virt_addr_valid() can validly be used here? Looking
at its implementation
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
a kaddr in kmap space (i.e. above high_memory) would return a bogus
physical address, and hence pfn_valid() on the resulting pfn is meaningless.
I'd instead simply compare the address in question against high_memory,
and perhaps instead of in arbitrary_virt_to_machine() in
ptep_modify_prot_commit() under an #ifdef CONFIG_HIGHPTE. But
performance-wise, CONFIG_HIGHPTE sucks under Xen anyway, so you'd
better not turn this on in the first place. We may want/need to provide
a means to disable this at run time so the same kernel when run natively
could still make use of it, but without impacting performance under Xen.
Jan
next prev parent reply other threads:[~2008-10-15 15:23 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-15 11:03 [PATCH]: Fix Xen domU boot with batched mprotect Chris Lalancette
2008-10-15 15:24 ` Jan Beulich [this message]
2008-10-15 16:23 ` [Xen-devel] " Jeremy Fitzhardinge
2008-10-16 7:28 ` Jan Beulich
2008-10-16 9:58 ` Jan Beulich
2008-10-16 16:10 ` Jeremy Fitzhardinge
2008-10-17 7:12 ` Jan Beulich
2008-10-17 15:19 ` Jeremy Fitzhardinge
2008-10-17 15:30 ` Jan Beulich
2008-10-17 15:36 ` Jeremy Fitzhardinge
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=48F6274D.76E4.0078.0@novell.com \
--to=jbeulich@novell.com \
--cc=clalance@redhat.com \
--cc=jeremy@goop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=xen-devel@lists.xensource.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox