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

Jonathan Cameron
2025-11-11T15:30:53Z

On Mon, 3 Nov 2025 19:47:51 +0100
Robert Richter <rrichter@amd.com> 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@amd.com>
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@huawei.com>

> 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;