Re: [PATCH v23 08/22] cxl/hdm: Add support for getting region from committed decoder

[PJ Waskiewicz <ppwaskie@kernel.org>]

On Thu, 2026-02-12 at 09:16 +0000, Alejandro Lucero Palau wrote:
> 
> On 2/11/26 22:11, Cheatham, Benjamin wrote:
> > On 2/1/2026 9:54 AM, alejandro.lucero-palau@amd.com wrote:
> > > From: Alejandro Lucero <alucerop@amd.com>
> > > 
> > > A Type2 device configured by the BIOS can already have its HDM
> > > committed. Add a cxl_get_committed_decoder() function for cheking
> > > so after memdev creation. A CXL region should have been created
> > > during memdev initialization, therefore a Type2 driver can ask
> > > for
> > > such a region for working with the HPA. If the HDM is not
> > > committed,
> > > a Type2 driver will create the region after obtaining proper HPA
> > > and DPA space.
> > > 
> > > Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> > > ---
> > >   drivers/cxl/core/hdm.c | 39
> > > +++++++++++++++++++++++++++++++++++++++
> > >   include/cxl/cxl.h      |  3 +++
> > >   2 files changed, 42 insertions(+)
> > > 
> > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > > index 6e516c69b2d2..a172ce4e9b19 100644
> > > --- a/drivers/cxl/core/hdm.c
> > > +++ b/drivers/cxl/core/hdm.c
> > > @@ -686,6 +686,45 @@ int cxl_dpa_alloc(struct
> > > cxl_endpoint_decoder *cxled, u64 size)
> > >   	return devm_add_action_or_reset(&port->dev,
> > > cxl_dpa_release, cxled);
> > >   }
> > >   
> > > +static int find_committed_endpoint_decoder(struct device *dev,
> > > const void *data)
> > > +{
> > > +	struct cxl_endpoint_decoder *cxled;
> > > +	struct cxl_port *port;
> > > +
> > > +	if (!is_endpoint_decoder(dev))
> > > +		return 0;
> > > +
> > > +	cxled = to_cxl_endpoint_decoder(dev);
> > > +	port = cxled_to_port(cxled);
> > > +
> > > +	return cxled->cxld.id == port->hdm_end;
> > Is this the way you're supposed to check if a decoder is committed?
> > The doc comment for @hdm_end in
> > struct cxl_port says it's just the last allocated decoder. If
> > allocated decoders are always committed then
> > I'm fine with this, otherwise I think you'd want to a register read
> > or something to find the commit state.
> 
> 
> Hi Ben,
> 
> 
> Yes, I think you are right. This works in my tests and it is safe 
> because I check the region does exist before using it. But the error 
> inside sfc should then not be fatal for cxl sfc initialization and 
> fallback to the other cxl initialization possibility.
> 

So I'm running into this situation I think.

When you're testing, are you surviving a reload of the driver?  Right
now, I can load and successfully create the region0 device.  However,
following the same teardown path in SFC, I cannot reload my driver
afterwards and map the region.  I get:

cxl_port endpoint5: failed to attach decoder5 to region0: -6  (ENXIO)
<driver> 0000:c1:00.0: CXL found committed decoder without a region
<driver> 0000:c1:00.0: CXL init failed

I'd be surprised if SFC in its current patch state would survive this
same insmod/rmmod/insmod test.

-PJ