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=-4.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 F0068C48BE0 for ; Fri, 11 Jun 2021 11:34:11 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id B48E0613CC for ; Fri, 11 Jun 2021 11:34:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B48E0613CC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HM9ex5gdkWfvoEPZXgBjMJgV7vgTzP3BsI2y0vaZh/4=; b=Z5bp6QiiYPR/D/ vrRUgTe+2It/SnqL+441OyVFaEEVrmZhCiwE/U2GpLJTViiGRuy99hFeKmP3dfm2wgmh7ctY8Dt5f UudcvZp6eXAvYu5nyNpSpNX2c+VKr1AQtCLTWO4khW0DgBEwizqvby9dwembZjE+YwaZz/W9hvaRu 29Eli0VACmusiwmoYA2xU2VHztO+hQS6SO46hC4wfWWrmippFMp4scNcn/1GIwE5b2Cem4koGU0Ie Yw+zqxJkKmvhlMrYGOZuLDt9XJqgCAgn1nniakpbfPL8LzKjTkx18EsmDJjNVXKdJKRlN0ObJUhHn WavuxsCxPx3HkZ1mQp/A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lrfOx-0053FX-JA; Fri, 11 Jun 2021 11:32:39 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lrfOt-0053Eb-GW for linux-arm-kernel@lists.infradead.org; Fri, 11 Jun 2021 11:32:37 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 28035613CC; Fri, 11 Jun 2021 11:32:35 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1lrfOr-006xrn-3l; Fri, 11 Jun 2021 12:32:33 +0100 Date: Fri, 11 Jun 2021 12:32:32 +0100 Message-ID: <87bl8cr60f.wl-maz@kernel.org> From: Marc Zyngier To: Lorenzo Pieralisi Cc: linux-arm-kernel@lists.infradead.org, Will Deacon , Catalin Marinas , Mark Rutland , Valentin Schneider , Alexandru Elisei , Russell King , kernel-team@android.com Subject: Re: [PATCH 0/3] arm64: Fix cpuidle with pseudo-NMI enabled In-Reply-To: <20210611094134.GA2789@lpieralisi> References: <20210608172715.2396787-1-maz@kernel.org> <20210610162823.GA20025@lpieralisi> <87eed8reyd.wl-maz@kernel.org> <20210611094134.GA2789@lpieralisi> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: lorenzo.pieralisi@arm.com, linux-arm-kernel@lists.infradead.org, will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, valentin.schneider@arm.com, alexandru.elisei@arm.com, linux@armlinux.org.uk, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210611_043235_608609_7F5ADF6F X-CRM114-Status: GOOD ( 50.41 ) 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: , 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 On Fri, 11 Jun 2021 10:41:34 +0100, Lorenzo Pieralisi wrote: > > On Fri, Jun 11, 2021 at 09:19:22AM +0100, Marc Zyngier wrote: > > Hi Lorenzo, > > > > On Thu, 10 Jun 2021 17:28:23 +0100, > > Lorenzo Pieralisi wrote: > > > > > > On Tue, Jun 08, 2021 at 06:27:12PM +0100, Marc Zyngier wrote: > > > > It appears that although cpu_do_idle() is correctly dealing with the > > > > PMR/DAIF duality, the PSCI cpu-suspend code has been left unaware of > > > > it. > > > > > > > > On a system that uses PSCI for idle (such as the Ampere Altra I have > > > > access to), the kernel dies as soon as it enters idle (interrupts are > > > > off at the GIC CPU interface level). Boo. > > > > > > After investigating a bit I realised that this should depend on > > > ICC_CTLR_EL3.PMHE - if that's clear the PMR should not affect the > > > GICR->CPU IRQ forwarding (or WakeRequest signal generation when the > > > GICR_WAKER.ProcessorSleep==1). > > > > You lost me here. I don't see what PMHE has to do here. It is solely > > used for 1:N distribution, and is the only way PMR does affect the > > propagation of interrupts to the CPU interface. Fortunately, nobody > > uses 1:N. > > > > > IIUC if PMHE == 0, the PMR plays no role in wfi completion (and > > > WakeSignal generation for a CPU/GICR in quiescent state). > > > > Of course it does. PMR gates interrupts *before* they are signalled to > > the CPU, meaning that if you keep interrupt masked at the PMR level, > > you will never wake up from WFI. Or am I missing your point entirely? > > For "simple" wfi (as in executing the wfi instruction) yes. The > IRQs are forwarded to the CPU interface that filters the IRQs based > on priorities and signal the I/F "pin" so that the core wakes up > and wfi completes - forgive me my misunderstanding. No worries. We're talking about the GIC architecture here, which has the potential to confuse anyone! :D > For deep sleep states where GICR_WAKER.ProcessorSleep == 1, the > WakeSignal (ie CPU reset) is generated independently of the PMR > value AFAIK. This means that even *if* an IRQ is supposed to be > masked by the PMR it would wake up a sleeping core _anyway_. I'm not sure we can draw this conclusion. It certainly isn't the behaviour I'm observing. Otherwise, my system would be able to wake-up without any additional hacks. It may wake-up the CPU interface, but not the whole core. It is also completely possible that firmware will use the PMR value as a hint to decide which interrupts can wake up the CPU if it is so inclined. > This behaviour is different from shallow C-states (and simple wfi). > > That's why I asked what path is causing trouble in > psci_cpu_suspend_enter(). It is the one where we don't loose context. Which makes sense, as it would otherwise behave like a full reset, and we'd end-up with some sane values. > > > I assume on Ampere Altra PMHE == 1. > > > > No, it is 0, as indicated by: > > > > > > [ 0.000000] GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation > > > > > > > This changes almost nothing to the need for this patchset but > > > at least we clarify this behaviour. > > > > > > Also, we should not be writing ICC_PMR_EL1 when > > > GICR_WAKER.ProcessorSleep == 1 (which may be set in > > > gic_cpu_pm_notifier()), this can hang the system. > > > > Why? PMR defines what interrupts will be presented to the CPU > > interface and trigger an exception. It doesn't affect putting the CPU > > to sleep nor the wake-up. > > I don't think we are allowed to have traffic between the CPU IF and > the GICR when ProcessorSleep == 1. So, again IIUC, we can't write > the PMR (if PMHE == 1) after putting the GICR in ProcessorSleep ==1 > because this would sync with the GICR. Hmmm. This restriction isn't obvious to me. There is some vague threats in 10.1, but nothing concrete. A.4.11 is more explicit, but doesn't really define what 'traffic' is. It is also tied to the stream protocol, which isn't actually mandated. But if it exists, PHME doesn't have any influence on PMR being sent back to the RD. That can happen at any time (it is just that we enforce it with a DSB in the case where PHME is set). Note that do_cpu_idle() already does this, so we'd already be on thin ice. It is also unclear whether 'traffic' occurs when the group enables are set to 0, which is the case when we set ProcessorSleep=1. And to be honest, setting ProcessorSleep=1 in the kernel (which implies DS=1) has always been pretty odd. I don't know of any HW that requires it. I'd be tempted to get rid of it for good. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel