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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id EED37C47DDF for ; Tue, 30 Jan 2024 13:07:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5E4016B0080; Tue, 30 Jan 2024 08:07:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 56EAF6B0082; Tue, 30 Jan 2024 08:07:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E7356B0085; Tue, 30 Jan 2024 08:07:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 29EAC6B0080 for ; Tue, 30 Jan 2024 08:07:29 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id C1E12140AF6 for ; Tue, 30 Jan 2024 13:07:28 +0000 (UTC) X-FDA: 81736003776.16.8D9F2D8 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf21.hostedemail.com (Postfix) with ESMTP id 1DEE21C001F for ; Tue, 30 Jan 2024 13:07:25 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf21.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706620046; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/9gO6r1nlR9TClAdhf6DEhhtE3vltwVttojRzTatWgg=; b=MBGfsSb8BwTiT28R0JdAqRzGDNa1fGLzuHefc/BLygC/Lo+irZQ42Y/ISfTd5j74+846Hi XrK2mYR17M8ZU+AGtmYi9kulAXK8k51gTzm6Qjba4vBBNyngpwrHc/DvAjT6MPlYCRbfy0 l2B8it4iMNdolOsu/XHMRUx2vS5jUBY= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf21.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706620046; a=rsa-sha256; cv=none; b=8jJgguUjCVK3U7/OueB4eEGbJa4xCjIZjjYiNgz2adCyWOEutt0WbYDcVjNMVSHbEReghF 8hzC6/kdXe8EBAZyMFSx6MndZ86PFvnzITCvJBcypHC4PG7oJyhA8Hnu2hJTVr2xccUjT7 FA5Nav71B8tzDigizxUrV6iyLcSs+Lw= 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 80839DA7; Tue, 30 Jan 2024 05:08:08 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.48.92]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9D8453F762; Tue, 30 Jan 2024 05:07:20 -0800 (PST) Date: Tue, 30 Jan 2024 13:07:17 +0000 From: Mark Rutland To: Tong Tiangen Cc: Catalin Marinas , Will Deacon , James Morse , Robin Murphy , Andrey Ryabinin , Alexander Potapenko , Alexander Viro , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Michael Ellerman , Nicholas Piggin , Christophe Leroy , "Aneesh Kumar K.V" , "Naveen N. Rao" , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, wangkefeng.wang@huawei.com, Guohanjun Subject: Re: [PATCH v10 2/6] arm64: add support for machine check error safe Message-ID: References: <20240129134652.4004931-1-tongtiangen@huawei.com> <20240129134652.4004931-3-tongtiangen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1DEE21C001F X-Stat-Signature: bf7awcdp5neap3so8ef5j4b9c7ioo6pt X-Rspam-User: X-HE-Tag: 1706620045-570519 X-HE-Meta: U2FsdGVkX19mAk5Ed6CJPVzusoxDpAtQXc3l6kF17iV29vQhY5aREfJWeyEDVpgw+qALgwZvK27x8vgL2QxD401GyGZsP91b/shog+rQSAe1lgeG4auiPeCNQ6j/G4NTKfAUnsrghrNnuhxum9usxHmgim08QQY17sF5WB0pJv03GemzrUPdj0EYVFLDSmhClG4KI5vMQYjfaCDXhRI/7NZUkWn5ZYIY3XZG8+KfSU3wm+c3kBPYJFbEZPoV1U9p87F+gFp91Fz1K4zLLHvy8JeLHObQvbOra8NEuBGETaemN1tdhiF3n9BfRw7ZfvJSyKAAUPCNCjXZKuqOffAj6fT/nzzRbeAQ+yYOAC0HXMCiOWMlAi7Q/Jt3Y8WA5cHlJmQDg5KTOf4lYd9LSVrug6ifvsOkxcmztV6VwzLHUzxgz45kOD9LE0IyZZ3gSDJbB2TwV9Z1c85zpLjWs5stkQ9lqMHaWaPC8xzOVXy50dVNYMf+w2vfU+J+ss/GlUdwEriwyC9z54GCr2cofZ3Pz2iPs6qoeEkisJttwRyZdeIwieL5nam/SoBPcoPl/eOqvoUlh+61sperusuc+YmY0xl5GjRKJXg8B+ajGQlpnDqcdBEZc1cVGWx42/udqshhOX083jCMvny8sC5hhtQ97W92+vXyyoU7BCrEz8ysp2T31PjsrEHe0cc/evaYe9mSjRsVf10CDG0FZVYS9J2WHhIy5J7LdR10WPi1uhaSmDCFgCLIdb3vf7BItrSUfKC5OE/gDEBWu3/Xe0lDlx4uCAPzARYfuaN2uJHf3EqkNGvx/qAqyLKeLcuTDDNhd8YmWCsdBlUYNlyNbqSd6VROQq0OoJCkCGCMePTve1vqVef7MtXI4IOW+ml2jSu/6kOps55pY1C+d+ZhLM87FohbtEjRmqFgZAcluBsl3pB4kWByT1uQ0f5gGJOxq0Th/MFq7hD1xlfoLDwnRT6F/90 Av826PZM V+E2yRCX8nhReWmSecCofhAi3qgmdmqlz8V8zXU30JdxqCH+MIlLIDVH7JQ1PyzZ4H8oNgTjF/h6fs2kyEpcXUmPl5y//TLxZzX3iF3+LBSySww2+dZHq/O4Sohm5TwpWiWvmtd5N45D9t+Nzdrl9HiGRB2vkVggPG5OowOJlLsq6zuV/l3ncLnFl9pVUX5GJfWFUpFpmreaA+7U= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Jan 30, 2024 at 06:57:24PM +0800, Tong Tiangen wrote: > 在 2024/1/30 1:51, Mark Rutland 写道: > > On Mon, Jan 29, 2024 at 09:46:48PM +0800, Tong Tiangen wrote: > > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > > > index 55f6455a8284..312932dc100b 100644 > > > --- a/arch/arm64/mm/fault.c > > > +++ b/arch/arm64/mm/fault.c > > > @@ -730,6 +730,31 @@ static int do_bad(unsigned long far, unsigned long esr, struct pt_regs *regs) > > > return 1; /* "fault" */ > > > } > > > +static bool arm64_do_kernel_sea(unsigned long addr, unsigned int esr, > > > + struct pt_regs *regs, int sig, int code) > > > +{ > > > + if (!IS_ENABLED(CONFIG_ARCH_HAS_COPY_MC)) > > > + return false; > > > + > > > + if (user_mode(regs)) > > > + return false; > > > > This function is called "arm64_do_kernel_sea"; surely the caller should *never* > > call this for a SEA taken from user mode? > > In do_sea(), the processing logic is as follows: > do_sea() > { > [...] > if (user_mode(regs) && apei_claim_sea(regs) == 0) { > return 0; > } > [...] > //[1] > if (!arm64_do_kernel_sea()) { > arm64_notify_die(); > } > } > > [1] user_mode() is still possible to go here,If user_mode() goes here, > it indicates that the impact caused by the memory error cannot be > processed correctly by apei_claim_sea(). > > > In this case, only arm64_notify_die() can be used, This also maintains > the original logic of user_mode()'s processing. My point is that either: (a) The name means that this should *only* be called for SEAs from a kernel context, and the caller should be responsible for ensuring that. (b) The name is misleading, and the 'kernel' part should be removed from the name. I prefer (a), and if you head down that route it's clear that you can get rid of a bunch of redundant logic and remove the need for do_kernel_sea(), anyway, e.g. | static int do_sea(unsigned long far, unsigned long esr, struct pt_regs *regs) | { | const struct fault_info *inf = esr_to_fault_info(esr); | bool claimed = apei_claim_sea(regs) == 0; | unsigned long siaddr; | | if (claimed) { | if (user_mode(regs)) { | /* | * APEI claimed this as a firmware-first notification. | * Some processing deferred to task_work before ret_to_user(). | */ | return 0; | } else { | /* | * TODO: explain why this is correct. | */ | if ((current->flags & PF_KTHREAD) && | fixup_exception_mc(regs)) | return 0; | } | } | | if (esr & ESR_ELx_FnV) { | siaddr = 0; | } else { | /* | * The architecture specifies that the tag bits of FAR_EL1 are | * UNKNOWN for synchronous external aborts. Mask them out now | * so that userspace doesn't see them. | */ | siaddr = untagged_addr(far); | } | arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr); | | return 0; | } > > > + > > > + if (apei_claim_sea(regs) < 0) > > > + return false; > > > + > > > + if (!fixup_exception_mc(regs)) > > > + return false; > > > + > > > + if (current->flags & PF_KTHREAD) > > > + return true; > > > > I think this needs a comment; why do we allow kthreads to go on, yet kill user > > threads? What about helper threads (e.g. for io_uring)? > > If a memroy error occurs in the kernel thread, the problem is more > serious than that of the user thread. As a result, related kernel > functions, such as khugepaged, cannot run properly. kernel panic should > be a better choice at this time. > > Therefore, the processing scope of this framework is limited to the user > thread. That's reasonable, but needs to be explained in a comment. Also, as above, I think you haven't conisderd helper threads (e.g. io_uring), which don't have PF_KTHREAD set but do have PF_USER_WORKER set. I suspect those need the same treatment as kthreads. > > > + set_thread_esr(0, esr); > > > > Why do we set the ESR to 0? > > The purpose is to reuse the logic of arm64_notify_die() and set the > following parameters before sending signals to users: > current->thread.fault_address = 0; > current->thread.fault_code = err; Ok, but there's no need to open-code that. As per my above example, please continue to use the existing call to arm64_notify_die() rather than open-coding bits of it. Mark.