Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:51 +0100
Robert Richter <rrichter@xxxxxxx> wrote:

> Add AMD Zen5 support for address translation.
> 
> Zen5 systems may be configured to use 'Normalized addresses'. Then,
> host physical addresses (HPA) are different from their system physical
> addresses (SPA). The endpoint has its own physical address space and
> an incoming HPA is already converted to the device's physical address
> (DPA). Thus it has interleaving disabled and CXL endpoints are
> programmed passthrough (DPA == HPA).
> 
> Host Physical Addresses (HPAs) need to be translated from the endpoint
> to its CXL host bridge, esp. to identify the endpoint's root decoder
> and region's address range. ACPI Platform Runtime Mechanism (PRM)
> provides a handler to translate the DPA to its SPA. This is documented
> in:
> 
>  AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
>  ACPI v6.5 Porting Guide, Publication # 58088
>  https://www.amd.com/en/search/documentation/hub.html
> 
> With Normalized Addressing this PRM handler must be used to translate
> an HPA of an endpoint to its SPA.
> 
> Do the following to implement AMD Zen5 address translation:
> 
> Introduce a new file core/atl.c to handle ACPI PRM specific address
> translation code. Naming is loosely related to the kernel's AMD
> Address Translation Library (CONFIG_AMD_ATL) but implementation does
> not depend on it, nor it is vendor specific. Use Kbuild and Kconfig
> options respectively to enable the code depending on architecture and
> platform options.
> 
> AMD Zen5 systems support the ACPI PRM CXL Address Translation firmware
> call (see ACPI v6.5 Porting Guide, Address Translation - CXL DPA to
> System Physical Address). Firmware enables the PRM handler if the
> platform has address translation implemented. Check firmware and
> kernel support of ACPI PRM using the specific GUID. On success enable
> address translation by setting up the earlier introduced root port
> callback, see function cxl_prm_translate_hpa_range(). Setup is done in
> cxl_setup_prm_address_translation(), it is the only function that
> needs to be exported. For low level PRM firmware calls, use the ACPI
> framework.
> 
> Identify the region's interleaving ways by inspecting the address
> ranges. Also determine the interleaving granularity using the address
> translation callback. Note that the position of the chunk from one
> interleaving block to the next may vary and thus cannot be considered
> constant. Address offsets larger than the interleaving block size
> cannot be used to calculate the granularity. Thus, probe the
> granularity using address translation for various HPAs in the same
> interleaving block.
> 
> Signed-off-by: Robert Richter <rrichter@xxxxxxx>
A few things below.  Given they are just trivial formatting things
and I assume you'll fix the build bot issues.

Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>

> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> new file mode 100644
> index 000000000000..d6aa7e6d0ac5
> --- /dev/null
> +++ b/drivers/cxl/core/atl.c
> @@ -0,0 +1,195 @@

> +static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> +{
> +	struct cxl_region_context *ctx = data;
> +	struct cxl_endpoint_decoder *cxled = ctx->cxled;
> +	struct cxl_decoder *cxld = &cxled->cxld;
> +	struct cxl_memdev *cxlmd = ctx->cxlmd;
> +	struct range hpa_range = ctx->hpa_range;
> +	struct pci_dev *pci_dev;
> +	u64 spa_len, len = range_len(&hpa_range);
> +	u64 addr, base_spa, base = hpa_range.start;

This is getting a bit nasty to read. I'd split the declarations
that initialize from the ones that don't. So use a couple more lines
to help readability a little.

> +	int ways, gran;
> +
> +	/*
> +	 * When Normalized Addressing is enabled, the endpoint
> +	 * maintains a 1:1 mapping between HPA and DPA. If disabled,
> +	 * skip address translation and perform only a range check.

Wrap is a little short. 80 chars for code and comments (unless there
is reason to go longer).

> +	 */
> +	if (hpa_range.start != cxled->dpa_res->start)
> +		return 0;
> +
> +	if (!IS_ALIGNED(hpa_range.start, SZ_256M) ||
> +	    !IS_ALIGNED(hpa_range.end + 1, SZ_256M)) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Unaligned decoder HPA range: %#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * Endpoints are programmed passthrough in Normalized
> +	 * Addressing mode.

Tiny bit early on the wrap.  Aim for 80 chars, which puts addressing on the line above.

> +	 */
> +	if (ctx->interleave_ways != 1) {
> +		dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
> +			ctx->interleave_ways, ctx->interleave_granularity);
> +		return -ENXIO;
> +	}
> +
> +	if (!cxlmd || !dev_is_pci(cxlmd->dev.parent)) {
> +		dev_dbg(&cxld->dev, "No endpoint found: %s, range %#llx-%#llx\n",
> +			dev_name(cxld->dev.parent), hpa_range.start,
> +			hpa_range.end);
> +		return -ENXIO;
> +	}
> +
> +	pci_dev = to_pci_dev(cxlmd->dev.parent);
> +
> +	/* Translate HPA range to SPA. */
> +	hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
> +	hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
> +
> +	if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * Since translated addresses include the interleaving
> +	 * offsets, align the range to 256 MB.
> +	 */
> +	hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
> +	hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
> +
> +	spa_len = range_len(&hpa_range);
> +	if (!len || !spa_len || spa_len % len) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	ways = spa_len / len;
> +	gran = SZ_256;
> +
> +	/*
> +	 * Determine interleave granularity
> +	 *
> +	 * Note: The position of the chunk from one interleaving block
> +	 * to the next may vary and thus cannot be considered
> +	 * constant. Address offsets larger than the interleaving
> +	 * block size cannot be used to calculate the granularity.

Wrap looks short.

> +	 */
> +	while (ways > 1 && gran <= SZ_16M) {

As ways isn't modified in here, I think it would clearer as
	if (ways > 1) {
		while (gran < SZ_16M) {
			addr = prm_cxl_dpa_spa(pci_dev, base + gran);
			if (addr != base_spa + gran)
				break;
			gran <<= 1;
		}
	}
> +		addr = prm_cxl_dpa_spa(pci_dev, base + gran);
> +		if (addr != base_spa + gran)
> +			break;
> +		gran <<= 1;
> +	}
> +
> +	if (gran > SZ_16M) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Cannot determine granularity: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	ctx->hpa_range = hpa_range;
> +	ctx->interleave_ways = ways;
> +	ctx->interleave_granularity = gran;
> +
> +	dev_dbg(&cxld->dev,
> +		"address mapping found for %s (hpa -> spa): %#llx+%#llx -> %#llx+%#llx ways:%d granularity:%d\n",
> +		dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
> +		spa_len, ways, gran);
> +
> +	return 0;
> +}
> +
> +void cxl_setup_prm_address_translation(struct cxl_root *cxl_root)
> +{
> +	struct device *host = cxl_root->port.uport_dev;
> +	u64 spa;
> +	struct prm_cxl_dpa_spa_data data = { .spa = &spa, };

Trailing comma is a tiny bit pointless as any change will have to replace
the line anyway.  So I'd drop it. (trivial so ignore if you like ;)

> +	int rc;