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=-8.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 5D19CC0650F for ; Mon, 5 Aug 2019 12:03:09 +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 29D07206A2 for ; Mon, 5 Aug 2019 12:03:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="sZxBdKyV" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 29D07206A2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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=ZCZLXkYXez90+n0fST88pucJrBXPnq7+13wZFEullvE=; b=sZxBdKyV/PA6pl B24mFzWf/j/PqSNpVTp8xK1nHUMVAmysKr//nus7Zr1XJRogLRl/QjzGzn844OLUEgqN8qb1Lttpd NGu0grDhzd3iZYMn1lJj4QYfQ+2gPsaMRYPi7HT5SWQhAjbJv9WXEYuQeFaFOcGtJhmhBScOvTsYK ZrQSuyKu18hBuLF4jI+DzE+Jj58y220doxl74c6Yx7s61oZcKDKO9I9mz/1X7xek4DUh0io6OVuzq OlD0lBr6dLvg7U91dHVNFTynclFw5Nidh8bbtH2RgREvMyOeqeI0JjQgHcNjm5TGcdATWJk8L+2d4 Erv+2AgoRuTyBg2Z8fxg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hubhk-0002rR-Am; Mon, 05 Aug 2019 12:03:08 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hubhh-0002mL-Gl for linux-arm-kernel@lists.infradead.org; Mon, 05 Aug 2019 12:03:07 +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 32E221570; Mon, 5 Aug 2019 05:03:04 -0700 (PDT) Received: from [10.1.197.61] (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EB0723F694; Mon, 5 Aug 2019 05:03:02 -0700 (PDT) Subject: Re: [PATCH v2 1/2] arm64: Relax ICC_PMR_EL1 accesses when ICC_CTLR_EL1.PMHE is clear To: Will Deacon , Catalin Marinas References: <20190802125208.73162-1-maz@kernel.org> <20190802125208.73162-2-maz@kernel.org> From: Marc Zyngier Organization: Approximate Message-ID: <97b2d8bd-427a-ace0-cb5e-903ea58bd4a0@kernel.org> Date: Mon, 5 Aug 2019 13:03:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190802125208.73162-2-maz@kernel.org> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190805_050305_652349_82CFE9A4 X-CRM114-Status: GOOD ( 27.69 ) 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: Suzuki K Poulose , liwei391@huawei.com, James Morse , Julien Thierry , huawei.libin@huawei.com, guohanjun@huawei.com, Will Deacon , linux-arm-kernel@lists.infradead.org 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 02/08/2019 13:52, Marc Zyngier wrote: > From: Marc Zyngier > > The GICv3 architecture specification is incredibly misleading when it > comes to PMR and the requirement for a DSB. It turns out that this DSB > is only required if the CPU interface sends an Upstream Control > message to the redistributor in order to update the RD's view of PMR. > > This message is only sent when ICC_CTLR_EL1.PMHE is set, which isn't > the case in Linux. It can still be set from EL3, so some special care > is required. But the upshot is that in the (hopefuly large) majority > of the cases, we can drop the DSB altogether. > > This relies on a new static key being set if the boot CPU has PMHE > set. The drawback is that this static key has to be exported to > modules. > > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Marc Zyngier > Cc: James Morse > Cc: Julien Thierry > Cc: Suzuki K Poulose > Signed-off-by: Marc Zyngier > --- > arch/arm64/include/asm/barrier.h | 12 ++++++++++++ > arch/arm64/include/asm/daifflags.h | 3 ++- > arch/arm64/include/asm/irqflags.h | 19 ++++++++++--------- > arch/arm64/include/asm/kvm_host.h | 3 +-- > arch/arm64/kernel/entry.S | 6 ++++-- > arch/arm64/kvm/hyp/switch.c | 4 ++-- > drivers/irqchip/irq-gic-v3.c | 17 +++++++++++++++++ > include/linux/irqchip/arm-gic-v3.h | 2 ++ > 8 files changed, 50 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h > index e0e2b1946f42..7d9cc5ec4971 100644 > --- a/arch/arm64/include/asm/barrier.h > +++ b/arch/arm64/include/asm/barrier.h > @@ -29,6 +29,18 @@ > SB_BARRIER_INSN"nop\n", \ > ARM64_HAS_SB)) > > +#ifdef CONFIG_ARM64_PSEUDO_NMI > +#define pmr_sync() \ > + do { \ > + extern struct static_key_false gic_pmr_sync; \ > + \ > + if (static_branch_unlikely(&gic_pmr_sync)) \ > + dsb(sy); \ > + } while(0) > +#else > +#define pmr_sync() do {} while (0) > +#endif > + > #define mb() dsb(sy) > #define rmb() dsb(ld) > #define wmb() dsb(st) > diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h > index 987926ed535e..00b16793505a 100644 > --- a/arch/arm64/include/asm/daifflags.h > +++ b/arch/arm64/include/asm/daifflags.h > @@ -8,6 +8,7 @@ > #include > > #include > +#include > #include > > #define DAIF_PROCCTX 0 > @@ -63,7 +64,7 @@ static inline void local_daif_restore(unsigned long flags) > > if (system_uses_irq_prio_masking()) { > gic_write_pmr(GIC_PRIO_IRQON); > - dsb(sy); > + pmr_sync(); > } > } else if (system_uses_irq_prio_masking()) { > u64 pmr; > diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h > index 7872f260c9ee..a5e7115d2f93 100644 > --- a/arch/arm64/include/asm/irqflags.h > +++ b/arch/arm64/include/asm/irqflags.h > @@ -8,6 +8,7 @@ > #ifdef __KERNEL__ > > #include > +#include > #include > #include > > @@ -36,14 +37,14 @@ static inline void arch_local_irq_enable(void) > } > > asm volatile(ALTERNATIVE( > - "msr daifclr, #2 // arch_local_irq_enable\n" > - "nop", > - __msr_s(SYS_ICC_PMR_EL1, "%0") > - "dsb sy", > + "msr daifclr, #2 // arch_local_irq_enable", > + __msr_s(SYS_ICC_PMR_EL1, "%0"), > ARM64_HAS_IRQ_PRIO_MASKING) > : > : "r" ((unsigned long) GIC_PRIO_IRQON) > : "memory"); > + > + pmr_sync(); > } > > static inline void arch_local_irq_disable(void) > @@ -118,14 +119,14 @@ static inline unsigned long arch_local_irq_save(void) > static inline void arch_local_irq_restore(unsigned long flags) > { > asm volatile(ALTERNATIVE( > - "msr daif, %0\n" > - "nop", > - __msr_s(SYS_ICC_PMR_EL1, "%0") > - "dsb sy", > - ARM64_HAS_IRQ_PRIO_MASKING) > + "msr daif, %0", > + __msr_s(SYS_ICC_PMR_EL1, "%0"), > + ARM64_HAS_IRQ_PRIO_MASKING) > : > : "r" (flags) > : "memory"); > + > + pmr_sync(); > } > > #endif > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index f656169db8c3..5ecb091c8576 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -600,8 +600,7 @@ static inline void kvm_arm_vhe_guest_enter(void) > * local_daif_mask() already sets GIC_PRIO_PSR_I_SET, we just need a > * dsb to ensure the redistributor is forwards EL2 IRQs to the CPU. > */ > - if (system_uses_irq_prio_masking()) > - dsb(sy); > + pmr_sync(); > } > > static inline void kvm_arm_vhe_guest_exit(void) > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index 9cdc4592da3e..6803dd5b4256 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -269,8 +269,10 @@ alternative_else_nop_endif > alternative_if ARM64_HAS_IRQ_PRIO_MASKING > ldr x20, [sp, #S_PMR_SAVE] > msr_s SYS_ICC_PMR_EL1, x20 > - /* Ensure priority change is seen by redistributor */ > - dsb sy > + mrs_s x21, SYS_ICC_CTLR_EL1 > + tbz x21, #6, .L__skip_pmr_sync\@ // Check for ICC_CTLR_EL1.PMHE > + dsb sy // Ensure priority change is seen by redistributor > +.L__skip_pmr_sync\@: > alternative_else_nop_endif > > ldp x21, x22, [sp, #S_PC] // load ELR, SPSR > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > index adaf266d8de8..eb131e09d207 100644 > --- a/arch/arm64/kvm/hyp/switch.c > +++ b/arch/arm64/kvm/hyp/switch.c > @@ -12,7 +12,7 @@ > > #include > > -#include > +#include > #include > #include > #include > @@ -605,7 +605,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu) > */ > if (system_uses_irq_prio_masking()) { > gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET); > - dsb(sy); > + pmr_sync(); > } > > vcpu = kern_hyp_va(vcpu); > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 005b70e398b8..38fcb7eebdde 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -87,6 +87,15 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key); > */ > static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis); > > +/* > + * Global static key controlling whether an update to PMR allowing more > + * interrupts requires to be propagated to the redistributor (DSB SY). > + * And this needs to be exported for modules to be able to enable > + * interrupts... > + */ > +DEFINE_STATIC_KEY_FALSE(gic_pmr_sync); > +EXPORT_SYMBOL(gic_pmr_sync); > + > /* ppi_nmi_refs[n] == number of cpus having ppi[n + 16] set as NMI */ > static refcount_t *ppi_nmi_refs; > > @@ -1494,6 +1503,14 @@ static void gic_enable_nmi_support(void) > for (i = 0; i < gic_data.ppi_nr; i++) > refcount_set(&ppi_nmi_refs[i], 0); > > + /* > + * Linux itself doesn't use 1:N distribution, so has no need to > + * set PMHE. The only reason to have it set is if EL3 requires it > + * (and we can't change it). > + */ > + if (read_sysreg_s(SYS_ICC_CTLR_EL1) & ICC_CTLR_EL1_PMHE_MASK) This raw sysreg access breaks 32bit, and should be replaced with gic_read_ctrl() instead. I'll post an updated version later in the week. Thanks, M. -- Jazz is not dead, it just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel