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 DDF8DC38142 for ; Tue, 24 Jan 2023 12:46:17 +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: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=2ASZMvnhVYtejQESfqnvudn59GphnY0PlFA4oDgxrsU=; b=iskKNhgIduQdbe vXjXpIo4lBAd4ivYnngc+fUEMMn5hWGpyFzsw8vC/kpo7pOJu27AjiY/lQ2nk9qqkul+zPCrgRH0q FjED11csAyvH3Z+LkzxLSsg6NLSwNrVxeWC+i9taysSWAr41TPF/VD6a8urt8UdGe5x3k9Ahdt7YK AWbw1UaVmkokdBN4P0JD3CTHu/HtJ878EgOfY8BVMiBP8DEbyZkP+X4HEPLjSDiF4sPKq/+mqDaK8 k5Do+LVqqcBWKggfQwMk9eDbKFHc/KCeUtku2zvg22e7ZpJ7OGKZwP5Dx2JEJvamolMqoVVP6xuC5 gj1tVgySQXJ1NpMR2hLA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pKIfx-003mdh-S3; Tue, 24 Jan 2023 12:45:21 +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 1pKIfu-003mbO-1b for linux-arm-kernel@lists.infradead.org; Tue, 24 Jan 2023 12:45:20 +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 A08AA1FB; Tue, 24 Jan 2023 04:45:43 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.11.85]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 467CD3F71E; Tue, 24 Jan 2023 04:44:51 -0800 (PST) Date: Tue, 24 Jan 2023 12:44:33 +0000 From: Mark Rutland To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, broonie@kernel.org, catalin.marinas@arm.com, will@kernel.org Subject: Re: [PATCH 3/4] arm64: add ARM64_HAS_GIC_PRIO_NO_PMHE cpucap Message-ID: References: <20230123124042.718743-1-mark.rutland@arm.com> <20230123124042.718743-4-mark.rutland@arm.com> <86bkmpmlkc.wl-maz@kernel.org> <868rhsmg87.wl-maz@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <868rhsmg87.wl-maz@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230124_044518_244162_D923D939 X-CRM114-Status: GOOD ( 37.64 ) 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 Tue, Jan 24, 2023 at 09:31:04AM +0000, Marc Zyngier wrote: > On Mon, 23 Jan 2023 14:30:17 +0000, > Mark Rutland wrote: > > > > On Mon, Jan 23, 2023 at 01:23:31PM +0000, Marc Zyngier wrote: > > > On Mon, 23 Jan 2023 12:40:41 +0000, > > > Mark Rutland wrote: > > > > > > > > When Priority Mask Hint Enable (PMHE) == 0b1, the GIC may use the PMR > > > > value to determine whether to signal an IRQ to a PE, and consequently > > > > after a change to the PMR value, a DSB SY may be required to ensure that > > > > interrupts are signalled to a CPU in finite time. When PMHE == 0b0, > > > > interrupts are always signalled to the relevant PE, and all masking > > > > occurs locally, without requiring a DSB SY. > > > > > > > > Since commit: > > > > > > > > f226650494c6aa87 ("arm64: Relax ICC_PMR_EL1 accesses when ICC_CTLR_EL1.PMHE is clear") > > > > > > > > ... we handle this dynamically: in most cases a static key is used to > > > > determine whether to issue a DSB SY, but the entry code must read from > > > > ICC_CTLR_EL1 as static keys aren't accessible from plain assembly. > > > > > > > > It would be much nicer to use an alternative instruction sequence for > > > > the DSB, as this would avoid the need to read from ICC_CTLR_EL1 in the > > > > entry code, and for most other code this will result in simpler code > > > > generation with fewer instructions and fewer branches. > > > > > > > > This patch adds a new ARM64_HAS_GIC_PRIO_NO_PMHE cpucap which is only > > > > set when ICC_CTLR_EL1.PMHE == 0b0 (and GIC priority masking is in use). > > > > This allows us to replace the existing users of the `gic_pmr_sync` > > > > static key with alternative sequences which default to a DSB SY and are > > > > relaxed to a NOP when PMHE is not in use. > > > > > > I personally find the "negative capability" pretty annoying, specially > > > considering that hardly anyone uses PMHE. The way the code reads with > > > this patch, it is always some sort of double negation. > > > > For the polarity and double-negation, I could rename this to > > ARM64_HAS_GIC_PRIO_RELAXED_SYNC, if that helps? > > It certainly reads much better. FWIW, I'll go with that for now; as below it's more painful to go from `NOP` to `DSB SY`, and either we end up needing a alt_patch_dsb_sy() callback, or generating the alternative sequences this patch was trying to avoid. I'll go clear up all the related naming and comments to talk in terms of "relaxed synchronization" rather than "no PMHE". Thanks, Mark. > > > Can't the DSB be patched-in instead, making the PMHE cap a "positive" > > > one? > > > > We could; my rationale for doing it this way is that we can use the common NOP > > patching helper, and avoid generating a copy of the `DSB SY` instruction per > > pmr_sync() call (which gets generated near to the call and never gets free, > > unlike the alt_instr entries), which adds up quickly when using pseudo-NMIs. > > Having an equivalent to alt_cb_patch_nops to patch in "DSB SY" would > result in similar gains, only less reusable... > > > > > > It shouldn't affect interrupt distribution as long as the > > > patching occurs before we take interrupts. For modules, the patching always > > > occurs before we can run the module, so this should be equally safe. > > > > I agree it shouldn't matter either way -- until we've patched in > > ARM64_HAS_GIC_PRIO_MASKING alternatives it's not going to matter. > > > > > The patch otherwise looks OK to me. > > > > Thanks! > > > > Do you have a preference between the ARM64_HAS_GIC_PRIO_RELAXED_SYNC or > > ARM64_HAS_GIC_PRIO_PMHE options above? > > ARM64_HAS_GIC_PRIO_PMHE would have my preference (it spells out the > feature that drives the property), but this requires a bit more work > (a new patching callback), and probably results in more limited gains > memory wise. > > 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