LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andi Kleen <ak@suse.de>, Zachary Amsden <zach@vmware.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] i386: For debugging, make the initial page table setup less forgiving.
Date: Wed, 25 Apr 2007 17:45:07 -0600	[thread overview]
Message-ID: <m13b2oow70.fsf@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <462FDF6D.8050903@goop.org> (Jeremy Fitzhardinge's message of "Wed, 25 Apr 2007 16:08:29 -0700")

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Eric W. Biederman wrote:
>> No.  Please just remove the conditionals on the leaf pages.
>>   
>
> So, to be specific, you mean make updating the pte_t entries (and pmd_t
> entries which refer to hugepages) entries unconditional?

I mean make updating pte_t and pmd_t entries that refer to identity
mapped physical pages unconditional.

>> We know exactly what we require them to be, there is minimal
>> cost and no downside to just setting the pte entries to
>> what we want them to be for the identity mapping.
>>
>> It doesn't make sense for paravirtualization or anything else to 
>> influence that.
>>
>> This may be redoing work that has been done before but it is
>> doing it all one common place.
>>   
>
> The issue is not a matter of avoiding duplicate work, but making sure
> all the pagetables are consistent from Xen's perspective.
>
> Specifically, you may not ever, at any time, create a writable mapping
> of a page which is currently part of an active pagetable.  This means
> that when we're creating mappings of physical memory, the pages which
> are part of the current pagetable must be mapped RO.  The easiest way I
> found to guarantee that is to copy the Xen-provided pagetable as a
> template, and only update pages which are missing.

Hmm.  I now see your problem.

> The other way I could do this is to have special-purpose init-time
> version of xen_set_pte which checks to see if it's making a RO mapping
> RW, and refuse to do it.  That would minimize the changes to mm/init.c,
> but give init-time set_pte rather unexpected hidden semantics.

Yes.  However how do we handle attempting to create this kind
of mapping when mmap /dev/mem?  or /dev/kmem?

I'm pretty certain there are other paths through the kernel where
we can get page table mapping.

Right now by leaving things read-only you are hiding from the kernel 
what you are really trying to do.  That makes me distinctly
uncomfortable.  In general when things get swept under the rug
we can never handle the properly.  Although this issue may be small
enough it doesn't matter.

I suspect what we want to do is come up with a function to call
to test to see if a page should be read-only and map such pages
_PAGE_KERNEL_RO, or _PAGE_KERNEL_RO_EXEC if it's code.

Speaking of things what are paravirt_alloc_pd and parafirt_alloc_pd 
supposed to do?

Eric

  reply	other threads:[~2007-04-25 23:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-13 21:49 H. Peter Anvin
2007-04-13 22:18 ` Zachary Amsden
2007-04-13 22:26   ` H. Peter Anvin
2007-04-13 22:40     ` Zachary Amsden
2007-04-13 22:26   ` Jeremy Fitzhardinge
2007-04-25 11:48 ` Andrew Morton
2007-04-25 15:28   ` H. Peter Anvin
2007-04-25 17:50     ` Eric W. Biederman
2007-04-25 17:56       ` H. Peter Anvin
2007-04-25 18:23         ` Eric W. Biederman
2007-04-25 18:18       ` Jeremy Fitzhardinge
2007-04-25 19:01         ` Eric W. Biederman
2007-04-25 19:19           ` Jeremy Fitzhardinge
2007-04-25 22:08           ` Jeremy Fitzhardinge
2007-04-25 22:27             ` Eric W. Biederman
2007-04-25 23:08               ` Jeremy Fitzhardinge
2007-04-25 23:45                 ` Eric W. Biederman [this message]
2007-04-26  0:14                   ` Jeremy Fitzhardinge
2007-04-27  5:02                     ` Eric W. Biederman
2007-04-26  3:27                   ` Zachary Amsden

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=m13b2oow70.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zach@vmware.com \
    --subject='Re: [PATCH] i386: For debugging, make the initial page table setup less forgiving.' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).