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 B9048C4708E for ; Wed, 7 Dec 2022 15:22:20 +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=WUim1O45MH+LxrRPWBkiz9pKTwXwDDHo0nQaef3fc80=; b=T/5MppKOrthP9C dk9xAw3WBP8ADZo+LWygiLejHJp1/LZfJ0VYRXluUwU0SC55HT4mku5VuKdsmnPtaIlgxr9XArX+2 TKv7U/Zw51VVclHqcfo7bNHdPjiMQaLh+X47xFiPaQZQtjcvaRXq+f31J0tunNM+WV0OzT1wAxXlq zim7YkyVIblfj3vxPaCxo4D4opuKQ1JkB2QQ/F5B2B3wZV+h3vs7rSaftNd7ZUpXw2c3Ws44W2yMk 4FxIqfAyInK7a5ccswDP6sC6fxs8k0csSKhdqQyVfwiok5QLCzxJSt5sjPSrKzzwDdq/My1ugLOX7 R+yqeBVyGOwxgZyMmijw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p2wEI-005jWW-Vs; Wed, 07 Dec 2022 15:21:03 +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 1p2wEE-005jNj-T6 for linux-arm-kernel@lists.infradead.org; Wed, 07 Dec 2022 15:21:00 +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 7DE53615B8; Wed, 7 Dec 2022 15:20:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BF02FC433D6; Wed, 7 Dec 2022 15:20:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670426456; bh=iGaN2U0uRKGQpBqnByKOmCJ6pZP9qzB+JTsRvn+ssWo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=YgFL3rs3gAFvzyx4qHrhZHlUVBTYsmk2r4rjYuEbzgN+atMEP10DeBcPaGgI0SCRf 2Amq4RwPoerxLZcS33Pu2BoiAYhgLnpqdJhU6uJ2KBQ6FHjT62aL4JpXAHdrCpPMF2 6WxctuJVK7kBf3CDD0/UKY1Nu8OAhngSva5wsnjDW7HvcJZnEZxhsF9OR9HvdqQx+e 7tekeJW4bq/HMN7KO3y1IF8DbUpM2zEBapqBdVr+GbA9Okch3Q/Cez/yYGd9rntUm/ w3UWVOReOiEBFwptXBopLgcNMBqk/T2hbOCceANNOM09sTTkfgU7cO7bV41ZCRYLEZ Oxdq/4mcLqmkg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.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 1p2wEA-00B9Kk-BB; Wed, 07 Dec 2022 15:20:54 +0000 Date: Wed, 07 Dec 2022 15:20:53 +0000 Message-ID: <868rjjkzoq.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, kvmarm@lists.linux.dev Subject: Re: [PATCH v2 14/14] irqchip/gic-v3: Implement FEAT_GICv3_NMI support In-Reply-To: <20221112151708.175147-15-broonie@kernel.org> References: <20221112151708.175147-1-broonie@kernel.org> <20221112151708.175147-15-broonie@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 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 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, kvmarm@lists.linux.dev 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-20221207_072059_043518_2CCC11E3 X-CRM114-Status: GOOD ( 48.26 ) 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 Sat, 12 Nov 2022 15:17:08 +0000, Mark Brown wrote: > > From: Lorenzo Pieralisi > > The FEAT_GICv3_NMI GIC feature coupled with the CPU FEAT_NMI enables > handling NMI interrupts in HW on aarch64, by adding a superpriority > interrupt to the existing GIC priority scheme. > > Implement GIC driver support for the FEAT_GICv3_NMI feature. > > Rename gic_supports_nmi() helper function to gic_supports_pseudo_nmis() > to make the pseudo NMIs code path clearer and more explicit. Please make this particular change a separate patch. It will make it a lot clearer what is the added logic. And maybe drop the final 's' in gic_supports_pseudo_nmis. > > Check, through the ARM64 capabilitity infrastructure, if support > for FEAT_NMI was detected on the core and the system has not overridden > the detection and forced pseudo-NMIs enablement. > > If FEAT_NMI is detected, it was not overridden (check embedded in the > system_uses_nmi() call) and the GIC supports the FEAT_GICv3_NMI feature, > install an NMI handler and initialize NMIs related HW GIC registers. > > Signed-off-by: Lorenzo Pieralisi > Signed-off-by: Mark Brown > --- > drivers/irqchip/irq-gic-v3.c | 143 ++++++++++++++++++++++++----- > include/linux/irqchip/arm-gic-v3.h | 4 + > 2 files changed, 125 insertions(+), 22 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 34d58567b78d..dc45e1093e7b 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -54,6 +54,7 @@ struct gic_chip_data { > u32 nr_redist_regions; > u64 flags; > bool has_rss; > + bool has_nmi; > unsigned int ppi_nr; > struct partition_desc **ppi_descs; > }; > @@ -145,6 +146,20 @@ enum gic_intid_range { > __INVALID_RANGE__ > }; > > +#ifdef CONFIG_ARM64 > +#include > + > +static inline bool has_v3_3_nmi(void) For consistency, something along the lines of 'gic_supports_v3_3_nmi' would be better. And drop the inline which the compiler should be able to figure out on its own. Also consider placing all the arm64-special stuff under the same #define (we already have one for some ugly Cavium crap). > +{ > + return gic_data.has_nmi && system_uses_nmi(); > +} > +#else > +static inline bool has_v3_3_nmi(void) > +{ > + return false; > +} > +#endif > + > static enum gic_intid_range __get_intid_range(irq_hw_number_t hwirq) > { > switch (hwirq) { > @@ -350,6 +365,42 @@ static int gic_peek_irq(struct irq_data *d, u32 offset) > return !!(readl_relaxed(base + offset + (index / 32) * 4) & mask); > } > > +static DEFINE_RAW_SPINLOCK(irq_controller_lock); Move this up together with the rest of the static data. And maybe call it gic_nmi_lock so that we know what it protects. > + > +static void gic_irq_configure_nmi(struct irq_data *d, bool enable) > +{ > + void __iomem *base, *addr; > + u32 offset, index, mask, val; > + > + offset = convert_offset_index(d, GICD_INMIR, &index); > + mask = 1 << (index % 32); > + > + if (gic_irq_in_rdist(d)) > + base = gic_data_rdist_sgi_base(); > + else > + base = gic_data.dist_base; > + > + addr = base + offset + (index / 32) * 4; > + > + raw_spin_lock(&irq_controller_lock); > + > + val = readl_relaxed(addr); > + val = enable ? (val | mask) : (val & ~mask); If you make val an unsigned long, you can write this as: __assign_bit(index % 32, &val, enable); and then you can drop the mask. > + writel_relaxed(val, addr); > + > + raw_spin_unlock(&irq_controller_lock); > +} > + > +static void gic_irq_enable_nmi(struct irq_data *d) > +{ > + gic_irq_configure_nmi(d, true); > +} > + > +static void gic_irq_disable_nmi(struct irq_data *d) > +{ > + gic_irq_configure_nmi(d, false); > +} > + > static void gic_poke_irq(struct irq_data *d, u32 offset) > { > void __iomem *base; > @@ -395,7 +446,7 @@ static void gic_unmask_irq(struct irq_data *d) > gic_poke_irq(d, GICD_ISENABLER); > } > > -static inline bool gic_supports_nmi(void) > +static inline bool gic_supports_pseudo_nmis(void) > { > return IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && > static_branch_likely(&supports_pseudo_nmis); > @@ -491,7 +542,7 @@ static int gic_irq_nmi_setup(struct irq_data *d) > { > struct irq_desc *desc = irq_to_desc(d->irq); > > - if (!gic_supports_nmi()) > + if (!gic_supports_pseudo_nmis() && !has_v3_3_nmi()) > return -EINVAL; > > if (gic_peek_irq(d, GICD_ISENABLER)) { > @@ -519,7 +570,10 @@ static int gic_irq_nmi_setup(struct irq_data *d) > desc->handle_irq = handle_fasteoi_nmi; > } > > - gic_irq_set_prio(d, GICD_INT_NMI_PRI); > + if (has_v3_3_nmi()) > + gic_irq_enable_nmi(d); > + else > + gic_irq_set_prio(d, GICD_INT_NMI_PRI); > > return 0; > } > @@ -528,7 +582,7 @@ static void gic_irq_nmi_teardown(struct irq_data *d) > { > struct irq_desc *desc = irq_to_desc(d->irq); > > - if (WARN_ON(!gic_supports_nmi())) > + if (WARN_ON(!gic_supports_pseudo_nmis() && !has_v3_3_nmi())) > return; > > if (gic_peek_irq(d, GICD_ISENABLER)) { > @@ -554,7 +608,10 @@ static void gic_irq_nmi_teardown(struct irq_data *d) > desc->handle_irq = handle_fasteoi_irq; > } > > - gic_irq_set_prio(d, GICD_INT_DEF_PRI); > + if (has_v3_3_nmi()) > + gic_irq_disable_nmi(d); > + else > + gic_irq_set_prio(d, GICD_INT_DEF_PRI); > } > > static void gic_eoi_irq(struct irq_data *d) > @@ -674,7 +731,7 @@ static inline void gic_complete_ack(u32 irqnr) > > static bool gic_rpr_is_nmi_prio(void) > { > - if (!gic_supports_nmi()) > + if (!gic_supports_pseudo_nmis()) > return false; > > return unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(GICD_INT_NMI_PRI)); > @@ -706,7 +763,8 @@ static void __gic_handle_nmi(u32 irqnr, struct pt_regs *regs) > gic_complete_ack(irqnr); > > if (generic_handle_domain_nmi(gic_data.domain, irqnr)) { > - WARN_ONCE(true, "Unexpected pseudo-NMI (irqnr %u)\n", irqnr); > + WARN_ONCE(true, "Unexpected %sNMI (irqnr %u)\n", > + gic_supports_pseudo_nmis() ? "pseudo-" : "", irqnr); > gic_deactivate_unhandled(irqnr); > } > } > @@ -782,9 +840,37 @@ static void __gic_handle_irq_from_irqsoff(struct pt_regs *regs) > __gic_handle_nmi(irqnr, regs); > } > > +#ifdef CONFIG_ARM64 > +static inline u64 gic_read_nmiar(void) > +{ > + u64 irqstat; > + > + irqstat = read_sysreg_s(SYS_ICC_NMIAR1_EL1); > + > + dsb(sy); > + > + return irqstat; > +} > + > +static asmlinkage void __exception_irq_entry gic_handle_nmi_irq(struct pt_regs *regs) I think this asmlinkage has been cargo-culted for a long time, and isn't relevant anymore, as we don't get here directly from some assembler code. > +{ > + u32 irqnr = gic_read_nmiar(); The only reason we indirect reads of IAR are for the sake of AArch32. Since we don't support NMIs for this architecture, and that this code is entirely behind a #ifdef, just inline the read of NMIAIR1_EL1 here. > + > + __gic_handle_nmi(irqnr, regs); > +} > + > +static inline void gic_setup_nmi_handler(void) > +{ > + if (has_v3_3_nmi()) > + set_handle_nmi_irq(gic_handle_nmi_irq); > +} > +#else > +static inline void gic_setup_nmi_handler(void) { } > +#endif > + > static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs) > { > - if (unlikely(gic_supports_nmi() && !interrupts_enabled(regs))) > + if (unlikely(gic_supports_pseudo_nmis() && !interrupts_enabled(regs))) > __gic_handle_irq_from_irqsoff(regs); > else > __gic_handle_irq_from_irqson(regs); > @@ -1072,7 +1158,7 @@ static void gic_cpu_sys_reg_init(void) > /* Set priority mask register */ > if (!gic_prio_masking_enabled()) { > write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1); > - } else if (gic_supports_nmi()) { > + } else if (gic_supports_pseudo_nmis()) { > /* > * Mismatch configuration with boot CPU, the system is likely > * to die as interrupt masking will not work properly on all > @@ -1753,20 +1839,8 @@ static const struct gic_quirk gic_quirks[] = { > } > }; > > -static void gic_enable_nmi_support(void) > +static void gic_enable_pseudo_nmis(void) > { > - int i; > - > - if (!gic_prio_masking_enabled()) > - return; > - > - ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL); > - if (!ppi_nmi_refs) > - return; > - > - 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 > @@ -1809,6 +1883,28 @@ static void gic_enable_nmi_support(void) > static_branch_enable(&gic_nonsecure_priorities); > > static_branch_enable(&supports_pseudo_nmis); > +} > + > +static void gic_enable_nmi_support(void) > +{ > + int i; > + > + if (!gic_prio_masking_enabled() && !has_v3_3_nmi()) > + return; > + > + ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL); > + if (!ppi_nmi_refs) > + return; > + > + for (i = 0; i < gic_data.ppi_nr; i++) > + refcount_set(&ppi_nmi_refs[i], 0); > + > + /* > + * Initialize pseudo-NMIs only if GIC driver cannot take advantage > + * of core (FEAT_NMI) and GIC (FEAT_GICv3_NMI) in HW > + */ > + if (!has_v3_3_nmi()) > + gic_enable_pseudo_nmis(); > > if (static_branch_likely(&supports_deactivate_key)) > gic_eoimode1_chip.flags |= IRQCHIP_SUPPORTS_NMI; > @@ -1872,6 +1968,7 @@ static int __init gic_init_bases(void __iomem *dist_base, > irq_domain_update_bus_token(gic_data.domain, DOMAIN_BUS_WIRED); > > gic_data.has_rss = !!(typer & GICD_TYPER_RSS); > + gic_data.has_nmi = !!(typer & GICD_TYPER_NMI); > > if (typer & GICD_TYPER_MBIS) { > err = mbi_init(handle, gic_data.domain); > @@ -1881,6 +1978,8 @@ static int __init gic_init_bases(void __iomem *dist_base, > > set_handle_irq(gic_handle_irq); > > + gic_setup_nmi_handler(); > + > gic_update_rdist_properties(); > > gic_dist_init(); > diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > index 728691365464..3306456c135f 100644 > --- a/include/linux/irqchip/arm-gic-v3.h > +++ b/include/linux/irqchip/arm-gic-v3.h > @@ -30,6 +30,7 @@ > #define GICD_ICFGR 0x0C00 > #define GICD_IGRPMODR 0x0D00 > #define GICD_NSACR 0x0E00 > +#define GICD_INMIR 0x0F80 > #define GICD_IGROUPRnE 0x1000 > #define GICD_ISENABLERnE 0x1200 > #define GICD_ICENABLERnE 0x1400 > @@ -39,6 +40,7 @@ > #define GICD_ICACTIVERnE 0x1C00 > #define GICD_IPRIORITYRnE 0x2000 > #define GICD_ICFGRnE 0x3000 > +#define GICD_INMIRnE 0x3B00 > #define GICD_IROUTER 0x6000 > #define GICD_IROUTERnE 0x8000 > #define GICD_IDREGS 0xFFD0 > @@ -83,6 +85,7 @@ > #define GICD_TYPER_LPIS (1U << 17) > #define GICD_TYPER_MBIS (1U << 16) > #define GICD_TYPER_ESPI (1U << 8) > +#define GICD_TYPER_NMI (1U << 9) > > #define GICD_TYPER_ID_BITS(typer) ((((typer) >> 19) & 0x1f) + 1) > #define GICD_TYPER_NUM_LPIS(typer) ((((typer) >> 11) & 0x1f) + 1) > @@ -238,6 +241,7 @@ > #define GICR_ICFGR0 GICD_ICFGR > #define GICR_IGRPMODR0 GICD_IGRPMODR > #define GICR_NSACR GICD_NSACR > +#define GICR_INMIR0 GICD_INMIR > > #define GICR_TYPER_PLPIS (1U << 0) > #define GICR_TYPER_VLPIS (1U << 1) Otherwise looks reasonable. 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