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 8A2C1C4332F for ; Thu, 10 Nov 2022 08:29:30 +0000 (UTC) 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=Mp0wxHYkVkUI+FDsdYa2SfrIoMRKI2FVmdhcwwWdJnE=; b=edVw2t2W7LJfb7 /Nmn4Jz7NWXb1uXJBX+xpOwawOzrFn4LaQd7OnrHmZ1ZYRvZsK7bjG7fHFt/62IA7nVTsYZnkpVFH PQJTJXRgH9bFIDiwo0FCgELROyY/1duJk37VNQMLnb/p/KYaetxOQIDa9DzaPH84XExqquzUJiRWE OTH1WgQtISAAhDVIQu+CmYXL+aYF8V6FKvz7VKfb1K3jiFC3x8t2TfPtL92ENGIfChQhprkbUsaDw ry6tRbOf3UDWbxs3Z6E+objGfkzBgPTMFL+uKhRSZ37AezyEVcXM1kfvHP+iKzpZmpLo/q9K3IwWa qyeJBQcX0PPRgEEBo+FQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ot2vC-004GV8-IM; Thu, 10 Nov 2022 08:28:26 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ot2v9-004GUU-RO for linux-arm-kernel@lists.infradead.org; Thu, 10 Nov 2022 08:28:25 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6A13F61DC0; Thu, 10 Nov 2022 08:28:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CC1ECC433C1; Thu, 10 Nov 2022 08:28:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1668068902; bh=TiwzFb5abfQHG8UAlAYneya7FpEO7dn46yVbdgJ9he8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=sGYC6dIr20MLWZ2wDJDVyGK8gFHmE/Fu3OrBg2XwDuLwFmPfeMfseeQ/83z8qwNWm CaKmJDwsi0IJYBa6b0cKytJy9eMkxeOO/6pF8t5Yecbjzx5EU8RH8+DPcfLQG3xEHp ymJDEwqt8Ojvv4MDWeklkl/+YmIFAXfTGBUMot/32aFIHhtKd3Fgr17KZnzEJ9QnO1 0oKwE6xlZbq1vOBsIx2gcQ572PToTxfGi1mHGk3lKJ6jV/JHuPibCb0MI0YsnqKgwf RADbQC4OoS3xcNikTzlzG547zGdPS2yNB4HKcp1msAGk/MwXZUJl1vP8x1UpyqyZp6 EhN8WqxV+HVJQ== Received: from ip-185-104-136-29.ptr.icomera.net ([185.104.136.29] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1ot2v6-0056MU-AK; Thu, 10 Nov 2022 08:28:20 +0000 Date: Thu, 10 Nov 2022 08:26:44 +0000 Message-ID: <87leojuse3.wl-maz@kernel.org> From: Marc Zyngier To: Mark Brown Cc: Catalin Marinas , Will Deacon , Lorenzo Pieralisi , Mark Rutland , Sami Mujawar , Thomas Gleixner , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1 00/18] arm64/nmi: Support for FEAT_NMI In-Reply-To: References: <20221104235453.870573-1-broonie@kernel.org> <87r0ygfh2s.wl-maz@kernel.org> 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: 185.104.136.29 X-SA-Exim-Rcpt-To: broonie@kernel.org, catalin.marinas@arm.com, will@kernel.org, lpieralisi@kernel.org, mark.rutland@arm.com, Sami.Mujawar@arm.com, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org 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-20221110_002823_988310_9D7A3CDE X-CRM114-Status: GOOD ( 55.02 ) 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 Wed, 09 Nov 2022 15:35:25 +0000, Mark Brown wrote: > > [1 ] > On Sun, Nov 06, 2022 at 11:38:51AM +0000, Marc Zyngier wrote: > > Mark Brown wrote: > > > > interrupts of any kind to add masking for NMIs. This masking is not > > > added to our standard interrupt masking operations since that would > > > result in widespread masking of NMIs which would undermine their value. > > > Given that there's a large amount of kernel code a good proportion of > > > which I'm not terribly familar with it is likely that this area of the > > > series needs attention in review as there may be be be areas that have > > > been missed or misunderstood. > > > Can you at least clarify why the no-quite-NMI cannot follow the > > pattern established by the pseudo-NMI support? I would have expected > > the critical sections to strictly map between the two so-called-NMI > > implementations. > > That's fair, I'll expand on this topic in the cover for next version. > It is basically similar, there's a few differences where I couldn't > convince myself that we were intentionally masking NMIs like > el0_svc_common() and it did seem like we wanted to control things > separately during entry given the separate report path we have with the > architected version. It's possible I've completely misunderstood this > one way or another. > > I did look at a couple of alernatives that had more overlap. I looked > at pulling the pNMI code out of the DAIF handling but given the > implementation the benefits seemed unclear for the complexity given that > it is actually working with DAIF. I did also strongly consider putting > the NMI handling into the DAIF handling but that was making it difficult > to distinguish the handling of architected NMIs and the implicit > coverage didn't seem ideal, I could have another spin through that > approach though - I was on the edge with the approach there. It sounds > like you'd really prefer that one. My gripe with the current approach is that it took us a *very* long while to make pNMI work correctly in all cases (I'm pretty sure that Mark Rutland still has nightmares about it). But now that we have the framework for it and the mental model to match, I'd really want both NMI methods to be merely two implementations of the same concept. The only obvious difference should be that we can identify NMIs early and that acking a NMI cannot result in acking an IRQ. I'd expect everything else to be otherwise similar. > > > > Using this feature in KVM guests will require the implementation of vGIC > > > support which is not present in this series, and there is also no usage > > > of the feature at EL2. While FEAT_NMI does add a new writable register > > > ALLINT the value is already context switched for EL1 via SPSR_EL2.ALLINT > > > and we can't trap read access to the register so we don't manage the > > > write trap that is available in HCRX_EL2.TALLINT. Guests can read from > > > the register anyway and should only be able to affect their own state. > > > ... and create some architectural state that is impossible to manage > > and migrate. Great. > > We can't already manage and migrate SPSR_EL2 (or SPSR_EL1 for that > matter)? Of course we can. SPSR_EL2 is nothing but the guest's PSTATE, and thankfully we manage it. Same thing for SPSR_EL1. > I might have missed part of how that's handled or used, > especially given that I've not actively worked through this yet, but if > we're not doing so then we've got other problems with for example > PSTATE.BTYPE or PSTATE.SSBS. It looks like it's available and I'm not > seeing any filtering of the values or anything. Imagine you run a VM on some FEAT_NMI-enabled HW. The guest uses the PSTATE.ALLINT bit because it can. Now we migrate the VM somewhere else that doesn't have ALLINT, and the VM is dead. This is a general problem with the architecture, but we definitely should take care of it as we're enabling these features. > > Note that ALLINT.ALLINT is similar to DAIF or SBSS, it's directly > mirroring the state of PSTATE.ALLINT. The only access EL2 has to the > EL1 state for ALLINT is via SPSR, on entry to EL2 ALLINT is reset as per > the configuration in SCTLR_EL2.{SPINTMASK,NMI} and then the EL1 value is > restored as part of exception return. > > > Frankly, we are accumulating a bunch of half implemented features > > (SME, SPE) for which there is no KVM support, and I don't think this > > is the correct direction of travel. > > Yeah, it's not great. For NMIs Lorenzo is looking at the GIC side, I'll > let him clarify his schedule. Here it's more the case that implementing > the architecture side without an interrupt controller isn't useful (and > certainly not usefully testable) than anything else - my expectation is > that it should work fine, though from your comments above it sounds like > there will be issues with the handling of PSTATE that I've not realised. The vgic support should be rather trivial. It is only a matter of adding the required interrupt state, reworking the priority sorting and LR stuffing, handling the additional MMIO range and dealing with the GIC versioning. Trapping of ALLINT should be done at the same time. If people cannot be bothered, I'll end-up doing it myself. 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