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=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 76ACBC48BD1 for ; Fri, 11 Jun 2021 09:44:00 +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 3E635613D5 for ; Fri, 11 Jun 2021 09:44:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3E635613D5 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=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=A0uoeFJ/ESx7c9xlUJTz3b6tAkYKcPO8X+gN3TRapQ4=; b=kFcrXJncZU+2Xu qxRcryWAmKM08/5e73dNaWpPKyP/+rdg8eSl7/sUts1x9MxJbojckprUBhJ0xV5HVv4uT3zmBTG4j Ut4yKl2AeSvqQYUqUNQzovvin+EuDUO7b+PguUZqB3qOFftlsJCnsQoWik2ht+q8xLQDNnNZr81e6 +kYmC21onKcbq6dV8nvfB7L+L+dHTwARHWSccj5kWobHl70tmoGjB6QGdsgqpFsfdZKz8L2QmcSp9 p2Bzl1MrO6+nqJfDB5/ycUKq21QiXLXysqwGT5d42UJIPK4VeTLu3JZW3rkQCjwfkqMo9Q0thTFyG JYczuclQupnj989r/feA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lrdff-004XQq-Ng; Fri, 11 Jun 2021 09:41:47 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lrdfc-004XQQ-Eh for linux-arm-kernel@lists.infradead.org; Fri, 11 Jun 2021 09:41:46 +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 3B633D6E; Fri, 11 Jun 2021 02:41:41 -0700 (PDT) Received: from lpieralisi (e121166-lin.cambridge.arm.com [10.1.196.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8C21D3F719; Fri, 11 Jun 2021 02:41:39 -0700 (PDT) Date: Fri, 11 Jun 2021 10:41:34 +0100 From: Lorenzo Pieralisi To: Marc Zyngier 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 Message-ID: <20210611094134.GA2789@lpieralisi> References: <20210608172715.2396787-1-maz@kernel.org> <20210610162823.GA20025@lpieralisi> <87eed8reyd.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87eed8reyd.wl-maz@kernel.org> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210611_024144_613508_6B7C4F7A X-CRM114-Status: GOOD ( 38.50 ) 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, 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. 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_. 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(). > > 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. > > I wonder whether this arm_cpuidle_{save,restore}_context() should > > be moved into the gic_cpu_pm_notifier() itself - which would > > solve also the PSCI suspend issue Sudeep raised - it would be > > Moving from PMR to DAIF masking is something we only do on particular > spots (exception entry/exit, guest entry/exit) as it affects the > behaviour of simple things such as local_irq_*(). Moving it to a > higher level feels super dangerous. Yes it is dangerous and ugly. It is also true that what needs to be saved/restore on idle entry is becoming extremely complicated and hard to track across kernel subsystems and that's unfortunate too, that's what is causing these bugs. Thanks, Lorenzo > > a bit ugly though (CPU PM notifiers are run in S2R and CPUidle > > automatically and this would work for any S2R/CPUidle backend > > other than PSCI even though that does not/will never exist on > > arm64 ;-)) > > > > https://lore.kernel.org/linux-arm-kernel/20210608182044.ayqa6fbab4jyz7kp@bogus > > > > I still believe this series is right - just raised these points > > for discussion. > > 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