Re: [PATCH] MIPS: Close races in TLB modify handlers.

[David Daney <david.s.daney@gmail.com>]

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