From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A974C64E7B for ; Wed, 2 Dec 2020 14:02:50 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 33799208FE for ; Wed, 2 Dec 2020 14:02:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 33799208FE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=wtI208d2xRYTAxU13R5uMIpVTNFvBumRmyO+sKBIOIg=; b=zRZxueY53+7cE/8Hc7iQsRMu+ Uep1EgFXC8Z9zYRWXm/lGUS3hzTS8m3VcNjQnBIRrndb4OcBMqnXkeJ6EfL6R67uWZ34q35VC3pZH UAhBYECOfoTNkyPd2Mwj2AmZBFTznpqLKAhvUEmHBR1AE/DOh6cWUK3QacrjVXyKa8G4rmRv6fntC 7FFIZ2BMV6H48HfqTj2NtiS+hClWnDW8wC8vQ03vpkPukX/NxQneFd9am6TbuvBFrT+8/1UAQKEfr toF2EuoQA79vP021t5TGwN1/HNjzIObF5XaFRbuH5DrWcOpmnKeXp8qCj3MyCyjrO7ffKtnn7Nf33 g1HexgI2g==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kkSg2-0005ap-8N; Wed, 02 Dec 2020 14:00:14 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kkSfy-0005ZK-Cd for linux-arm-kernel@lists.infradead.org; Wed, 02 Dec 2020 14:00:12 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 397B530E; Wed, 2 Dec 2020 06:00:09 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.23.201]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C523E3F718; Wed, 2 Dec 2020 06:00:04 -0800 (PST) Date: Wed, 2 Dec 2020 13:59:57 +0000 From: Mark Rutland To: Alex Belits Subject: Re: [PATCH v5 6/9] task_isolation: arch/arm64: enable task isolation functionality Message-ID: <20201202135957.GA66958@C02TD0UTHF1T.local> References: <8d887e59ca713726f4fcb25a316e1e932b02823e.camel@marvell.com> <91496c0cf8d24717a2641fc4d02063f3f10dc733.camel@marvell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <91496c0cf8d24717a2641fc4d02063f3f10dc733.camel@marvell.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201202_090010_619514_8196AF80 X-CRM114-Status: GOOD ( 28.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-arch@vger.kernel.org" , "nitesh@redhat.com" , "pauld@redhat.com" , "catalin.marinas@arm.com" , "peterz@infradead.org" , "linux-api@vger.kernel.org" , "frederic@kernel.org" , "mtosatti@redhat.com" , "linux-kernel@vger.kernel.org" , "rostedt@goodmis.org" , "mingo@kernel.org" , "leon@sidebranch.com" , "netdev@vger.kernel.org" , "peterx@redhat.com" , "trix@redhat.com" , Prasun Kapoor , "tglx@linutronix.de" , "will@kernel.org" , "davem@davemloft.net" , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Alex, On Mon, Nov 23, 2020 at 05:58:06PM +0000, Alex Belits wrote: > In do_notify_resume(), call task_isolation_before_pending_work_check() > first, to report isolation breaking, then after handling all pending > work, call task_isolation_start() for TIF_TASK_ISOLATION tasks. > > Add _TIF_TASK_ISOLATION to _TIF_WORK_MASK, and _TIF_SYSCALL_WORK, > define local NOTIFY_RESUME_LOOP_FLAGS to check in the loop, since we > don't clear _TIF_TASK_ISOLATION in the loop. > > Early kernel entry code calls task_isolation_kernel_enter(). In > particular: > > Vectors: > el1_sync -> el1_sync_handler() -> task_isolation_kernel_enter() > el1_irq -> asm_nmi_enter(), handle_arch_irq() > el1_error -> do_serror() > el0_sync -> el0_sync_handler() > el0_irq -> handle_arch_irq() > el0_error -> do_serror() > el0_sync_compat -> el0_sync_compat_handler() > el0_irq_compat -> handle_arch_irq() > el0_error_compat -> do_serror() > > SDEI entry: > __sdei_asm_handler -> __sdei_handler() -> nmi_enter() As a heads-up, the arm64 entry code is changing, as we found that our lockdep, RCU, and context-tracking management wasn't quite right. I have a series of patches: https://lore.kernel.org/r/20201130115950.22492-1-mark.rutland@arm.com ... which are queued in the arm64 for-next/fixes branch. I intend to have some further rework ready for the next cycle. I'd appreciate if you could Cc me on any patches altering the arm64 entry code, as I have a vested interest. That was quite obviously broken if PROVE_LOCKING and NO_HZ_FULL were chosen and context tracking was in use (e.g. with CONTEXT_TRACKING_FORCE), so I'm assuming that this series has not been tested in that configuration. What sort of testing has this seen? It would be very helpful for the next posting if you could provide any instructions on how to test this series (e.g. with pointers to any test suite that you have), since it's very easy to introduce subtle breakage in this area without realising it. > > Functions called from there: > asm_nmi_enter() -> nmi_enter() -> task_isolation_kernel_enter() > asm_nmi_exit() -> nmi_exit() -> task_isolation_kernel_return() > > Handlers: > do_serror() -> nmi_enter() -> task_isolation_kernel_enter() > or task_isolation_kernel_enter() > el1_sync_handler() -> task_isolation_kernel_enter() > el0_sync_handler() -> task_isolation_kernel_enter() > el0_sync_compat_handler() -> task_isolation_kernel_enter() > > handle_arch_irq() is irqchip-specific, most call handle_domain_irq() > There is a separate patch for irqchips that do not follow this rule. > > handle_domain_irq() -> task_isolation_kernel_enter() > do_handle_IPI() -> task_isolation_kernel_enter() (may be redundant) > nmi_enter() -> task_isolation_kernel_enter() The IRQ cases look very odd to me. With the rework I've just done for arm64, we'll do the regular context tracking accounting before we ever get into handle_domain_irq() or similar, so I suspect that's not necessary at all? > > Signed-off-by: Chris Metcalf > [abelits@marvell.com: simplified to match kernel 5.10] > Signed-off-by: Alex Belits > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/barrier.h | 1 + > arch/arm64/include/asm/thread_info.h | 7 +++++-- > arch/arm64/kernel/entry-common.c | 7 +++++++ > arch/arm64/kernel/ptrace.c | 10 ++++++++++ > arch/arm64/kernel/signal.c | 13 ++++++++++++- > arch/arm64/kernel/smp.c | 3 +++ > 7 files changed, 39 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 1515f6f153a0..fc958d8d8945 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -141,6 +141,7 @@ config ARM64 > select HAVE_ARCH_PREL32_RELOCATIONS > select HAVE_ARCH_SECCOMP_FILTER > select HAVE_ARCH_STACKLEAK > + select HAVE_ARCH_TASK_ISOLATION > select HAVE_ARCH_THREAD_STRUCT_WHITELIST > select HAVE_ARCH_TRACEHOOK > select HAVE_ARCH_TRANSPARENT_HUGEPAGE > diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h > index c3009b0e5239..ad5a6dd380cf 100644 > --- a/arch/arm64/include/asm/barrier.h > +++ b/arch/arm64/include/asm/barrier.h > @@ -49,6 +49,7 @@ > #define dma_rmb() dmb(oshld) > #define dma_wmb() dmb(oshst) > > +#define instr_sync() isb() I think I've asked on prior versions of the patchset, but what is this for? Where is it going to be used, and what is the expected semantics? I'm wary of exposing this outside of arch code because there aren't strong cross-architectural semantics, and at the least this requires some documentation. If it's unused, please delete it. [...] > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index 43d4c329775f..8152760de683 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -77,6 +78,8 @@ asmlinkage void notrace el1_sync_handler(struct pt_regs *regs) > { > unsigned long esr = read_sysreg(esr_el1); > > + task_isolation_kernel_enter(); For regular context tracking we only acount the user<->kernel transitions. This is a kernel->kernel transition, so surely this is not necessary? If nothing else, it doesn't feel well-balanced. I havwe not looked at the rest of this patch (or series) in detail. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel