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=-6.9 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham 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 0D306C65BAE for ; Thu, 13 Dec 2018 08:54:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CAEFD20849 for ; Thu, 13 Dec 2018 08:54:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CAEFD20849 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727427AbeLMIyc (ORCPT ); Thu, 13 Dec 2018 03:54:32 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:56562 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726578AbeLMIyc (ORCPT ); Thu, 13 Dec 2018 03:54:32 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9671880D; Thu, 13 Dec 2018 00:54:31 -0800 (PST) Received: from [10.1.197.36] (e112298-lin.cambridge.arm.com [10.1.197.36]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A41E53F6A8; Thu, 13 Dec 2018 00:54:29 -0800 (PST) Subject: Re: [PATCH v7 11/25] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking To: Ard Biesheuvel Cc: linux-arm-kernel , Linux Kernel Mailing List , Daniel Thompson , joel@joelfernandes.org, Marc Zyngier , Christoffer Dall , James Morse , Catalin Marinas , Will Deacon , Mark Rutland , oleg@redhat.com References: <1544633245-6036-1-git-send-email-julien.thierry@arm.com> <1544633245-6036-12-git-send-email-julien.thierry@arm.com> <19500d6b-62a3-21cb-9ac0-a4e5d4714a63@arm.com> From: Julien Thierry Message-ID: <31e41461-763f-aa7d-91ea-b493ede81eed@arm.com> Date: Thu, 13 Dec 2018 08:54:27 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/12/2018 18:10, Ard Biesheuvel wrote: > On Wed, 12 Dec 2018 at 18:59, Julien Thierry wrote: >> >> >> >> On 12/12/2018 17:27, Ard Biesheuvel wrote: >>> On Wed, 12 Dec 2018 at 17:48, Julien Thierry wrote: >>>> >>>> Instead disabling interrupts by setting the PSR.I bit, use a priority >>>> higher than the one used for interrupts to mask them via PMR. >>>> >>>> When using PMR to disable interrupts, the value of PMR will be used >>>> instead of PSR.[DAIF] for the irqflags. >>>> >>>> Signed-off-by: Julien Thierry >>>> Suggested-by: Daniel Thompson >>>> Cc: Catalin Marinas >>>> Cc: Will Deacon >>>> Cc: Ard Biesheuvel >>>> Cc: Oleg Nesterov >>>> --- >>>> arch/arm64/include/asm/efi.h | 5 +- >>>> arch/arm64/include/asm/irqflags.h | 123 +++++++++++++++++++++++++++++--------- >>>> 2 files changed, 99 insertions(+), 29 deletions(-) >>>> >>>> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h >>>> index 7ed3208..a9d3ebc 100644 >>>> --- a/arch/arm64/include/asm/efi.h >>>> +++ b/arch/arm64/include/asm/efi.h >>>> @@ -42,7 +42,10 @@ >>>> >>>> efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...); >>>> >>>> -#define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT) >>>> +#define ARCH_EFI_IRQ_FLAGS_MASK \ >>>> + (system_uses_irq_prio_masking() ? \ >>>> + GIC_PRIO_IRQON : \ >>>> + (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)) >>>> >>> >>> This mask is used to determine whether we return from a firmware call >>> with a different value for the I flag than we entered it with. So >>> instead of changing the mask, we should change the way we record DAIF, >>> given that the firmware is still going to poke the I bit if it >>> misbehaves, regardless of whether the OS happens to use priorities for >>> interrupt masking. >>> >> >> Thanks for pointing that out, so this change makes little sense... >> >> The annoying part is that the flag checking takes place in the arch >> agnostic code. >> >> Would introducing some overriddable efi_get_flags() or efi_save_flags() >> that default to local_save_flags() seem like an acceptable solution? >> >> This way I could override it for arm64 and still return the DAIF bits. >> > > I don't follow the reasoning below about irqflags exactly, but is > there any way we could simply but both PMR and DAIF in flags? We could > even update the mask here to ensure that the firmware doesn't corrupt > the PMR. > So, that was the case in my previous versions of the series, and as you said, that covered checking both DAIF bits and PMR on return from EFI services. But Catalin suggested that irqflags could just use PMR when we enable the priority masking feature. Catalin's suggestion does simplify things, except for this part. However, it doesn't seem to far-fetched to me that the architecture could have a more generic way to tell the EFI driver "this is the set of stuff that I care about and you should return from runtime services with this stuff in the same state as before" without the "set of stuff" being limited to irqflags. But maybe this would be over-engineering just to deal with my use-case... Thanks, -- Julien Thierry 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=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS autolearn=unavailable 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 315D4C67839 for ; Thu, 13 Dec 2018 08:54:50 +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 F359E2086D for ; Thu, 13 Dec 2018 08:54:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="P1yIRlaa" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F359E2086D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7+vPGRlyYsXO4fqR/SAXYJsm7gS0KRVM5OQu/bVSfXc=; b=P1yIRlaacW/dDr BBStkLfGYrEPot2CXkrIglUhjhg5BuPDTMJOG7g1z3WMVp8jAOvl6bD/Y1NFomFeGU8JyRK9k/RZ5 v+Q3QOEGGa23WsYkNpYGOmHKg3i35ICvDEnD7//h47TV0rWq+fjQWbjAJWnupSLjGi6ZIXUupJkZ3 GhqSMMEkdJUSw5GoIPLBitdhXYzDKuXXy0GOuDyB4RNo4Hw7G4V2acmb3dF82BxXokiA6BlHZuSej EdRILZSPsSVhscDQigUkz6Q749ZcK6Eda6OLbspIyDezZ2g4e609fNnAc3HK7CHCmUWGdtG+IjSdy 4GZCSuk6FGzVS6cWRFfA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gXMlc-0004CC-FU; Thu, 13 Dec 2018 08:54:48 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gXMlZ-00048X-3A for linux-arm-kernel@lists.infradead.org; Thu, 13 Dec 2018 08:54:46 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9671880D; Thu, 13 Dec 2018 00:54:31 -0800 (PST) Received: from [10.1.197.36] (e112298-lin.cambridge.arm.com [10.1.197.36]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A41E53F6A8; Thu, 13 Dec 2018 00:54:29 -0800 (PST) Subject: Re: [PATCH v7 11/25] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking To: Ard Biesheuvel References: <1544633245-6036-1-git-send-email-julien.thierry@arm.com> <1544633245-6036-12-git-send-email-julien.thierry@arm.com> <19500d6b-62a3-21cb-9ac0-a4e5d4714a63@arm.com> From: Julien Thierry Message-ID: <31e41461-763f-aa7d-91ea-b493ede81eed@arm.com> Date: Thu, 13 Dec 2018 08:54:27 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20181213_005445_195212_FDBCB8BB X-CRM114-Status: GOOD ( 23.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Daniel Thompson , Marc Zyngier , Catalin Marinas , Will Deacon , Linux Kernel Mailing List , Christoffer Dall , James Morse , oleg@redhat.com, joel@joelfernandes.org, linux-arm-kernel Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 12/12/2018 18:10, Ard Biesheuvel wrote: > On Wed, 12 Dec 2018 at 18:59, Julien Thierry wrote: >> >> >> >> On 12/12/2018 17:27, Ard Biesheuvel wrote: >>> On Wed, 12 Dec 2018 at 17:48, Julien Thierry wrote: >>>> >>>> Instead disabling interrupts by setting the PSR.I bit, use a priority >>>> higher than the one used for interrupts to mask them via PMR. >>>> >>>> When using PMR to disable interrupts, the value of PMR will be used >>>> instead of PSR.[DAIF] for the irqflags. >>>> >>>> Signed-off-by: Julien Thierry >>>> Suggested-by: Daniel Thompson >>>> Cc: Catalin Marinas >>>> Cc: Will Deacon >>>> Cc: Ard Biesheuvel >>>> Cc: Oleg Nesterov >>>> --- >>>> arch/arm64/include/asm/efi.h | 5 +- >>>> arch/arm64/include/asm/irqflags.h | 123 +++++++++++++++++++++++++++++--------- >>>> 2 files changed, 99 insertions(+), 29 deletions(-) >>>> >>>> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h >>>> index 7ed3208..a9d3ebc 100644 >>>> --- a/arch/arm64/include/asm/efi.h >>>> +++ b/arch/arm64/include/asm/efi.h >>>> @@ -42,7 +42,10 @@ >>>> >>>> efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...); >>>> >>>> -#define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT) >>>> +#define ARCH_EFI_IRQ_FLAGS_MASK \ >>>> + (system_uses_irq_prio_masking() ? \ >>>> + GIC_PRIO_IRQON : \ >>>> + (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)) >>>> >>> >>> This mask is used to determine whether we return from a firmware call >>> with a different value for the I flag than we entered it with. So >>> instead of changing the mask, we should change the way we record DAIF, >>> given that the firmware is still going to poke the I bit if it >>> misbehaves, regardless of whether the OS happens to use priorities for >>> interrupt masking. >>> >> >> Thanks for pointing that out, so this change makes little sense... >> >> The annoying part is that the flag checking takes place in the arch >> agnostic code. >> >> Would introducing some overriddable efi_get_flags() or efi_save_flags() >> that default to local_save_flags() seem like an acceptable solution? >> >> This way I could override it for arm64 and still return the DAIF bits. >> > > I don't follow the reasoning below about irqflags exactly, but is > there any way we could simply but both PMR and DAIF in flags? We could > even update the mask here to ensure that the firmware doesn't corrupt > the PMR. > So, that was the case in my previous versions of the series, and as you said, that covered checking both DAIF bits and PMR on return from EFI services. But Catalin suggested that irqflags could just use PMR when we enable the priority masking feature. Catalin's suggestion does simplify things, except for this part. However, it doesn't seem to far-fetched to me that the architecture could have a more generic way to tell the EFI driver "this is the set of stuff that I care about and you should return from runtime services with this stuff in the same state as before" without the "set of stuff" being limited to irqflags. But maybe this would be over-engineering just to deal with my use-case... Thanks, -- Julien Thierry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel