Received: by 2002:a05:6500:1b45:b0:1f5:f2ab:c469 with SMTP id cz5csp718248lqb; Wed, 17 Apr 2024 08:50:45 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCWfjEocDPrqgIgYcc0eVxzjrI0qWLMPSW/Yr0lBqLMdiIIw4bn1aVH8O+yOJrK/UAeMMGHNCwOsEsDDS56DJdsFtD9l9Tucz6Cy3kpg6A== X-Google-Smtp-Source: AGHT+IGdQV8quZbwjEuROMEgCUCjg1zeSRm7HgDBLRyFyJDtj2SWQMJpgkscdEzDVpCPvDidglvZ X-Received: by 2002:a17:907:7e94:b0:a51:e5ac:7b7 with SMTP id qb20-20020a1709077e9400b00a51e5ac07b7mr12421748ejc.71.1713369045426; Wed, 17 Apr 2024 08:50:45 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1713369045; cv=pass; d=google.com; s=arc-20160816; b=QKrHloz9fpZT+rhdH4XI2RwDbNRVYF+lxy/Aq2zR75Cl/Y7BQDhYMP6+7h4Kt/5rLU 8vg3WmJI4VADJXTb9VDpLKtjuNqiCLmhg6zBJcz9F3EJTwXBaT1/xS86m+T74ud6xJoZ XbSjviqf9lqcmEd06x/AiRU38daclDfYam2d+tGtcFdk60LvYx18OMXEIELwyuFt1EL7 X0WIKFxzXzN7Ctalh88uiadjCBiJy11Ka7QTD0YEO9pXobt/Z5czkuytBlmNpfzDyuXw AmdBfaINmFJGuOT25uIBqUwqGw7Ybtis9Q/7Av/KhA+min/auKBevAGsAqvoYtLiWOxT EHKA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:organization:references :in-reply-to:message-id:subject:cc:to:from:date; bh=AwaMYVtBZ1TrFq6Qf4kpihlv3ADSDJOpP7dnmOKUu6o=; fh=raVXkpv41GKuJfobPM/EsdqWgAnM8RgwHspirpbY5wI=; b=R2KZukUn53hJMgy32owY8RMbERBTgYnmDG3rQ6Bp7/vs7p+Ztpyvk0tU40aw1K3O0P V/9WP9z+dc1fPcORAzWBPGSSmAoIEk4QLxam/oqQSmdX06A/jlEWTcWO8ssRxvi7IAWw ERaEvs0ucNtUB+rAzCdfrZe/ejb+GIO61pmQgA/fepraFM3rPI+1xtxHANb0y0Hnnwa/ VMGbASJVVUznX3ynwYbesGwJOHGXQ8p0REI8PgVbHs7T+qljweyIkR3MhcH/E4WAS6PW fYOnoW1lggj7+7zg7eCx2C1+GHv2Qewcx/lpZHxXisBoEmHJDiej3syGsATQ8oXI3HdK IoyQ==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=huawei.com dmarc=pass fromdomain=huawei.com); spf=pass (google.com: domain of linux-kernel+bounces-148834-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-148834-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id t27-20020a170906179b00b00a523af47254si5510035eje.446.2024.04.17.08.50.45 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Apr 2024 08:50:45 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-148834-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=huawei.com dmarc=pass fromdomain=huawei.com); spf=pass (google.com: domain of linux-kernel+bounces-148834-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-148834-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 0040A1F27055 for ; Wed, 17 Apr 2024 15:40:51 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 9F5A314883D; Wed, 17 Apr 2024 15:38:50 +0000 (UTC) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D13DD1487F1; Wed, 17 Apr 2024 15:38:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.176.79.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713368329; cv=none; b=UwGM78zlH1SFT0qa/RkCosQinq/sg2+bUsLzNJu2G/pGW93bb4hXIkAn3QihgsD6ppU6WEeVp7m0coVHBWHBPZG13f5Jl7Mwz8c/mt9Vkzj2P0jwlcBF2mAJPyjZlDxgSExhMZCSNNhuLpLpvToAuMxTV4J9ddSTP0rR2t5uKQA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713368329; c=relaxed/simple; bh=GEwKcew9dkrK7al83zqmzbXTl8DFGOiIto9YUHLUKgI=; h=Date:From:To:CC:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=AENnMVv308UNu3yM9loaH62fdAS9vBwj0Nb6Rujzxr+QEZdJ1vWkWBdqGoLQxWTCv4xFFJLg6LF6kuiHXuAl0PH9ATok8lCfZMQHaGHedqhrsizK6+4NJKKZyYP2FW+iWN2LasFH9QE9aPxRNFJtPo3GIdxSeYsDvc412AqA/XA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=Huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=185.176.79.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=Huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4VKQ4W5nkBz6F951; Wed, 17 Apr 2024 23:36:43 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 35D8B140B38; Wed, 17 Apr 2024 23:38:44 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Wed, 17 Apr 2024 16:38:43 +0100 Date: Wed, 17 Apr 2024 16:38:42 +0100 From: Jonathan Cameron To: Salil Mehta CC: Thomas Gleixner , Peter Zijlstra , "linux-pm@vger.kernel.org" , "loongarch@lists.linux.dev" , "linux-acpi@vger.kernel.org" , "linux-arch@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "kvmarm@lists.linux.dev" , "x86@kernel.org" , Russell King , "Rafael J . Wysocki" , "Miguel Luis" , James Morse , Jean-Philippe Brucker , Catalin Marinas , Will Deacon , Ingo Molnar , Borislav Petkov , Dave Hansen , Linuxarm , "justin.he@arm.com" , "jianyong.wu@arm.com" Subject: Re: [PATCH v6 06/16] ACPI: processor: Register deferred CPUs from acpi_processor_get_info() Message-ID: <20240417163842.0000415e@Huawei.com> In-Reply-To: <22ace9b108ee488eb017f5b3e8facb8d@huawei.com> References: <20240417131909.7925-1-Jonathan.Cameron@huawei.com> <20240417131909.7925-7-Jonathan.Cameron@huawei.com> <22ace9b108ee488eb017f5b3e8facb8d@huawei.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: lhrpeml100004.china.huawei.com (7.191.162.219) To lhrpeml500005.china.huawei.com (7.191.163.240) On Wed, 17 Apr 2024 16:03:51 +0100 Salil Mehta wrote: > > From: Jonathan Cameron > > Sent: Wednesday, April 17, 2024 2:19 PM > > > > From: James Morse > > > > The arm64 specific arch_register_cpu() call may defer CPU registration until > > the ACPI interpreter is available and the _STA method can be evaluated. > > > > If this occurs, then a second attempt is made in acpi_processor_get_info(). > > Note that the arm64 specific call has not yet been added so for now this will > > be called for the original hotplug case. > > > > For architectures that do not defer until the ACPI Processor driver loads > > (e.g. x86), for initially present CPUs there will already be a CPU device. If > > present do not try to register again. > > > > Systems can still be booted with 'acpi=off', or not include an ACPI > > description at all as in these cases arch_register_cpu() will not have > > deferred registration when first called. > > > > This moves the CPU register logic back to a subsys_initcall(), while the > > memory nodes will have been registered earlier. > > Note this is where the call was prior to the cleanup series so there should be > > no side effects of moving it back again for this specific case. > > > > [PATCH 00/21] Initial cleanups for vCPU HP. > > https://lore.kernel.org/all/ZVyz%2FVe5pPu8AWoA@shell.armlinux.org.uk/ > > > > e.g. 5b95f94c3b9f ("x86/topology: Switch over to GENERIC_CPU_DEVICES") > > > > Signed-off-by: James Morse > > Reviewed-by: Gavin Shan > > Tested-by: Miguel Luis > > Tested-by: Vishnu Pajjuri > > Tested-by: Jianyong Wu > > Signed-off-by: Russell King (Oracle) > > Co-developed-by: Jonathan Cameron > > Signed-off-by: Joanthan Cameron > > --- > > v6: Squash the two paths for conventional CPU Hotplug and arm64 > > vCPU HP. > > v5: Update commit message to make it clear this is moving the > > init back to where it was until very recently. > > > > No longer change the condition in the earlier registration point > > as that will be handled by the arm64 registration routine > > deferring until called again here. > > --- > > drivers/acpi/acpi_processor.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c > > index 7ecb13775d7f..0cac77961020 100644 > > --- a/drivers/acpi/acpi_processor.c > > +++ b/drivers/acpi/acpi_processor.c > > @@ -356,8 +356,18 @@ static int acpi_processor_get_info(struct > > acpi_device *device) > > * > > * NOTE: Even if the processor has a cpuid, it may not be present > > * because cpuid <-> apicid mapping is persistent now. > > + * > > + * Note this allows 3 flows, it is up to the arch_register_cpu() > > + * call to reject any that are not supported on a given architecture. > > + * A) CPU becomes present. > > + * B) Previously invalid logical CPU ID (Same as becoming present) > > + * C) CPU already present and now being enabled (and wasn't > > registered > > + * early on an arch that doesn't defer to here) > > */ > > - if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id)) { > > + if ((!invalid_logical_cpuid(pr->id) && cpu_present(pr->id) && > > + !get_cpu_device(pr->id)) || > > + invalid_logical_cpuid(pr->id) || > > + !cpu_present(pr->id)) { > > Hi Salil, Thanks for quick review! > Logic is clear but it is ugly. We should turn them into macro or inline. You've found the 'ugly' in this approach vs keeping them separate. For this version I wanted to keep it clear that indeed this condition is a complex mess of different things (and to let people compare it easily with the two paths in v5 to convinced themselves this is the same) It's also a little tricky to do, so will need some thought. I don't think a simple acpi_cpu_is_hotplug() condition is useful as it just moves the complexity away from where a reader is looking and it would only be used in this one case. It doesn't separate well into finer grained subconditions because (C) is a messy case of the vCPU HP case and a not done something else earlier. The disadvantage of only deferring for arm64 and not other architectures. The best I can quickly come up with is something like this: #define acpi_cpu_not_present(cpu) \ (invalid_logical_cpuid(cpu) || !cpu_present(cpu)) #define acpi_cpu_not_enabled(cpu) \ (!invalid_logical_cpuid(cpu) || cpu_present(cpu)) if ((apci_cpu_not_enabled(pr->id) && !get_cpu_device(pr->id) || acpi_cpu_not_present(pr->id)) Which would still need the same amount of documentation. The code still isn't enough for me to immediately be able to see what is going on. So maybe worth it... I'm not sure. Rafael, you get to keep this fun, what would you prefer? Jonathan > > > Thanks > Salil.