linux-kernel - Re: [PATCH] PREEMPT_RT_FULL: ARM context switch needs IRQs enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111216110100.GA6342@arm.com>
Date:	Fri, 16 Dec 2011 11:01:00 +0000
From:	Catalin Marinas <catalin.marinas@....com>
To:	Frank Rowand <frank.rowand@...sony.com>
Cc:	"tglx@...utronix.de" <tglx@...utronix.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"rostedt@...dmis.org" <rostedt@...dmis.org>
Subject: Re: [PATCH] PREEMPT_RT_FULL: ARM context switch needs IRQs enabled

Hi Frank,

On Fri, Dec 16, 2011 at 03:20:45AM +0000, Frank Rowand wrote:
> ARMv6 and later have VIPT caches and the TLBs are tagged with an ASID
> (application specific ID). The number of ASIDs is limited to 256 and
> the allocation algorithm requires IPIs when all the ASIDs have been
> used.  The IPIs require interrupts enabled during context switch for
> deadlock avoidance.
> 
> The RT patch mm-protect-activate-switch-mm.patch disables irqs around
> activate_mm() and switch_mm(), which are the portion of the ARMv6
> context switch that require interrupts enabled.
> 
> The solution for the ARMv6 processors could be to _not_ disable irqs.
> A more conservative solution is to provide the same environment that
> the scheduler provides, that is preempt_disable().  This is more
> resilient for possible future changes to the ARM context switch code
> that is not aware of the RT patches.
> 
> This patch will conflict slightly with Catalin's patch set to remove
> __ARCH_WANT_INTERRUPTS_ON_CTXSW, when that is accepted:
> 
>    http://lkml.indiana.edu/hypermail/linux/kernel/1111.3/01893.html
> 
> When Catalin's patch set is accepted, this RT patch will need to reverse
> the change in patch 6 to arch/arm/include/asm/system.h:
> 
>    -#ifndef CONFIG_CPU_HAS_ASID
>    -#define __ARCH_WANT_INTERRUPTS_ON_CTXSW
>    -#endif
> 
> Signed-off-by: Frank Rowand <frank.rowand@...sony.com>

The whole point of my patches was to no longer define
__ARCH_WANT_INTERRUPTS_ON_CTXSW on ARM, so bringing it back in is not
feasible.

But by reading your patch I realised that there are two more switch_mm()
calls outside the scheduler switch and the post-switch hook would not be
called:

1. idle_task_exit() - this function switches to the init_mm. With my new
patches, even if the switch does not explicitly happen because it
requires a new ASID, the ARMv6+ code sets the init_mm.pgd anyway. On
ARMv5 such hook would not happen but we don't have CONFIG_HOTPLUG_CPU
either. But even if it happens to work, it's not correct behaviour in
either case.

2. use_mm() - this also assumes an immediate switch_mm() which may not
happen.

My question - can we not use activate_mm() directly in both cases? This
function would ensure that the new mm is actually active on ARM and it
does not disable the interrupts for the critical code. You may still
need some RT patch if the RT series disables the interrupts for the
switch_mm() calls.

If that sounds reasonable, I'll repost my series with activate_mm() fix.

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/