From: Ralf Baechle <ralf@linux-mips.org>
To: David Daney <david.daney@cavium.com>
Cc: "Maciej W. Rozycki" <macro@linux-mips.org>, linux-mips@linux-mips.org
Subject: Re: [PATCH] MIPS: Close races in TLB modify handlers.
Date: Fri, 1 Jul 2011 09:27:06 +0100 [thread overview]
Message-ID: <20110701082706.GA8308@linux-mips.org> (raw)
In-Reply-To: <4E0D0D8C.7000200@cavium.com>
On Thu, Jun 30, 2011 at 04:58:04PM -0700, David Daney wrote:
> On 06/30/2011 04:34 PM, Maciej W. Rozycki wrote:
> >Hi David,
> >
> >>Page table entries are made invalid by writing a zero into the the PTE
> >>slot in a page table. This creates a race condition with the TLB
> >>modify handlers when they are updating the PTE.
> >>
> >>CPU0 CPU1
> >>
> >>Test for _PAGE_PRESENT
> >>. set to not _PAGE_PRESENT (zero)
> >>Set to _PAGE_VALID
> >>
> >>So now the page not present value (zero) is suddenly valid and user
> >>space programs have access to physical page zero.
> >>
> >>We close the race by putting the test for _PAGE_PRESENT and setting of
> >>_PAGE_VALID into an atomic LL/SC section. This requires more
> >>registers than just K0 and K1 in the handlers, so we need to save some
> >>registers to a save area and then restore them when we are done.
> >
> > Hmm, good catch, but doesn't your change pessimise the UP case?
>
> It may, It is really just a first version of the patch. I am
> looking for feedback and testing.
>
> >It looks
> >to me like you save& restore the scratch registers even though the race
> >does not apply to UP (you can't interrupt a TLB handler, not at this
> >stage).
>
> That's right. I will look at trying to generate the old code
> sequences for non-SMP.
We can replace all the CONFIG_SMPs in tlbex.c (existing and those added
by your patch) with num_possible_cpus > 1 which will improve readability
and give SMP kernels running on a single processor the uniprocessor TLB
exception handler.
But that's something for a followup patch; your patch is big enough as it
is, it's not as straight forward as it may sound and the 3.0 clock is
ticking ...
Ralf
next prev parent reply other threads:[~2011-07-01 8:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-30 22:31 [PATCH] MIPS: Close races in TLB modify handlers David Daney
2011-06-30 23:34 ` Maciej W. Rozycki
2011-06-30 23:58 ` David Daney
2011-07-01 8:27 ` Ralf Baechle [this message]
2011-07-01 16:38 ` David Daney
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=20110701082706.GA8308@linux-mips.org \
--to=ralf@linux-mips.org \
--cc=david.daney@cavium.com \
--cc=linux-mips@linux-mips.org \
--cc=macro@linux-mips.org \
/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