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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 38B13C83F25 for ; Thu, 29 Aug 2024 13:49:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=fij7bmUZ1BwtygZobmPkPFaBK/SFCbQ/kLne1FVouMA=; b=vbqEb3vOzADLeg/6km+FIIsFtq qs58f/l7yNflGD418/hyFIUjUD/XbDIIN4uQHnZ4XYFuboDQF443X3Conm0UIsJnrC4Aw05PDQmLV MTp3CBWo9SaMP/OuiF2+bSWOOvzdZb+bMwZ3FMFOazDvMX72z5Iv+8mPZFEup0AlgntVdXxFjIJd9 Fl0PbQ1/xscqPrfBPxOkLJB/4smsn03u4GzU6cZ7YmVUUyiS6BAwcTmRK/754g2PpvImbiDrRp6xp 5+s+LDZwVqasMRGV6xKEnT+DjkfQ0VkL2gFEP1dg4ivDTwwFzcGr3lPahPRM3eiDU1ltJPd9WV1FI 4Y8nAiMw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sjfWl-00000002GFS-1Y9r; Thu, 29 Aug 2024 13:49:31 +0000 Received: from mail-oo1-xc30.google.com ([2607:f8b0:4864:20::c30]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sjfVY-00000002Fv7-26oZ for linux-arm-kernel@lists.infradead.org; Thu, 29 Aug 2024 13:48:18 +0000 Received: by mail-oo1-xc30.google.com with SMTP id 006d021491bc7-5de92d2e9b3so370474eaf.2 for ; Thu, 29 Aug 2024 06:48:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724939295; x=1725544095; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=fij7bmUZ1BwtygZobmPkPFaBK/SFCbQ/kLne1FVouMA=; b=JNNQBhGuRiN9QGnZRMVxzJbOU0XUqBqopdrzbcaU3dIP74Z2kRhzwCqrD6b/jDLPsn cWXWtcaacTHHWawWkraSL3ySmGnbRjKnot+GZ0/pj7czbryxXRLUKl1TaMdxZK1Jzc+X +/iwao/ZC/NlhDSgur5FsQxDrF5eFCYJMLe0AKV/0SsV57wzrmspehV24BrqNu8jm9DF 23H2PDf4u8wz807lOHD33nKb1HKmzDCnlCHLuumubWzq9sL62cG2OzCjEuxCN8wAxErU bFFG6WdmCr31wQhXSNHqmHsXs2cGMDw8V3ebrkpfORKCmkLHhum5J++1swWaDeK/QeHi c4vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724939295; x=1725544095; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fij7bmUZ1BwtygZobmPkPFaBK/SFCbQ/kLne1FVouMA=; b=kqMoVR81ycSxow5bNxr9Kh/ncUuYYO//lUuiS5bIxt6pxLXpcvpVzRzTqfWlg2/Qp/ puVHJ3+PPYFelHF7TUngO5kFomSYZHRW5CxuuXy36ASQ9NsC0nhkNsAlFJi2dgaJaGcV SpnM1k5f7kGIo4uLIxFrQuVymWzK/1PD1pqc7NMB35lYybDtmoaHCuBFFHIoe3bon6Pn Jukv02jx3+r2CgyZ5ydXi6EpFN9w7ql6uNuSZXwaj+QIp7ZobJdkpQgXS50yBFy6xOzh u2uEhTj8UtEFQOYJT+tu7xOQhlWmHkDENjYnQBzOKDxvKcnGdu8nA8J3DNmNg1s9Dg+Y 0zxA== X-Forwarded-Encrypted: i=1; AJvYcCUDesIPDd3/HmpYgbJSN7MLq6T3ZjFUFLKoGa55TZKwGvIAPVrVNao1SGSVMGPPaH1wKRbydZhzfvmXU1QlgMdN@lists.infradead.org X-Gm-Message-State: AOJu0Yz1Bj870iD1ehZqxYg7CQPqcWK2zuiJtxpzlIN3uYgVMCje3GVq 6tVXEWLxB7Je0i1l7nf6dnDhvoh0nW8cC6r6ftnaCrePh9naS23DeoESDjg/SQA= X-Google-Smtp-Source: AGHT+IGCpp26qT5ACYfhkp4StVnu7Amwf484YTNlNSrglEKytGqqjoOG91Qqwhfb8r2kOPo+eK2/Zw== X-Received: by 2002:a05:6359:4c10:b0:1a5:6b71:dde6 with SMTP id e5c5f4694b2df-1b603bebb88mr396283355d.3.1724939294686; Thu, 29 Aug 2024 06:48:14 -0700 (PDT) Received: from [192.168.0.122] ([59.188.211.160]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7d22e66d8f4sm1236270a12.0.2024.08.29.06.48.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 29 Aug 2024 06:48:14 -0700 (PDT) Message-ID: <18007fe0-cc5d-4887-ab41-ed4cf8217b60@gmail.com> Date: Thu, 29 Aug 2024 21:48:10 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true Content-Language: en-MW To: Thomas Gleixner , Hector Martin , Sven Peter , Alyssa Rosenzweig , Rob Herring , Krzysztof Kozlowski , Conor Dooley , asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Cc: ~postmarketos/upstreaming@lists.sr.ht, Konrad Dybcio References: <20240829110436.46052-1-towinchenmi@gmail.com> <20240829110436.46052-3-towinchenmi@gmail.com> <87zfova32u.ffs@tglx> From: Nick Chan In-Reply-To: <87zfova32u.ffs@tglx> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240829_064816_582236_13E96C15 X-CRM114-Status: GOOD ( 27.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 29/8/2024 21:08, Thomas Gleixner wrote: > On Thu, Aug 29 2024 at 19:02, Nick Chan wrote: >> Starting from the A11 (T8015) SoC, Apple introuced system registers for >> fast IPI and UNCORE PMC control. These sysregs do not exist on earlier >> A7-A10 SoCs and trying to access them results in an instant crash. >> >> Restrict sysreg access within the AIC driver to configurations where >> use_fast_ipi is true to allow AIC to function properly on A7-A10 SoCs. > > use_fast_ipi is an implementation detail and does not mean anything > here. It's sufficient to say: > > Only access system registers on SoCs which provide them. > > Hmm? Seems like a good idea. Will be in v2. Though if the commit message is that generic, do you think it's correct to squash the third patch into this commit? It is also preventing sysreg access on SoC that do not provide them. > >> While at it, remove the IPI-always-ack path on aic_handle_fiq. > > It's not while at it. It's part of handling this correctly. In v2 it will be mentioned as as part of the sysreg access restriction. > >> If we are able to reach there, we are on an IPI-capable system and >> should be using one of the IPI-capable compatibles, anyway. > > 'we' can't reach that code ever. > > https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog Acked. In v2 the commit message will not impersonate the code anymore. > > >> Signed-off-by: Konrad Dybcio >> Signed-off-by: Nick Chan > > This Signed-off-by chain is invalid. If Konrad authored the patch then > you need to have a 'From: Konrad ...' line at the top of the change log. > > If you worked together on this then this needs a Co-developed-by > tag. See Documentation/process/... This patch was entirely made by Konrad, but now that changes are requested, it will be a Co-developed-by in v2. > >> >> - if (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING) { >> - if (static_branch_likely(&use_fast_ipi)) { >> - aic_handle_ipi(regs); >> - } else { >> - pr_err_ratelimited("Fast IPI fired. Acking.\n"); >> - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1); >> - } >> + if (static_branch_likely(&use_fast_ipi) && >> + (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING)) { >> + aic_handle_ipi(regs); >> } >> >> if (TIMER_FIRING(read_sysreg(cntp_ctl_el0))) >> @@ -574,8 +571,9 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs) >> AIC_FIQ_HWIRQ(irq)); >> } >> >> - if (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ && >> - (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) { >> + if (static_branch_likely(&use_fast_ipi) && >> + (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ) && >> + (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) { >> /* Same story with uncore PMCs */ >> pr_err_ratelimited("Uncore PMC FIQ fired. Masking.\n"); >> sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE, >> @@ -811,7 +809,8 @@ static int aic_init_cpu(unsigned int cpu) >> /* Mask all hard-wired per-CPU IRQ/FIQ sources */ >> >> /* Pending Fast IPI FIQs */ >> - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1); >> + if (static_branch_likely(&use_fast_ipi)) >> + write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1); >> >> /* Timer FIQs */ >> sysreg_clear_set(cntp_ctl_el0, 0, ARCH_TIMER_CTRL_IT_MASK); >> @@ -832,8 +831,9 @@ static int aic_init_cpu(unsigned int cpu) >> FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_OFF)); >> >> /* Uncore PMC FIQ */ >> - sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE, >> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF)); >> + if (static_branch_likely(&use_fast_ipi)) >> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE, >> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF)); > > Please see the bracket rules in the tip maintainers doc. Acked. Brackets will be added to function references in commit message of v2. > > Thanks, > > tglx Nick Chan