Re: [PATCH v10 05/12] cxl: Cache endpoint decoder settings during PCI enumeration
From: Srirangan Madhavan
Date: Tue Sep 01 2026 - 23:24:41 EST
On 8/26/26 11:30 AM, Lucero Palau, Alejandro wrote:
+ up_read(&cxl_rwsem.dpa);
+ return 0;
+ }
Why to release the lock here? or why is this needed at all?
If I understand this correctly, this is happening before the cxl core
does any sort of initialization, indeed, the potential pdev driver can
not be probed either, that seems to be the reason here, to get the HDM
data if committed for doing the reset before the driver binds. Who could
be setting pdev->hdm?
I can not see the necessity and if I'm right, you can avoid the
pdev->hdm check as well at the end and avoid the call for releasing
info. Another comment below.
Hi Alejandro,
Yes, that is the intent: capture committed HDM state during PCI enumeration so it is available to reset paths before a driver binds.
pdev->hdm may already have been populated by the call from pci_bus_add_device() when the fallback call from CXL core enumeration runs. The initial check makes that later call a no-op. Since the lock is released during allocation and MMIO access, the final check protects the check-then-publish window if the two
call sites overlap.
snip
+ up_read(&cxl_rwsem.dpa);
+
+ rc = cxl_pci_hdm_capable(pdev);
+ if (rc)
+ return rc;
+
+ rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
+ if (rc)
+ return rc;
+ rc = rc2;
+ }
+ }
+
+ if (rc)
+ goto out_free_info;
+
+ down_write(&cxl_rwsem.dpa);
+ if (!pdev->hdm) {
+ pdev->hdm = info;
+ info = NULL;
+ }
+ up_write(&cxl_rwsem.dpa);
+
+ cxl_pci_hdm_info_free(info);
This seems weird if info is being assigned above to the pdev. Maybe it
would be clearer if done in an else branch instead:
down_write(&cxl_rwsem.dpa);
if (!pdev->hdm) {
pdev->hdm = info;
info = NULL;
} else {
cxl_pci_hdm_info_free(info);
}
up_write(&cxl_rwsem.dpa);
Thanks,
Alejandro.
In v11, I converted both lock regions to scoped guards and followed your suggestion to free the newly allocated duplicate explicitly in the else branch.
--
Regards,
Srirangan