Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: David Daney <david.s.daney@gmail.com>
To: Ralf Baechle <ralf@linux-mips.org>
Cc: David Daney <david.daney@cavium.com>,
	"Maciej W. Rozycki" <macro@linux-mips.org>,
	linux-mips@linux-mips.org
Subject: Re: [PATCH] MIPS: Close races in TLB modify handlers.
Date: Fri, 01 Jul 2011 09:38:20 -0700	[thread overview]
Message-ID: <4E0DF7FC.7010007@gmail.com> (raw)
In-Reply-To: <20110701082706.GA8308@linux-mips.org>

On 07/01/2011 01:27 AM, Ralf Baechle wrote:
> 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 ...

I am testing a slight revision (using Context/XContext to get the 
logical CPU number instead of physical CPU number from EBase).  I will 
not send this revision until Tuesday because we are on holiday until then.

David Daney

      reply	other threads:[~2011-07-01 16:38 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
2011-07-01 16:38       ` David Daney [this message]

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=4E0DF7FC.7010007@gmail.com \
    --to=david.s.daney@gmail.com \
    --cc=david.daney@cavium.com \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@linux-mips.org \
    --cc=ralf@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