Re: [PATCH v11 05/12] cxl: Cache endpoint decoder settings during PCI enumeration

[Srirangan Madhavan <smadhavan@nvidia.com>]

On 9/9/26 7:21 PM, Li Ming wrote:
> External email: Use caution opening links or attachments
> 
> 
> 在 2026/9/10 08:07, Srirangan Madhavan 写道:
>> On 9/2/26 7:03 AM, Li Ming wrote:
>>>> +             return true;
>>>> +
>>>> +     pci_cxl_hdm_init(pdev);
>>>
>>> I forgot to ask a question in v10, sorry about that.
>>>
>>> I am wondering why calling pci_cxl_hdm_init() in
>>> cxl_pci_hdm_decoder_count_match() is needed. pci_bus_add_device()
>>> already does pci_cxl_hdm_init(), so if my understanding is correct,
>>> pdev->hdm is not NULL here, so this pci_cxl_hdm_init() here will not do
>>> anything except checking if pdev->hdm is NULL. I am not sure if I miss
>>> some cases for that.
>>
>> Hi Li Ming!
>> Your question is valid. pci_bus_add_device() is the normal
>> initialization path. The later call is a retry for cases where that
>> attempt did not publish pdev->hdm, for example because allocation or
>> MMIO mapping failed at that point.
>> If initialization succeeded earlier, the existing cache is detected
>> and the retry returns without rebuilding it. I clarified this behavior
>> in the v12 commit message.
>>
> Hi Srirangan,
> 
> 
> Thanks for your explaination.
> 
> I think removing this retry should be better if it is only used for the
> case that allocation or MMIO mapping failure during
> pci_bus_add_device(). Because these failures are not expected to happen
> often, and removing it can make pci_cxl_hdm_init() implementation
> simpler. If only pci_bus_add_device() calls pci_cxl_hdm_init(), you
> won't have to hold cxl_rwsem.dpa and check whether pdev->hdm is NULL in
> __pci_cxl_hdm_init(). Besides, hold cxl_rwsem.dpa lock in
> pci_cxl_hdm_release() is not needed, because the function is called in
> pci_release_dev() where is at the end of this pci device's lifecycle.
> 
> 
> Ming

Hi Ming!

In v12 I removed the retry from the CXL core path and kept 
pci_cxl_hdm_init() only in pci_bus_add_device().
-- 
Regards,
Srirangan