Received-SPF: pass (google.com: domain of linux-kernel+bounces-106698-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 18 Mar 2024 14:26:41 -0700
From: Dan Williams <dan.j.williams@intel.com>
To: Robert Richter <rrichter@amd.com>, "Rafael J. Wysocki"
	<rafael@kernel.org>, Alison Schofield <alison.schofield@intel.com>, "Dan
 Williams" <dan.j.williams@intel.com>, Dave Hansen
	<dave.hansen@linux.intel.com>
CC: <linux-acpi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-cxl@vger.kernel.org>, Robert Richter <rrichter@amd.com>, Derick Marks
	<derick.w.marks@intel.com>, Len Brown <lenb@kernel.org>
Subject: Re: [PATCH 1/3] x86/numa: Fix SRAT lookup for CFMWS ranges with
 numa_fill_memblks()
Message-ID: <65f8b191c0422_aa222941b@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240318210904.2188120-1-rrichter@amd.com>
 <20240318210904.2188120-2-rrichter@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240318210904.2188120-2-rrichter@amd.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?HjbnuLsYH2i1QKk1ahJmF+qntBbljbzKEtyVw0QOTeOvhkyLVFNO7JuW4QHT?=
 =?us-ascii?Q?rAoATsHhrrDvmLPjSXVkE66irHFLKe6tPeB9Mk8J0OjOJxd5ViKcGkHCzgrI?=
 =?us-ascii?Q?nh/fEOyOI6C1Oe4llJgkcQL9lPaH+grH3S5iWKtw21yWusk2l7XhHB7s+mgX?=
 =?us-ascii?Q?IGypCYlVO8uzBxASV6+x3l0E23xosiroyB2UwfDM9/F/lNyP4NMmxTH5nJ6X?=
 =?us-ascii?Q?xy+XPIzEHu9iEisJiWkB6YDkdAZA3uPxMOchJ29GPR/wLFbMfYh8IXjg+pbr?=
 =?us-ascii?Q?Q3PNjfAwSt00L5O2ifudStA5HZiYy5N820S0lAIMBpHxf6SQkTedU1pkwvuE?=
 =?us-ascii?Q?0e2UE5ZkpQJ5aNcONBZIDNAGhfNNVHmWMi4kmK59xx6HBYQVArhPwwnZCRNG?=
 =?us-ascii?Q?nA9dM19z2W6ZaKVs21CUJSFgFzB6sVySBBKZztSAky+T06Qgosvi3XVBTvmk?=
 =?us-ascii?Q?k3KlosIYH8QeDySKchHyDJRSgHHsbEoarbgt3bskBuM29mv8a0oNkSNO5BTZ?=
 =?us-ascii?Q?1ogORuiooBQ/8oxCgx80xg+EA517m316ePd4rNADRuCegXwCDSo3DDWURyGO?=
 =?us-ascii?Q?cS322TFjSxCJ8mMX6yNdbh6MLeaiAzSRhPRaO8XKE48OAdn8x0otUhqRBh3c?=
 =?us-ascii?Q?mQjHvVZh+LBZqLJoOtojqueWkF3qrLoZ0q6cWrDazfRSvKuzV6rA09CjaTWl?=
 =?us-ascii?Q?NGwDFRwEy7g8Hecd9Hb3BzR/BBLTODllfAg6p/dY/TPJoR2mGffR7btD80PW?=
 =?us-ascii?Q?GL1q8F/Z5FO+beQf3ELDK4fH23VUBkEP5qLY2KqsUWuZx66kimiQsAux2hw1?=
 =?us-ascii?Q?apjCpBYlklFSg/sEUJcMuv096dry1drMk9Pk8E0HrcRojAICybfHd3nRq+P+?=
 =?us-ascii?Q?m7P8Apl99E3Orql5/yRUNwjQgT4ogiqJRPC72QW1xNLNNlnh/kmIDr6zGEyE?=
 =?us-ascii?Q?9oczrhZguKI9E5s+aEQz/LDie39zpbx1AyNGMYzsRj4SST0MGYzea5Ult7CE?=
 =?us-ascii?Q?ikrcaClh2+Alsv5gL7u54mYNSnH44cpwTzN3Gq1UtzxQk4HXCCIBkfZOiD4I?=
 =?us-ascii?Q?O5MbN73p0hnWtyZQEIbJ9h68k0UTqN0Tgno+9iINQemMKDwMyKxV/p0dcw2Q?=
 =?us-ascii?Q?V/2lkO3AeYdT6EjKSpTAYRS3YJNNT3XhHSJPkwvrD0cVHC/gKS2Rpe9n1fuj?=
 =?us-ascii?Q?QYH2rlfDQNHEeTPw7H/+Qv6493qwJ5rI4FXbdASqlLbre2Gf2YsnlUnB+iZW?=
 =?us-ascii?Q?Jeglhegh7dF/4LSI3yj5uNF/8nk/wZv3SCLjGSOf5oSzJAmFywwVda+uVzwS?=
 =?us-ascii?Q?O4LfrZOv+fsVqyN8usX13gugZ0cRxIhR5RK5baPZVI7uyntp1+47LTne3h3R?=
 =?us-ascii?Q?AY+iQfhlwzhrRc7F+ZZFzNjKx7Q6aFDvKB6BxHCgMQIVGHsuSk/MsBdc5W0h?=
 =?us-ascii?Q?kVdq7KlF9iLluk1/EpZqNzTmZ4CFtxnYEJmQwhEAv9JurrAlzN/RaYBQx3ef?=
 =?us-ascii?Q?bEm2Mvwag76OhoaitgYTldhm2RuGHATFtCRE7eNEHGc6J7rJFUPwvaW2k37i?=
 =?us-ascii?Q?1+RRHM82OpwjrbagzZTGxs5zQ3ypkKAsDLEo7q5zBeXzRPjNgJGyRKRieBvX?=
 =?us-ascii?Q?Sg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 46a762dd-a67e-471a-89eb-08dc47921c22
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Mar 2024 21:26:44.4449
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: u8H8m4WwePvii6ho9o+BwNJJ9LLFTHoPBN9eZrUMmqnAvsKps6NUUTx7Uxwd7zcLralXplmv+a/2/L2M/GPcHTvCCF8L9Z8kV1YDicGWGKY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB7152
X-OriginatorOrg: intel.com

Robert Richter wrote:
> With kconfig option NUMA_KEEP_MEMINFO disabled the SRAT lookup done
> with numa_fill_memblks() fails returning NUMA_NO_MEMBLK (-1). An
> existing SRAT memory range cannot be found for a CFMWS address range.
> This causes the addition of a duplicate numa_memblk with a different
> node id and a subsequent page fault and kernel crash during boot.
> 
> Note that the issue was initially introduced with [1]. But since
> phys_to_target_node() was originally used that returned the valid node
> 0, an additional numa_memblk was not added. Though, the node id was
> wrong too.
> 
> Fix this by enabling NUMA_KEEP_MEMINFO for x86 with ACPI and NUMA
> enabled.
> 
> [1] fd49f99c1809 ("ACPI: NUMA: Add a node and memblk for each CFMWS not in SRAT")
> 
> Fixes: 8f1004679987 ("ACPI/NUMA: Apply SRAT proximity domain to entire CFMWS window")
> Cc: Derick Marks <derick.w.marks@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/acpi/numa/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/numa/Kconfig b/drivers/acpi/numa/Kconfig
> index 849c2bd820b9..2f4ac6ac6768 100644
> --- a/drivers/acpi/numa/Kconfig
> +++ b/drivers/acpi/numa/Kconfig
> @@ -3,6 +3,7 @@ config ACPI_NUMA
>  	bool "NUMA support"
>  	depends on NUMA
>  	depends on (X86 || ARM64 || LOONGARCH)
> +	select NUMA_KEEP_MEMINFO if X86
>  	default y if ARM64

A fix is needed, yes, but this is the wrong one. NUMA_KEEP_MEMINFO is
only about marking numa_meminfo data as not "__init". Since
numa_fill_memblks() *is* an __init function, it should have no
dependency on NUMA_KEEP_MEMINFO.

The fix here involves moving the definition of numa_fill_memblks() out
of the "#ifdef CONFIG_NUMA_KEEP_MEMINFO" in
arch/x86/include/asm/sparsemem.h so that it does not fallback to the
default definition in include/linux/numa.h.

It should also be the case that cxl_acpi needs this:

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 67998dbd1d46..1bf25185c35b 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -6,6 +6,7 @@ menuconfig CXL_BUS
        select FW_UPLOAD
        select PCI_DOE
        select FIRMWARE_TABLE
+       select NUMA_KEEP_MEMINFO if NUMA
        help
          CXL is a bus that is electrically compatible with PCI Express, but
          layers three protocols on that signalling (CXL.io, CXL.cache, and