Received-SPF: pass (google.com: domain of linux-kernel+bounces-108119-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 19 Mar 2024 13:44:53 -0700
From: Dan Williams <dan.j.williams@intel.com>
To: Robert Richter <rrichter@amd.com>, "Rafael J. Wysocki" <rafael@kernel.org>
CC: Dave Hansen <dave.hansen@linux.intel.com>, Dan Williams
	<dan.j.williams@intel.com>, Alison Schofield <alison.schofield@intel.com>,
	<linux-acpi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-cxl@vger.kernel.org>, Robert Richter <rrichter@amd.com>, Len Brown
	<lenb@kernel.org>
Subject: RE: [PATCH v2 3/3] ACPI/NUMA: Remove architecture dependent
 remainings
Message-ID: <65f9f9452dd50_7702a294bb@dwillia2-xfh.jf.intel.com.notmuch>
References: <20240319120026.2246389-1-rrichter@amd.com>
 <20240319120026.2246389-4-rrichter@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240319120026.2246389-4-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?slYuOTP/OBuccQbxz+cX1QQDw3hEz6H6nQkSH7HjDSaZngcFTYAi7cIzilpn?=
 =?us-ascii?Q?qv4J1TxtvELhrM3C6iP8njRAoE9+UUnAsKWGuZtj7XRAHjg7MNuLV6At5KWK?=
 =?us-ascii?Q?tMNxLJTLimOLppgYjSzMwTFMZBqpvvTc4nAwGrDYFSN3l/eRfj7IowYckLA1?=
 =?us-ascii?Q?UIhJCAGPk6TrlFdIa4DURusQdU5TkeVuAx6uSf3t43db+rCDyIxQB0cUP5qB?=
 =?us-ascii?Q?cknEsTD7B3mgOpslXojfLewDI0h7QKUPYNai0+hXVlTadNk/KcKO35Vf27Qx?=
 =?us-ascii?Q?kxOIAIDh762mHag6APoTK5melN6JzXlgHJK4uM7ClDuJfWSog7fNuAmeb+5o?=
 =?us-ascii?Q?054CtRtoJYoHlM0/VsMrmuAHl0+fTPuev6cPTpLq2sW8i1xKk2fv9D/gbRt3?=
 =?us-ascii?Q?Jid23ftnvYl7oxI/2fLmnrHXOeyNbbX5ugYNex9HIafJ6kS7gTFsmE297WcK?=
 =?us-ascii?Q?Tfc7jDMjBg/FV2Aoknil2cJKV89pu5UZbPa8qK2M/Ax5oFYvokmrnWdUrIn4?=
 =?us-ascii?Q?heKswVFTW+WkRWMsDnMQ/bNsulCwKbqnNoFVYdr5xcJG9GERD5n2IbRN0Gfo?=
 =?us-ascii?Q?TweiQaLz6wpAXeEqClpdKclwI2wpmBZ5l12ipEK7mTGw+ZbDYiT3xRt8/kpg?=
 =?us-ascii?Q?+uk8xU2Q6Zq8evM1fRYc19IIMmh0WQ7M4tR+5E8iszaX1+MVC2BuKs1YrzMU?=
 =?us-ascii?Q?FZJb3DVin1Z8Wb7OHgo91g1R6pBssY+iYMs+dCtasrrHjL7DUa+U9y90H99A?=
 =?us-ascii?Q?a/TN+aYXh6Wd/77WjJV/jLDnqAWsnZNiM1nQTsUfqKhGBo/oX7BENpZgsWq2?=
 =?us-ascii?Q?n2ttPpv2KYpYc/4ZMHJuUrXJJWgAh6SEOTey0jsatIFRLUD6UdvhVPyPpc69?=
 =?us-ascii?Q?Ju+jX9Z88imW1oWkBQwOHcerXTuUrHHPUxJ4KJPjeFjg75VHWqZUGJ9yCdxP?=
 =?us-ascii?Q?2te3maP0zLllhfrgr5t/MCO3bcGCP9XnSDeo0BH6sMpwHOP4QSHOumMDakBb?=
 =?us-ascii?Q?Wc4Jiw1g8AmhdRdDJNMEwyqW45Y7y/VgxUV3BBJEeGLt5c9frG5ZxWLUq5uB?=
 =?us-ascii?Q?/LzLWSRc4cZpRWbwHMjHJrC+RNSRXGImm2qoFHmxwLgaT0dgAyivJGwagUHr?=
 =?us-ascii?Q?KmrFDXRM5wFlWJllKNrIJnti2xdNY82RRY9QUzblwEZwcwSXHBRFhZwFSco2?=
 =?us-ascii?Q?HazR6WjxLO8bSwgQA6YsmZg6WTRnIn94UkA18xHcMOkgunYW92+46BCtzuOs?=
 =?us-ascii?Q?yWT0fGSoidZQZJf/KJ/bZaE3XGagZ2fqiNJkOMpHiAUr9iZgQDTiCGoYbG8E?=
 =?us-ascii?Q?r1Jy71k5TNR8u3EA+SiNpSBdFY1R8mAv2GfpXCqUTlA9dn7FZROBn2FM8hDm?=
 =?us-ascii?Q?HxNhabb0wWLBXFObc0edemKmIytyA5y7I4c0IJ/yG5U7w9eEaMTl4NiP70Dr?=
 =?us-ascii?Q?UH8x9vgiZYEy8XV0SZ6ToUYOCR9jSemkxaIretKtBUWrBnEAoP+4r2po1ngN?=
 =?us-ascii?Q?OLJ/4TP2BuM8BnNaTsX0ENpBTyAzuKAVkw5tPy+aiF2uEFGzoWRooUC3TWp9?=
 =?us-ascii?Q?1S4rOLT4lmid+nMWDhsj+X5bcf0mf5QZfhkLb0whoXDg/jJjkbKT4lRKIxQ6?=
 =?us-ascii?Q?6w=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e3e11ec1-eee8-41fa-691a-08dc48556f2f
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Mar 2024 20:44:55.5819
 (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: pIBDBOIo90hCBCxLtEETRryG91njzxkle3XQ2yR6tvGWlH5+v5Ng68A9CP+SKq1/8xBbBHwRin4X68Pmgg5dN2UowmBT3AO2RW50t+ZJCoE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB6125
X-OriginatorOrg: intel.com

Robert Richter wrote:
> With the removal of the Itanium architecture [1] the last architecture
> dependent functions:
> 
>  acpi_numa_slit_init(), acpi_numa_memory_affinity_init()
> 
> were removed. Remove its remainings in the header files too an make
> them static.

It would have helped to clarify that this is a refactoring and absorbing
logic into the helpers to validate the correctness of the conversion
approach. I almost asked for it to be split to make that clearer, but
figured it out eventually.

> [1] cf8e8658100d arch: Remove Itanium (IA-64) architecture

Checkpatch does not like this format and has some other things to say
about this legacy code being touched.

> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/acpi/numa/srat.c | 68 +++++++++++++---------------------------
>  include/linux/acpi.h     |  5 ---
>  2 files changed, 21 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> index 50ae8557e8d1..910609a9754b 100644
> --- a/drivers/acpi/numa/srat.c
> +++ b/drivers/acpi/numa/srat.c
> @@ -208,16 +208,21 @@ int __init srat_disabled(void)
>  	return acpi_numa < 0;
>  }
>  
> -#if defined(CONFIG_X86) || defined(CONFIG_ARM64) || defined(CONFIG_LOONGARCH)
>  /*
>   * Callback for SLIT parsing.  pxm_to_node() returns NUMA_NO_NODE for
>   * I/O localities since SRAT does not list them.  I/O localities are
>   * not supported at this point.
>   */
> -void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
> +static int __init acpi_parse_slit(struct acpi_table_header *table)
>  {
> +	struct acpi_table_slit *slit = (struct acpi_table_slit *)table;
>  	int i, j;
>  
> +	if (!slit_valid(slit)) {
> +		pr_info("SLIT table looks invalid. Not used.\n");
> +		return -EINVAL;
> +	}
> +
>  	for (i = 0; i < slit->locality_count; i++) {
>  		const int from_node = pxm_to_node(i);
>  
> @@ -234,19 +239,25 @@ void __init acpi_numa_slit_init(struct acpi_table_slit *slit)
>  				slit->entry[slit->locality_count * i + j]);
>  		}
>  	}
> +
> +	return 0;
>  }
>  
> -/*
> - * Default callback for parsing of the Proximity Domain <-> Memory
> - * Area mappings
> - */
> -int __init
> -acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
> +static int __initdata parsed_numa_memblks;

Checkpatch wants __initdata moved after the symbol name.

Otherwise this conversion looks correct.

> +static int __init
> +acpi_parse_memory_affinity(union acpi_subtable_headers * header,
> +			   const unsigned long table_end)
>  {
> +	struct acpi_srat_mem_affinity *ma;
>  	u64 start, end;
>  	u32 hotpluggable;
>  	int node, pxm;
>  
> +	ma = (struct acpi_srat_mem_affinity *)header;
> +
> +	acpi_table_print_srat_entry(&header->common);
> +
>  	if (srat_disabled())
>  		goto out_err;
>  	if (ma->header.length < sizeof(struct acpi_srat_mem_affinity)) {
> @@ -293,6 +304,8 @@ acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
>  
>  	max_possible_pfn = max(max_possible_pfn, PFN_UP(end - 1));
>  
> +	parsed_numa_memblks++;
> +
>  	return 0;
>  out_err_bad_srat:
>  	bad_srat();
> @@ -448,27 +461,6 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
>  	(*fake_pxm)++;
>  	return 0;
>  }
> -#else
> -static inline void acpi_table_print_cedt(void) {}

Would be nice to move this patch before the printing patch otherwise
feels icky to add a line in patch2 that gets deleted in patch3.