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.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,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 24AD0C433E0 for ; Thu, 18 Feb 2021 15:05:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D81B964E6F for ; Thu, 18 Feb 2021 15:05:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230139AbhBRPFA (ORCPT ); Thu, 18 Feb 2021 10:05:00 -0500 Received: from marcansoft.com ([212.63.210.85]:58684 "EHLO mail.marcansoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232596AbhBRMxY (ORCPT ); Thu, 18 Feb 2021 07:53:24 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: marcan@marcan.st) by mail.marcansoft.com (Postfix) with ESMTPSA id 05A1C3FA55; Thu, 18 Feb 2021 12:51:42 +0000 (UTC) To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, Marc Zyngier , Rob Herring , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Will Deacon , Linus Walleij , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210215121713.57687-1-marcan@marcan.st> <20210215121713.57687-9-marcan@marcan.st> <20210217122200.GC5556@C02TD0UTHF1T.local> From: Hector Martin Subject: Re: [PATCH v2 08/25] arm64: Always keep DAIF.[IF] in sync Message-ID: Date: Thu, 18 Feb 2021 21:51:40 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20210217122200.GC5556@C02TD0UTHF1T.local> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: es-ES Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17/02/2021 21.22, Mark Rutland wrote: >> Root irqchip drivers can discriminate between IRQs and FIQs by checking >> the ISR_EL1 system register. > > I think we can remove this note for now. If we go with seperate handlers > this won't be necessary, and if not this would be better placed on a > commit adding the FIQ handling capability. Indeed, this doesn't make sense any more. Changed for v3. > Judging by `git grep -Wi daif -- arch/arm64`, with this patch applied, > we'll also need fixups in: > > * gic_arch_enable_irqs() in arch/arm64/include/asm/arch_gicv3.h > * save_and_disable_irq() in arch/arm64/include/asm/assembler.h (noted below) > * local_daif_save_flags() in arch/arm64/include/asm/daifflags.h > (the fake DAIF should have F set too) > * __cpu_do_idle_irqprio() in arch/arm64/kernel/process.c Good catches. A few of those are irrelevant for M1 but need to be done now that we're making this change globally, others I just missed from the beginning. There's also an incorrect comment in entry.S: /* * DA_F were cleared at start of handling. If anything is set in * DAIF, we come back from an NMI, so skip preemption */ mrs x0, daif orr x24, x24, x0 Now only DA__ are cleared. This actually pairs with gic_arch_enable_irqs() and begs the question: in priority masking systems, do we unmask both IRQ and FIQ (the gic_arch_enable_irqs change), or do we leave FIQ masked (which instead would need an AND in that part of entry.S so as to not consider FIQ masked as meaning we're coming back from an NMI)? And a minor related one: should init_gic_priority_masking() WARN if FIQ is masked too? This probably goes with the above. Either way, this was nontrivial to make sense of, so I'll make that entry.S comment clearer while I'm touching it. > I think save_and_diable_irq below needs to be updated too, since it > only sets DAIF.I and leaves DAIF.F as-is. Totally missed this one! Fixed for v3. >> - * FIQ is never expected, but we mask it when we disable debug exceptions, and >> - * unmask it at all other times. >> + * FIQ is never expected on most platforms, but we keep it synchronized >> + * with the IRQ mask status. On platforms that do not expect FIQ, that vector >> + * triggers a kernel panic. On platforms that do, the FIQ vector is unified >> + * with the IRQ vector. >> */ > > Can we please delete this bit, though? Now that we say IRQ and FIQ are > masked/unmasked together, I don't think the rest is necessary to > understand the masking logic, and it's one less thing to keep in sync > with changes to the entry code. Gone :) -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub 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.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,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 35A61C433DB for ; Thu, 18 Feb 2021 12:53:13 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 C45F564E28 for ; Thu, 18 Feb 2021 12:53:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C45F564E28 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=marcan.st 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=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:Subject: From:References:To:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=UPq3JVLn4mXxMGhjt0qPxYTig3nnvkasARc9Ct4z/MU=; b=s0xoVSuvyqdCmWqxOjc6Vln0Q H5V34ErYNPahtOGmLvGeigDV/0QgGBUjrONZpLQe4AIF31dW01QlbvpeoCUNfgjBsLua+nVOnlar1 eFYg9zo3GutEy6y7SxnBBACmGSxJLj0faP0YTkzAtIzYOW9PRh0cx5dmmjxJBPI2CcobMB21sdNYq ckmGWCF0dzvSQV7j7Ony/v70DhUo8nkxpzCDNo2OHk/f614ChYJuAftAwB3hpcJu/CxdJgQo1KkA9 72B1rWmnSsZ9W5PEr7s/l7LhUkhv/yIQ1kjSgdtXAM3ElxVV5/i46tHvHn2vzI6VfsGDj/iLadN/w hYjZ2tTMA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lCimn-0006jg-ID; Thu, 18 Feb 2021 12:52:01 +0000 Received: from marcansoft.com ([212.63.210.85] helo=mail.marcansoft.com) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1lCimj-0006hs-Rw for linux-arm-kernel@lists.infradead.org; Thu, 18 Feb 2021 12:51:59 +0000 Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: marcan@marcan.st) by mail.marcansoft.com (Postfix) with ESMTPSA id 05A1C3FA55; Thu, 18 Feb 2021 12:51:42 +0000 (UTC) To: Mark Rutland References: <20210215121713.57687-1-marcan@marcan.st> <20210215121713.57687-9-marcan@marcan.st> <20210217122200.GC5556@C02TD0UTHF1T.local> From: Hector Martin Subject: Re: [PATCH v2 08/25] arm64: Always keep DAIF.[IF] in sync Message-ID: Date: Thu, 18 Feb 2021 21:51:40 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20210217122200.GC5556@C02TD0UTHF1T.local> Content-Language: es-ES X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210218_075158_039161_E5A5233B X-CRM114-Status: GOOD ( 24.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Arnd Bergmann , Rob Herring , Tony Lindgren , Marc Zyngier , Linus Walleij , linux-kernel@vger.kernel.org, Krzysztof Kozlowski , devicetree@vger.kernel.org, Alexander Graf , Olof Johansson , Mohamed Mediouni , Stan Skowronek , Will Deacon , linux-arm-kernel@lists.infradead.org, Mark Kettenis Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 17/02/2021 21.22, Mark Rutland wrote: >> Root irqchip drivers can discriminate between IRQs and FIQs by checking >> the ISR_EL1 system register. > > I think we can remove this note for now. If we go with seperate handlers > this won't be necessary, and if not this would be better placed on a > commit adding the FIQ handling capability. Indeed, this doesn't make sense any more. Changed for v3. > Judging by `git grep -Wi daif -- arch/arm64`, with this patch applied, > we'll also need fixups in: > > * gic_arch_enable_irqs() in arch/arm64/include/asm/arch_gicv3.h > * save_and_disable_irq() in arch/arm64/include/asm/assembler.h (noted below) > * local_daif_save_flags() in arch/arm64/include/asm/daifflags.h > (the fake DAIF should have F set too) > * __cpu_do_idle_irqprio() in arch/arm64/kernel/process.c Good catches. A few of those are irrelevant for M1 but need to be done now that we're making this change globally, others I just missed from the beginning. There's also an incorrect comment in entry.S: /* * DA_F were cleared at start of handling. If anything is set in * DAIF, we come back from an NMI, so skip preemption */ mrs x0, daif orr x24, x24, x0 Now only DA__ are cleared. This actually pairs with gic_arch_enable_irqs() and begs the question: in priority masking systems, do we unmask both IRQ and FIQ (the gic_arch_enable_irqs change), or do we leave FIQ masked (which instead would need an AND in that part of entry.S so as to not consider FIQ masked as meaning we're coming back from an NMI)? And a minor related one: should init_gic_priority_masking() WARN if FIQ is masked too? This probably goes with the above. Either way, this was nontrivial to make sense of, so I'll make that entry.S comment clearer while I'm touching it. > I think save_and_diable_irq below needs to be updated too, since it > only sets DAIF.I and leaves DAIF.F as-is. Totally missed this one! Fixed for v3. >> - * FIQ is never expected, but we mask it when we disable debug exceptions, and >> - * unmask it at all other times. >> + * FIQ is never expected on most platforms, but we keep it synchronized >> + * with the IRQ mask status. On platforms that do not expect FIQ, that vector >> + * triggers a kernel panic. On platforms that do, the FIQ vector is unified >> + * with the IRQ vector. >> */ > > Can we please delete this bit, though? Now that we say IRQ and FIQ are > masked/unmasked together, I don't think the rest is necessary to > understand the masking logic, and it's one less thing to keep in sync > with changes to the entry code. Gone :) -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel