Re: [PATCH v13 08/15] cxl: Refresh cached PCI HDM decoder settings

[Srirangan Madhavan <smadhavan@nvidia.com>]

On 9/23/26 2:40 PM, Cheatham, Benjamin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 9/22/2026 3:39 AM, Srirangan Madhavan wrote:
>> Early PCI discovery creates the HDM cache, while later CXL enumeration and
>> decoder operations provide updated programming state.
>>
>> Refresh the PCI snapshot when decoders are enumerated, committed, or reset
>> so reset recovery need not walk the CXL topology. Ignore updates when no
>> cache exists and reject decoder-count mismatches.
>>
>> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
>> ---
>>   drivers/cxl/core/hdm.c | 66 ++++++++++++++++++++++++++++++++++++++++++
>>   include/cxl/cxl.h      | 22 ++++++++++++++
>>   include/linux/pci.h    |  6 ++++
>>   3 files changed, 94 insertions(+)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index 98268e8e15b2..0ae250ea4ce3 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -753,6 +753,51 @@ static void cxl_decoder_snapshot(struct cxl_decoder *cxld,
>>        }
>>   }
>>
>> +#ifdef CONFIG_CXL_RESET
> 
> The CXL maintainers are normally pretty against #ifdefs in c code. I was going to suggest
> moving it to resource.c, but it looks like cxl_decoder_snapshot() needs to also be in this
> file. So, I think the fix is to put:
> 
> if (!IS_ENABLED(CONFIG_CXL_RESET))
>          return;
> 
> immediately after the variable declarations.
> 

Fixed in v14 patch 9. Now cxl_hdm_refresh_decoder() is compiled without 
it. CXL_BUS selects CXL_RESET in patch 6, so whenever hdm.c is built, 
reset support is enabled; the IS_ENABLED() check is unnecessary.

>> +static void cxl_hdm_refresh_decoder(struct cxl_hdm *cxlhdm,
>> +                                 struct cxl_decoder *cxld)
>> +{
>> +     struct cxl_port *port = cxlhdm->port;
>> +     void __iomem *hdm = cxlhdm->regs.hdm_decoder;
>> +     struct pci_dev *pdev __free(pci_dev_put) =
>> +             cxl_port_get_uport_pci_dev(port);
> 
> This line needs to be at the first usage, i.e. right before "if (!pdev || !hdm)" below, to prevent
> lifetime bugs.
> 

In v14, the pdev declaration is immediately before if (!pdev || !hdm), 
so its cleanup lifetime begins at first use.

>> @@ -1089,9 +1136,27 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>>   {
>>        void __iomem *hdm = cxlhdm->regs.hdm_decoder;
>>        struct cxl_port *port = cxlhdm->port;
>> +#ifdef CONFIG_CXL_RESET
>> +     struct pci_dev *pdev __free(pci_dev_put) =
>> +             cxl_port_get_uport_pci_dev(port);
>> +#endif
> 
> This shouldn't be here, and if you move it to be inside the below if statement
> then you won't need the #ifdef as well.
> 
<snip>
>
>> +#endif
>> +
> 
> Add IS_ENABLED(CONFIG_CXL_RESET) to the above if condition, like above.
> 

pdev declaration is now inside the endpoint/HDM check, immediately 
before it is used. Removed the #ifdef. Since CXL_BUS selects CXL_RESET, 
an IS_ENABLED(CONFIG_CXL_RESET) condition is unnecessary whenever this 
function is built.

-- 
Regards,
Srirangan