Re: [PATCH v5 06/14] cxl: Store the access coordinates for the generic ports

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

On Mon, 08 May 2023 13:47:29 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Each CXL host bridge is represented by an ACPI0016 device. A generic port
> device handle that is an ACPI device is represented by a string of
> ACPI0016 device HID and UID. Create a device handle from the ACPI device
> and retrieve the access coordinates from the stored memory targets. The
> access coordinates are stored under the cxl_dport that is associated with
> the CXL host bridge.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/acpi.c |   28 ++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h  |    2 ++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index f9b35e8fe810..675a4f423f4b 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -537,8 +537,26 @@ static int cxl_get_chbcr(union acpi_subtable_headers *header, void *arg,
>  	return 0;
>  }
>  
> +static int get_genport_coordinates(struct device *dev, struct cxl_dport *dport)
> +{
> +	struct acpi_device *hb = to_cxl_host_bridge(NULL, dev);
> +	u8 handle[ACPI_SRAT_DEVICE_HANDLE_SIZE] = { 0 };
> +	int rc;
> +
> +	/* ACPI spec 6.5 tABLE 5.65 */

tABLE?

> +	memcpy(handle, acpi_device_hid(hb), 8);
> +	memcpy(&handle[8], acpi_device_uid(hb), 4);
> +
> +	rc = acpi_get_genport_coordinates(handle, dport->genport_coord);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
>  static int add_host_bridge_dport(struct device *match, void *arg)
>  {
> +	int ret;
>  	acpi_status rc;
>  	struct device *bridge;
>  	unsigned long long uid;
> @@ -594,6 +612,16 @@ static int add_host_bridge_dport(struct device *match, void *arg)
>  	if (IS_ERR(dport))
>  		return PTR_ERR(dport);
>  
> +	dport->genport_coord = devm_kzalloc(dport->dport,
> +					    sizeof(*dport->genport_coord),
> +					    GFP_KERNEL);

It's pretty small - worth allocating separately?

Maybe add something on why to the patch description if there is another reason
for this dance.

> +	if (!dport->genport_coord)
> +		return -ENOMEM;
> +
> +	ret = get_genport_coordinates(match, dport);
> +	if (ret)
> +		dev_dbg(match, "Failed to get generic port perf coordinates.\n");
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index c1e2c3703a63..033b822a20f2 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -626,6 +626,7 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
>   * @rcrb: base address for the Root Complex Register Block
>   * @rch: Indicate whether this dport was enumerated in RCH or VH mode
>   * @port: reference to cxl_port that contains this downstream port
> + * @genport_coord: access coordinates (performance) from ACPI generic port
>   * @coord: access coordinates (performance) for switch from CDAT
>   * @link_latency: calculated PCIe downstream latency
>   */
> @@ -636,6 +637,7 @@ struct cxl_dport {
>  	resource_size_t rcrb;
>  	bool rch;
>  	struct cxl_port *port;
> +	struct access_coordinate *genport_coord;
>  	struct access_coordinate coord;
>  	long link_latency;
>  };
> 
>