Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Robert Richter <rrichter@amd.com>]

On Thu, Nov 13, 2025 at 01:36:26PM -0700, Dave Jiang wrote:
> 
> 
> On 11/13/25 1:05 PM, Robert Richter wrote:
> > On 12.11.25 09:34:34, Dave Jiang wrote:
> >>
> >>
> >> On 11/11/25 5:54 AM, Robert Richter wrote:
> >>> On 04.11.25 10:13:34, Dave Jiang wrote:
> >>>>
> >>>>
> >>>> On 11/3/25 11:47 AM, Robert Richter wrote:
> >>>>> There is only support to translate addresses from an endpoint to its
> >>>>> CXL host bridge, but not in the opposite direction from the bridge to
> >>>>> the endpoint. Thus, the endpoint address range cannot be determined
> >>>>> and setup manually for a given SPA range of a region. If the endpoint
> >>>>> has address translation enabled, lock it to prevent the kernel from
> >>>>> reconfiguring it.
> >>>>>
> >>>>> Reviewed-by: Gregory Price <gourry@gourry.net>
> >>>>> Signed-off-by: Robert Richter <rrichter@amd.com>
> >>>>> ---
> >>>>>  drivers/cxl/core/atl.c | 10 ++++++++++
> >>>>>  1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> >>>>> index d6aa7e6d0ac5..5c15e4d12193 100644
> >>>>> --- a/drivers/cxl/core/atl.c
> >>>>> +++ b/drivers/cxl/core/atl.c
> >>>>> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> >>>>>  		return -ENXIO;
> >>>>>  	}
> >>>>>  
> >>>>> +	/*
> >>>>> +	 * There is only support to translate from the endpoint to its
> >>>>> +	 * parent port, but not in the opposite direction from the
> >>>>> +	 * parent to the endpoint. Thus, the endpoint address range
> >>>>> +	 * cannot be determined and setup manually. If the address range
> >>>>> +	 * was translated and modified, forbid reprogramming of the
> >>>>> +	 * decoders and lock them.
> >>>>> +	 */
> >>>>> +	cxld->flags |= CXL_DECODER_F_LOCK;
> >>>>
> >>>  
> >>>> Feels like this should be something the BIOS should enforce if that
> >>>> is the expectation? And the kernel checks and warns if that is not
> >>>> the case.
> >>>
> >>> I think this is more a limitation of the kernel implementation rather
> >>> than the BIOS. The BIOS provides enought information by CFMWS, PRM,
> >>> HDM and PCI topology. In theory and if there is demand for it, support
> >>> could be added for driver region setup.
> >>
> > 
> >> But shouldn't the BIOS set the decoder lock rather than the kernel
> >> setting a software lock flag based on assumption of the PRM based
> >> setup?
> > 
> > If BIOS locks the decoders, it cannot be removed even for the case
> > there the OS can actually handle it.
> 

> Oh so the current implementation is auto region by BIOS but in the
> future it may not be? But if you add a lock flag, you wouldn't be
> able to remove it later anyhow since it's presented as locked?

The BIOS provides all necessary data for address translation, so that
decoders can be reconfigured (including normalized endpoint
addresses). There is no reason to lock the decoders by the BIOS, as
otherwise, with a capable kernel (or other OS), it would not be
possible to shutdown auto-generated regions.

However, current kernel implementation does not support this and is
unable to create the region. That is why the kernel and not the BIOS
should lock the decoders.

-Robert