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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 50C01C433EF for ; Sat, 23 Oct 2021 10:37:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 30DD861056 for ; Sat, 23 Oct 2021 10:37:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230359AbhJWKkJ (ORCPT ); Sat, 23 Oct 2021 06:40:09 -0400 Received: from mail.kernel.org ([198.145.29.99]:34778 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229721AbhJWKkI (ORCPT ); Sat, 23 Oct 2021 06:40:08 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 52B8460ED5; Sat, 23 Oct 2021 10:37:49 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1meEPL-0014PX-0U; Sat, 23 Oct 2021 11:37:47 +0100 Date: Sat, 23 Oct 2021 11:37:46 +0100 Message-ID: <87o87gt4at.wl-maz@kernel.org> From: Marc Zyngier To: Valentin Schneider Cc: linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Will Deacon , Mark Rutland , Thomas Gleixner , Sebastian Andrzej Siewior , Ard Biesheuvel Subject: Re: [PATCH 3/3] irqchip/gic-v3-its: Limit memreserve cpuhp state lifetime In-Reply-To: <20211022103307.1711619-4-valentin.schneider@arm.com> References: <20211022103307.1711619-1-valentin.schneider@arm.com> <20211022103307.1711619-4-valentin.schneider@arm.com> 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 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: valentin.schneider@arm.com, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, will@kernel.org, mark.rutland@arm.com, tglx@linutronix.de, bigeasy@linutronix.de, ardb@kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 22 Oct 2021 11:33:07 +0100, Valentin Schneider wrote: > > The new memreserve cpuhp callback only needs to survive up until a point > where every CPU in the system has booted once. Beyond that, it becomes a > no-op and can be put in the bin. > > Signed-off-by: Valentin Schneider > --- > drivers/irqchip/irq-gic-v3-its.c | 23 ++++++++++++++++++++--- > include/linux/irqchip/arm-gic-v3.h | 1 + > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index a6a4af59205e..4ae9ae6b90fe 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -5206,6 +5206,15 @@ int its_cpu_init(void) > } > > #ifdef CONFIG_EFI > +static void rdist_memreserve_cpuhp_cleanup_workfn(struct work_struct *work) > +{ > + cpuhp_remove_state_nocalls(gic_rdists->cpuhp_memreserve_state); > + gic_rdists->cpuhp_memreserve_state = CPUHP_INVALID; > +} > + > +static DECLARE_WORK(rdist_memreserve_cpuhp_cleanup_work, > + rdist_memreserve_cpuhp_cleanup_workfn); > + > static int its_cpu_memreserve_lpi(unsigned int cpu) > { > struct page *pend_page = gic_data_rdist()->pend_page; > @@ -5226,7 +5235,7 @@ static int its_cpu_memreserve_lpi(unsigned int cpu) > * invocation of this callback, or in a previous life before kexec. > */ > if (gic_data_rdist()->flags & RDIST_FLAGS_PENDTABLE_RESERVED) > - return 0; > + goto out; > > gic_data_rdist()->flags |= RDIST_FLAGS_PENDTABLE_RESERVED; > > @@ -5234,6 +5243,11 @@ static int its_cpu_memreserve_lpi(unsigned int cpu) > paddr = page_to_phys(pend_page); > WARN_ON(gic_reserve_range(paddr, LPI_PENDBASE_SZ)); > > +out: > + /* This only needs to run once per CPU */ > + if (cpumask_equal(&cpus_booted_once_mask, cpu_possible_mask)) > + schedule_work(&rdist_memreserve_cpuhp_cleanup_work); Which makes me wonder. Do we actually need any flag at all if all we need to check is whether the CPU has been through the callback at least once? I have the strong feeling that we are tracking the same state multiple times here. Also, could the cpuhp callbacks ever run concurrently? If they could, two CPUs could schedule the cleanup work in parallel, with interesting results. You'd need a cmpxchg on the cpuhp state in the workfn. > + > return 0; > } > #endif > @@ -5421,13 +5435,14 @@ static void __init its_acpi_probe(void) > static void __init its_acpi_probe(void) { } > #endif > > -static int __init its_lpi_memreserve_init(void) > +static int __init its_lpi_memreserve_init(struct rdists *rdists) > { > int state; > > if (!efi_enabled(EFI_CONFIG_TABLES)) > return 0; > > + rdists->cpuhp_memreserve_state = CPUHP_INVALID; > state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, > "irqchip/arm/gicv3/memreserve:online", > its_cpu_memreserve_lpi, > @@ -5435,6 +5450,8 @@ static int __init its_lpi_memreserve_init(void) > if (state < 0) > return state; > > + rdists->cpuhp_memreserve_state = state; > + > return 0; > } > > @@ -5465,7 +5482,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists, > if (err) > return err; > > - err = its_lpi_memreserve_init(); > + err = its_lpi_memreserve_init(rdists); > if (err) > return err; > > diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > index 0dc34d7d735a..95479b315918 100644 > --- a/include/linux/irqchip/arm-gic-v3.h > +++ b/include/linux/irqchip/arm-gic-v3.h > @@ -624,6 +624,7 @@ struct rdists { > u64 flags; > u32 gicd_typer; > u32 gicd_typer2; > + int cpuhp_memreserve_state; > bool has_vlpis; > bool has_rvpeid; > bool has_direct_lpi; Thanks, M. -- Without deviation from the norm, progress is not possible.