Received-SPF: pass (google.com: domain of linux-kernel+bounces-200959-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 4 Jun 2024 16:30:28 +0100
From: Simon Horman <horms@kernel.org>
To: Wei Huang <wei.huang2@amd.com>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, netdev@vger.kernel.org,
	bhelgaas@google.com, corbet@lwn.net, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	alex.williamson@redhat.com, gospo@broadcom.com,
	michael.chan@broadcom.com, ajit.khaparde@broadcom.com,
	somnath.kotur@broadcom.com, andrew.gospodarek@broadcom.com,
	manoj.panicker2@amd.com, Eric.VanTassell@amd.com,
	vadim.fedorenko@linux.dev, bagasdotme@gmail.com
Subject: Re: [PATCH V2 6/9] PCI/TPH: Retrieve steering tag from ACPI _DSM
Message-ID: <20240604153028.GU491852@kernel.org>
References: <20240531213841.3246055-1-wei.huang2@amd.com>
 <20240531213841.3246055-7-wei.huang2@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240531213841.3246055-7-wei.huang2@amd.com>

On Fri, May 31, 2024 at 04:38:38PM -0500, Wei Huang wrote:
> According to PCI SIG ECN, calling the _DSM firmware method for a given
> CPU_UID returns the steering tags for different types of memory
> (volatile, non-volatile). These tags are supposed to be used in ST
> table entry for optimal results.
> 
> Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com>
> Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com>
> Signed-off-by: Wei Huang <wei.huang2@amd.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com> 
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>

...

> diff --git a/drivers/pci/pcie/tph.c b/drivers/pci/pcie/tph.c

...

> +static bool invoke_dsm(acpi_handle handle, u32 cpu_uid, u8 ph,
> +		       u8 target_type, bool cache_ref_valid,
> +		       u64 cache_ref, union st_info *st_out)
> +{
> +	union acpi_object in_obj, in_buf[3], *out_obj;
> +
> +	in_buf[0].integer.type = ACPI_TYPE_INTEGER;
> +	in_buf[0].integer.value = 0; /* 0 => processor cache steering tags */
> +
> +	in_buf[1].integer.type = ACPI_TYPE_INTEGER;
> +	in_buf[1].integer.value = cpu_uid;
> +
> +	in_buf[2].integer.type = ACPI_TYPE_INTEGER;
> +	in_buf[2].integer.value = ph & 3;
> +	in_buf[2].integer.value |= (target_type & 1) << 2;
> +	in_buf[2].integer.value |= (cache_ref_valid & 1) << 3;
> +	in_buf[2].integer.value |= (cache_ref << 32);
> +
> +	in_obj.type = ACPI_TYPE_PACKAGE;
> +	in_obj.package.count = ARRAY_SIZE(in_buf);
> +	in_obj.package.elements = in_buf;
> +
> +	out_obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, MIN_ST_DSM_REV,
> +				    ST_DSM_FUNC_INDEX, &in_obj);

Hi Wei Huang, Eric, all,

This seems to break builds on ARM (32bit) with multi_v7_defconfig.

  .../tph.c:221:39: error: use of undeclared identifier 'pci_acpi_dsm_guid'
  221 |         out_obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, MIN_ST_DSM_REV,
      |

I suspect a dependency on ACPI in Kconfig is appropriate.

> +
> +	if (!out_obj)
> +		return false;
> +
> +	if (out_obj->type != ACPI_TYPE_BUFFER) {
> +		pr_err("invalid return type %d from TPH _DSM\n",
> +		       out_obj->type);
> +		ACPI_FREE(out_obj);
> +		return false;
> +	}

...