public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Pekka Paalanen <pq@iki.fi>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Vegard Nossum <vegard.nossum@gmail.com>,
	Stuart Bennett <stuart@freedesktop.org>
Subject: Re: testing pmdval/pteval page presence bit
Date: Tue, 10 Feb 2009 14:42:56 -0800	[thread overview]
Message-ID: <499202F0.10108@goop.org> (raw)
In-Reply-To: <20090209215849.5a788261@daedalus.pq.iki.fi>

Pekka Paalanen wrote:
> Hi all,
>
> This question is related to mmiotrace which toggles the page presence
> bit to trigger page faults on ioremapped regions. Page faults are used
> to trace MMIO reads and writes of proprietary drivers.
>
> I understood that large pages use pmd's instead of pte's. If there is a
> union like this:
>
> +	union {
> +		pmdval_t pmdval;
> +		pteval_t pteval;
> +	} saved;	/* stored value prior to arming */
>
> and it is being assigned the proper content, as in the following:
>
> +static int clear_page_present(struct kmmio_fault_page *f, bool clear)
>  {
>  	pteval_t pteval;
>  	pmdval_t pmdval;
>  	unsigned int level;
>  	pmd_t *pmd;
> +	pte_t *pte = lookup_address(f->page, &level);
>  
>  	if (!pte) {
> +		pr_err("kmmio: no pte for page 0x%08lx\n", f->page);
>  		return -1;
>  	}
>  
>  	switch (level) {
>  	case PG_LEVEL_2M:
>  		pmd = (pmd_t *)pte;
> +		if (clear) {
> +			f->saved.pmdval = pmd_val(*pmd);
> +			pmdval = f->saved.pmdval & ~_PAGE_PRESENT;
> +		} else
> +			pmdval = f->saved.pmdval;
>  		set_pmd(pmd, __pmd(pmdval));
>  		break;
>  
>  	case PG_LEVEL_4K:
> +		if (clear) {
> +			f->saved.pteval = pte_val(*pte);
> +			pteval = f->saved.pteval & ~_PAGE_PRESENT;
> +		} else
> +			pteval = f->saved.pteval;
>  		set_pte_atomic(pte, __pte(pteval));
>  		break;
>
>
> Then regardless of was it pmdval or pteval being set, the test
>
> 	if (!(faultpage->saved.pteval & _PAGE_PRESENT))
>
> should be ok. But is it?
> Can large page (pmd) presence be handled just like a normal page (pte)?
>   

_PAGE_PRESENT is meaningful for both ptes and pmds; you can use 
pmd_present() to test for it rather than open-coding it.

But there's one other theoretical problem with this code.  In general it 
isn't safe to just toggle the _PAGE_PRESENT bit on its own, because the 
rest of the non-present pte could get interpreted as a swap entry.  If 
you're guaranteed that these are kernel mappings then there's no problem 
in practice.

    J

  reply	other threads:[~2009-02-10 22:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-09 19:58 testing pmdval/pteval page presence bit Pekka Paalanen
2009-02-10 22:42 ` Jeremy Fitzhardinge [this message]
2009-02-11 18:08   ` Pekka Paalanen
2009-02-11 18:51     ` 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=499202F0.10108@goop.org \
    --to=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=pq@iki.fi \
    --cc=stuart@freedesktop.org \
    --cc=vegard.nossum@gmail.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