From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 11524402B8F for ; Mon, 3 Aug 2026 11:34:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785756883; cv=none; b=cqkkFfH0s39KaYNbxVTMl4zDlYWL20TM+SS+Ytqe05+xSzBHpJV2Bh6f7W6YEFpD7PXolGJpduwjRkcrGvcR/6iiFBLm0zKJx/L4aTf0zUkwz5bZcSXymr034UNg+z8b238C1jRCqllnBIFxbocCDLMr6g3x410YxLRiJb/ChAA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785756883; c=relaxed/simple; bh=hvUjH62FMs4eWRYreksi0y6ODe1YRla6sWjuNOUTH44=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=dlBtydyLPZd0hy6FW8jZbUh3b8TjEsHOz9lWMcmEX+2NInrRUDGQbkuG+KxQoudgwEVjiGU1p+EyixsAKMgbEFvYMmc0UEVQlJBINRIbA4a5MtgNAB9rHRDFaBZcMZXbto3dHgyIc4gzqeXVZ5mnAkE9xnId3iC5XPOX+TkSF20= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=i0BEASft; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="i0BEASft" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-2ccdf36f63dso130375ad.0 for ; Mon, 03 Aug 2026 04:34:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1785756881; x=1786361681; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=2wni3Q+wShyY65f0uKlTNx2h4S9tj48PjL0JpbjcW/Q=; b=i0BEASftTF0eIJOml4u4mgVzwjzn1O02lCCOkEO40LBLve7YRZ9E2ZZY6BHg1TQ5XG dsqrqDPxK3TFMuWJBdqVstoZ0+H8W3ae+N46fbjwHgWKIZVUBmKkPMzdI83HJQxFGR8E 2l4RaD9nvX06TECOd+sw4beZJI0Bsz6UhyiR/A3Vh8o9WWX/7+bsGNsLa7URXZtsvKvZ EfogU4160PF2wbz1g+umA/tkvCz+/xv1jJl9Xw5ZWy75hSKS/bQjnRMiy5DxrNa03ITX Mv4f3p4t+qDQHcYxPwkNPpPJyz2dkFedhDq2yfHttI/kPRKy6SzImRniwUC0A5quNkrH O5aQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785756881; x=1786361681; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=2wni3Q+wShyY65f0uKlTNx2h4S9tj48PjL0JpbjcW/Q=; b=OlhtcWWTtP4FNqgpAAyytkpp9kUjqq8PP6x13dKyvYeeVwdBlw5XpnQMkUsEOnIGtj Uf08N+omDZHOhCT0hVmKZjWH3xxsS7KHxPgm+HbP1M6C0+wg5ttcdTbJKTagu4H9nr13 pX2pxUfNWHRmL13LsZ1BXO+C3uY8wBdHtyXhs7HLXQECh4S0BWjXfylffF6S9wTmZBYX 6lXTzlYy3eJkcaj5NqK0kM5TKcSLwl+KU/6ljiw6uP3AaOpLEg25DMyNx0in8Qq9Klfr h/xcoBK2uNzoLAdD92RecEv8YWYX1a+DrvJgYDIyws359xeS+pkZEbLSwuZV1+fZ9sV/ cEjg== X-Forwarded-Encrypted: i=1; AHgh+RrXMZ10Q4vZ6nZ7DqE7xZ+7BcDBqO5fSM0gmewxFVyBPhHDcsdJnMYHnY5Dm/q+ICXHWdOHjsHAgRggmhI=@vger.kernel.org X-Gm-Message-State: AOJu0YwnLVaQ2muMWlWXpNDEbx2lJKfCtVgoWPUZY9hAq4RqRCZXlb+k RnnIY3n4knxcjPqPyylimyuYt6y9m1K0ud/0YMsC9XTKto6Nxhne0iZAwCZB54+CrA== X-Gm-Gg: AR+sD12xk8nmRrp9WCRNqg0+jWXcbGgTYFGb0Qj1CwqJ1ygZgOObsqj44ZNoSAlZ9MA ICCJVcassDzmap/hQ3Y08LNUZA7AOtwC9O8x4TXn+00WEX6ruO3Sxm44f6bV1sG4DthWmr6GINN ezsio0sUFEmtU/hKcnVF48l/8HhEAcFHafByk86EL+huSZVhk9iSeH/pQwzYmpccB6nPq/ZknNb E2hs86HnHqtWB8df0d//6s7Nkte9hrhmOijn2N8+O5Xiu0cMJxCJbltSQpvhAr0GV7GrGA3q+Zz g2RO+8io4lW4sCJW7ezguVRNs4CaNX+CoCNsF4fUGnWoUf3gfmNcAV/ZM3jq45ZfxRnnBLeYGRC O0zSxKJ3sRK66Yh/HlU4dJ1E+Jsz3lGtORsNjwgGiEhxpUanFlGPRkQ82dpNL/x/yNRZS6MaBkQ PicVKL5TJCMd9ZTuh79X/eRSpIh5yENxNRCqKxjZWp5QMxBJvq3xIWwnRB1/9fOx/A9PAzmTlAx FF7y/lR55FrfSo9HyoUCKk= X-Received: by 2002:a17:903:1aec:b0:2ca:cbe5:c3af with SMTP id d9443c01a7336-2d056f884a0mr11365075ad.7.1785756880556; Mon, 03 Aug 2026 04:34:40 -0700 (PDT) Received: from google.com (21.168.124.34.bc.googleusercontent.com. [34.124.168.21]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2d04ae67d0esm36522585ad.30.2026.08.03.04.34.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Aug 2026 04:34:40 -0700 (PDT) Date: Mon, 3 Aug 2026 11:34:33 +0000 From: Pranjal Shrivastava To: Prakash Gupta Cc: Will Deacon , Robin Murphy , Joerg Roedel , Rob Clark , Connor Abbott , linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, Akhil P Oommen , Pratyush Brahma Subject: Re: [PATCH v3] iommu/arm-smmu: Use pm_runtime in fault handlers Message-ID: References: <20260630-smmu-rpm-v3-1-f69874a580fa@oss.qualcomm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260630-smmu-rpm-v3-1-f69874a580fa@oss.qualcomm.com> On Tue, Jun 30, 2026 at 02:32:46PM +0530, Prakash Gupta wrote: Hi Prakash, On a second thought, I'd like to discuss one more thing > Commit d4a44f0750bb ("iommu/arm-smmu: Invoke pm_runtime across the driver") > enabled pm_runtime for the arm-smmu device. On systems where the SMMU > sits in a power domain, all register accesses must be done while the > device is runtime active to avoid unclocked register reads and > potential NoC errors. > > So far, this has not been an issue for most SMMU clients because > stall-on-fault is enabled by default. While a translation fault is > being handled, the SMMU stalls further translations for that context > bank, so the fault handler would not race with a powered-down SMMU. > > Adreno SMMU now disables stall-on-fault in the presence of fault > storms to avoid saturating SMMU resources and hanging the GMU. With > stall-on-fault disabled, the SMMU can generate faults while its power > domain may no longer be enabled, which makes unclocked accesses to > fault-status registers in the SMMU fault handlers possible. > > Guard the context and global fault handlers with > arm_smmu_rpm_get_if_active() and arm_smmu_rpm_put() so that all SMMU > fault register accesses are done with the SMMU powered. If the SMMU is > not runtime active, the fault can be safely ignored as > arm_smmu_device_reset() clears fault registers on resume. > > Additionally, disable fault reporting in arm_smmu_runtime_suspend() > before powering down. pm_runtime_get_if_active() returns 0 during > RPM_SUSPENDING, so without this, level-triggered fault interrupts would > cause an interrupt storm while the device is being suspended. > arm_smmu_device_reset() re-enables fault reporting on resume. > > Fixes: b13044092c1e ("drm/msm: Temporarily disable stall-on-fault after a page fault") > Co-developed-by: Pratyush Brahma > Signed-off-by: Pratyush Brahma > Signed-off-by: Prakash Gupta > @@ -2306,6 +2329,25 @@ static int __maybe_unused arm_smmu_runtime_resume(struct device *dev) [...] I believe, there is a small race condition in the suspend path that can lead to unclocked register access crashes. (Something similar to what I've attempted to handle in arm-smmu-v3 [1]) In arm_smmu_runtime_suspend(), we disable interrupt reporting in sCR0 and SCTLR, and then immediately call clk_bulk_disable(). This disables the interrupt generation but what about the interrupt handlers running *during* suspend? I believe we could have this race: CPU 0 (Suspend Context) CPU 1 (Interrupt/ISR Context) ----------------------- ----------------------------- 1. arm_smmu_context_fault() starts. 2. rpm_get_if_active() returns 1. (Clocks are ON) 3. arm_smmu_runtime_suspend() - Clears CFIE/GFIE in registers (stops new IRQs from firing) 4. clk_bulk_disable() (Clocks are CUT) 5. Attempts MMIO access (e.g, to clear CB_FSR or CB_RESUME). --> [CRASH] Unclocked MMIO access I believe similar to arm-smmu-v3 [1], we must call synchronize_irq() on context interrupts after disabling them in the SCTLR but before we cut the clocks. This forces CPU 0's suspend thread to sleep and wait for any active ISRs to safely drain while the SMMU still has clocks. We can simply add this loop to arm_smmu_runtime_suspend(): > static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev) > { > struct arm_smmu_device *smmu = dev_get_drvdata(dev); > + int i; > + u32 reg; > + > + /* > + * Disable fault reporting before powering down to prevent unclocked > + * register accesses in the fault handlers if an interrupt races with > + * the suspend callback (e.g. device in RPM_SUSPENDING state). > + * arm_smmu_device_reset() re-enables fault reporting on resume. > + */ > + reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0); > + reg &= ~(ARM_SMMU_sCR0_GFRE | ARM_SMMU_sCR0_GFIE | > + ARM_SMMU_sCR0_GCFGFRE | ARM_SMMU_sCR0_GCFGFIE); > + arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sCR0, reg); > + > + for (i = 0; i < smmu->num_context_banks; i++) { > + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_SCTLR); > + reg &= ~(ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE); > + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_SCTLR, reg); > + } for (i = 0; i < smmu->num_context_irqs; i++) synchronize_irq(smmu->irqs[i]); > > clk_bulk_disable(smmu->num_clks, smmu->clks); > Since we're disabling those interrupts and fixing concurrency, this seems like the perfect opportunity to add the sync_irq too.. What do you think? With that loop added: Reviewed-by: Pranjal Shrivastava Thanks, Praan [1] https://lore.kernel.org/all/20260728210928.1050849-10-praan@google.com/