Re: [PATCH v2] cxl: Add endpoint decoder flags clear when PCI reset happens

[Dave Jiang <dave.jiang@intel.com>]

On 3/18/26 7:12 AM, Cheatham, Benjamin wrote:
> On 3/17/2026 6:49 PM, Dave Jiang wrote:
>> When a PCI reset happens, the lock and enable flags of the CXL device
>> should be cleared to avoid stale state flags after reset. Add flag
>> clearing during cxl_reset_done() to clear the relevant endpoint
>> decoder flags for all decoders of the endpoint device.
>>
>> Reported-by: Dan Williams <dan.j.williams@intel.com>
>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v2:
>> - Rename CLEAR_MASK to RESET_MASK (Alison)
>> - Make the helper function take a mask argument to specify which flags
>>   to clear (Alison)
>> ---
>>  drivers/cxl/cxl.h |  1 +
>>  drivers/cxl/pci.c | 19 +++++++++++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index 9b947286eb9b..d09c84bcc015 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -333,6 +333,7 @@ int cxl_dport_map_rcd_linkcap(struct pci_dev *pdev, struct cxl_dport *dport);
>>  #define CXL_DECODER_F_LOCK  BIT(4)
>>  #define CXL_DECODER_F_ENABLE    BIT(5)
>>  #define CXL_DECODER_F_NORMALIZED_ADDRESSING BIT(6)
>> +#define CXL_DECODER_F_RESET_MASK (CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK)
>>  
>>  enum cxl_decoder_type {
>>  	CXL_DECODER_DEVMEM = 2,
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index fbb300a01830..a0bba4b4db92 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -1030,6 +1030,20 @@ static void cxl_error_resume(struct pci_dev *pdev)
>>  		 dev->driver ? "successful" : "failed");
>>  }
>>  
>> +static int cxl_decoder_clear_flags(struct device *dev, void *data)
>> +{
>> +	struct cxl_endpoint_decoder *cxled;
>> +	unsigned long *flags = data;
>> +
>> +	if (!is_endpoint_decoder(dev))
>> +		return 0;
> 
> I would either a) rename this function to cxl_endpoint_decoder_clear_flags() or b) change
> it to use struct cxl_decoder devices directly. The generality of option b is nicer imo,
> but you'd have to throw in an extra check for switch decoders (or add a is_cxl_decoder() helper).

Thanks for the review Ben. We can probably do both to make it extra clear.

DJ

> 
> With either of those:
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> 
> Thanks,
> Ben
>> +
>> +	cxled = to_cxl_endpoint_decoder(dev);
>> +	cxled->cxld.flags &= ~(*flags);
>> +
>> +	return 0;
>> +}
>> +
>>  static void cxl_reset_done(struct pci_dev *pdev)
>>  {
>>  	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
>> @@ -1045,6 +1059,11 @@ static void cxl_reset_done(struct pci_dev *pdev)
>>  	guard(device)(&cxlmd->dev);
>>  	if (cxlmd->endpoint &&
>>  	    cxl_endpoint_decoder_reset_detected(cxlmd->endpoint)) {
>> +		unsigned long flags = CXL_DECODER_F_RESET_MASK;
>> +
>> +		device_for_each_child(&cxlmd->endpoint->dev, &flags,
>> +				      cxl_decoder_clear_flags);
>> +
>>  		dev_crit(dev, "SBR happened without memory regions removal.\n");
>>  		dev_crit(dev, "System may be unstable if regions hosted system memory.\n");
>>  		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
>>
>> base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
>