Re: [PATCH 4/4] acpi: numa: Add helper function to retrieve the performance attributes

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 05 May 2023 17:05:34 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Add helper to retrieve the performance attributes based on the device
> handle.  The helper function is exported so the CXL driver can use that
> to acquire the performance data between the CPU and the CXL host bridge.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Trivial comment inline. Otherwise LGTM

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/acpi/numa/hmat.c |   35 +++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h     |    9 +++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> index 951579e903cf..73d716e6096e 100644
> --- a/drivers/acpi/numa/hmat.c
> +++ b/drivers/acpi/numa/hmat.c
> @@ -107,6 +107,41 @@ static struct memory_target *find_mem_target(unsigned int mem_pxm)
>  	return NULL;
>  }
>  
> +static struct memory_target *acpi_find_genport_target(u8 *device_handle)
> +{
> +	struct memory_target *target;
> +
> +	list_for_each_entry(target, &targets, node) {
> +		if (!strncmp(target->device_handle, device_handle,
> +			     ACPI_SRAT_DEVICE_HANDLE_SIZE))

Using this for something that isn't a string?
memcmp() ?

Mind you I'm not sure what the nameless author this code was doing
in packing the device handle as a u8 array rather than a union of the two
types it can contain.  They probably had a reason lost to the mists of time...

> +			return target;
> +	}
> +
> +	return NULL;
> +}
> +
> +/**
> + * acpi_get_genport_coordinates - Retrieve the access coordinates for a generic port
> + * @device_handle: Device handle string (ACPI or PCI) to match up to the gen port
> + * @coord: The access coordinates written back out for the generic port
> + *
> + * Return: 0 on success. Errno on failure.
> + */
> +int acpi_get_genport_coordinates(u8 *device_handle,
> +				 struct access_coordinate *coord)
> +{
> +	struct memory_target *target;
> +
> +	target = acpi_find_genport_target(device_handle);
> +	if (!target)
> +		return -ENOENT;
> +
> +	*coord = target->coord[NODE_ACCESS_CLASS_GENPORT];
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(acpi_get_genport_coordinates, CXL);
> +
>  static __init void alloc_memory_initiator(unsigned int cpu_pxm)
>  {
>  	struct memory_initiator *initiator;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4c3dfe7587e9..d6a99fa430dd 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -15,6 +15,7 @@
>  #include <linux/device.h>
>  #include <linux/property.h>
>  #include <linux/uuid.h>
> +#include <linux/node.h>
>  
>  #ifndef _LINUX
>  #define _LINUX
> @@ -455,6 +456,8 @@ extern bool acpi_osi_is_win8(void);
>  #ifdef CONFIG_ACPI_NUMA
>  int acpi_map_pxm_to_node(int pxm);
>  int acpi_get_node(acpi_handle handle);
> +int acpi_get_genport_coordinates(u8 *device_handle,
> +				 struct access_coordinate *coord);
>  
>  /**
>   * pxm_to_online_node - Map proximity ID to online node
> @@ -489,6 +492,12 @@ static inline int acpi_get_node(acpi_handle handle)
>  {
>  	return 0;
>  }
> +
> +static inline int acpi_get_genport_coordinates(u8 *device_handle,
> +					       struct access_coordinate *coord)
> +{
> +	return -EOPNOTSUPP;
> +}
>  #endif
>  extern int acpi_paddr_to_node(u64 start_addr, u64 size);
>  
> 
>