[PATCH] ARM: mm: ensure TTBR0 is restored when changing ASID on rollover

On Wed, Jun 08, 2011 at 10:12:23PM +0100, Catalin Marinas wrote:

On 8 June 2011 21:36, Russell King - ARM Linux [off-list ref] wrote:

quoted

On Wed, Jun 08, 2011 at 09:23:23PM +0100, Will Deacon wrote:

quoted

On Wed, Jun 08, 2011 at 09:01:06PM +0100, Russell King - ARM Linux wrote:

quoted

However, these patches are introducing a brand new race between the
switch_mm code and the reset_context code.

With the new switch_mm() code, we switch TTBR0 to be the same as TTBR1.
If we then receive an IPI for reset_context(), we will change TTBR0
to point at a set of page tables which don't contain just global mappings.

After returning from reset_context(), we will resume switch_mm(), and
change the ASID value with the page tables pointing to non-global
mappings, violating the whole reason for the switch_mm() change.

Whilst this is a new race condition, it is analagous to the one we have
already and could be fixed at the same time.

Ok, I think we should revert the original patches then. ?They were rushed
in during the merge window, and as can be seen, rushing in patches because
we _think_ they're right is never the correct thing to do - we've ended
up with a completely broken situation as stuff now stands.

We rushed a series of patches fixing this but you didn't like the
patch disabling interrupts around cpu_switch_mm().

No.  We rushed the entire thing as can be seen due to the "forgotten"
patch and the submission of it _during_ the merge window.  Clearly
no one had thought enough about the inter-relationship between the
patches when deciding that only some of the patches should go in.

That's why there's a big reason to avoid merging any new patches during
the merge window - it gives time for such things to be found before it
becomes a problem.

Please note that the old switch_mm code with reserved ASID is broken
on A15 (and not just in theory), hence the need to use reserved TTBR0.

But A15 is not actively supported in mainline yet so that's not a
decision relevant to what we do with mainline.

quoted

Let's take out these changes and sort it out properly - not only do we
need to sort out these problems but we should also get rid of the
__ARCH_WANT_INTERRUPTS_ON_CTXSW thing completely. ?I have a patch which
I've only tested on SA-1110 which does this so far, but it needs a little
more work to clean up some stuff.

Even if you get rid of __ARCH_WANT_INTERRUPTS_ON_CTXSW, I would much
prefer to use the new switch_mm code as a base rather than going back
to the reserved ASID. The simplest way to fix the race condition you
mentioned is to also integrate the other patch from Will which
disables the interrupts around cpu_switch_mm(). After that we have
more time to review the __ARCH_WANT_INTERRUPTS_ON_CTXSW patch.

Once we have the context switching situation sorted for VIVT we can then
reconsider these patches - their pre-requisits will have been solved by
way of the VIVT situation being sorted.