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

[<dan.j.williams@intel.com>]

Gregory Price wrote:
> On Wed, Jan 21, 2026 at 02:09:27PM -0800, dan.j.williams@intel.com wrote:
> > > 
> > > I see. So the concern is including model-specific methods that would
> > > modify the CXL standard flow, correct?
> > 
> ...
> > 
> > As I told Robert, I want a generic "Normalized Address" facility of
> > which Zen5 is the first user.
> > 
> 
> Isn't that what this patch functionally is w/ a specific PRM function?
> 
>    rc = acpi_call_prm_handler(prm_cxl_dpa_spa_guid, &data);
> 
> Or is the request now: replace this with static table data?

As I mentioned at the bottom of this message to Yazen [1], the request
is to prove or disprove the hypothesis that a table would have sufficed,
but otherwise go ahead with merging this handler. Set a precedent that
the next attempt to solve a problem like this with PRM will face a
higher bar.

[1]: http://lore.kernel.org/69701f6de978_1d6f1001e@dwillia2-mobl4.notmuch

> point of ignorance: what facility would you use to expose such tables?

New sub-structure of the CEDT similar to the CXIMS.

> -----
> 
> When i initialially hacked up driver support for this mode before
> getting PRM support, the "hacked up translation code" I was this:
> 
>   /* Find 0-based offset into whole interleave region */
>   dev = (pdev->bus->number == 0xe1) ? 0 : 1;
>   offset = (0x100 * (((norm_addr >> 8) * 2) + dev)) + (norm_addr & 0xff);
> 
>   /* Find the SPA base for the address */
>   for (idx = 0; idx < cfmws_nr; idx++) {
>       size = cxl_get_cfmws_size(idx);
>       /* We may have a gap in the CFMWS */
>       if (offset < size) {
>           *sys_addr = cxl_get_cfmws_base(idx) + offset;
>           return 0;
>       }
>       offset -= size;
>    }
> 
> ------
> 
> This makes hard-assumptions about two things:
> 
>   device interleave index  - pcidev(0xe1) => 0
>   cfmws base               - all CFMWS are used for this one region
> 
> cxl_get_cfmws_base() was a call into ACPI code, and the acpi code just
> kept a global cache of the raw CEDT CFMWS structures (base + size);
> 
> So, assuming you had such tables, it would need to be like:
> 
>                   Normalized Decoders Table
>     --------------------------------------------------------
>     | CXL PCIDev | Decoder  | CFMW SPAN  |  Interleave IDX |
>     --------------------------------------------------------
>     |     d1     |    0     |    1,2     |        0        |
>     |     e1     |    0     |    1,2     |        1        |
>     --------------------------------------------------------
>   --------------------------------^
>   |            CFMW Index Table
>   |  -----------------------------------------
>   |  | CFMW ID |     BASE       |    SIZE    |
>   |  -----------------------------------------
>   |  |    0    | 0xb00000....   |     ...    |
>   |->|    1    | 0xc05000....   |            |
>   |->|    2    | 0x100500....   |            |
>      |    3    | 0x200000....   |     ...    |
>      -----------------------------------------
> 
> -------
> 
> The code above turns into
> 
> int cxl_normal_translate(pdev, norm_addr, u64* sys_addr)
> {
>     int i_idx = cxl_nrm_decoder_interleave_index(pdev);
>     int span, i;
>     u64 offset;
> 
>     if (i_idx < 0)
>     	return -EINVAL;
> 
>     span = cxl_nrm_decoder_window_span(pdev);
> 
>     /* Normalized offset into whole region */
>     offset = (0x100 * (((norm_addr >> 8) * 2) + i_idx)) + (norm_addr & 0xff);
> 
>     /* Find actual CFMW Base (might cross multiple w/ gaps) */
>     for (i = 0; i < span; i++) {
>         u64 base, size;
> 	int id;
> 
>         id = cxl_nrm_decoder_cfmws_id(i);
> 	if (id < 0)
> 	   return -EINVAL;
> 
>         if (!cxl_nrm_decoder_cfmws_data(id, &base, &size))
> 	   return -EINVAL;
> 
> 	if (offset < size) {
> 	    *sys_addr = cxl_get_cfmws_base(id) + offset;
> 	    return 0;
> 	}
>         offset -= size;
>     }
>     return -EINVAL;
> }
> 
> Where the cxl_nrm_*() functions just query the exposed tables - however
> that actually happens.
> 
> --------
> 
> I don't know whether the above math is actually true, it's basically
> just the simply interleave maths. If something else is going on, then
> this whole table thing might not actually work.
> 
> The rest of the patch set would more or less stay the same.

If the above is even close to being correct, I would merge that in a
heartbeat over this PRM proposal.

Robert, do you really want to be spending time on trying moving PRM to
userspace vs just doing the above?