> -----Original Message----- > From: Dave Jiang <[email protected]> > Sent: Friday, September 4, 2026 3:21 AM > To: Manish Honap <[email protected]>; [email protected]; [email protected]; > Ankit Agrawal <[email protected]>; [email protected]; alejandro.lucero- > [email protected]; Srirangan Madhavan <[email protected]>; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; Yishai Hadas <[email protected]>; Shameer > Kolothum Thodi <[email protected]>; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected] > Cc: Neo Jia <[email protected]>; Krishnakant Jaju <[email protected]>; Vikram > Sethi <[email protected]>; Zhi Wang <[email protected]>; linux- > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; linux- > [email protected]; [email protected] > Subject: Re: [PATCH v4 02/27] cxl/regs: Skip sub-block region request for BAR- > owning drivers > > External email: Use caution opening links or attachments > > > On 8/13/26 2:36 AM, [email protected] wrote: > > From: Manish Honap <[email protected]> > > > > cxl_map_component_regs() claims each mapped sub-block with > > devm_request_mem_region(). A driver that owns the whole component > > register BAR, such as vfio-cxl, has already claimed the full BAR, so > > the per-sub-block claim collides and the mapping fails. > > > > Add a bar_owned parameter to cxl_pci_setup_regs() and record it in > > skip_sub_bar_request on the register map. When set, cxl-core maps the > > sub-block without requesting the region and leaves ownership with the > > upper driver. cxl_pci passes false and keeps its existing claim, > > preserving /dev/mem tooling access to the rest of the component space. > > So currently it's either cxl_pci driver that binds to the PCI device or > vfio_cxl > driver right? Why does cxl_pci needs to pass in false? When vfio_cxl driver is > being attached, cxl_pci is not involved and all the bar ranges should be > free? Or > is there a contention between vfio_cxl and cxl_core that it calls on? > > DJ
You are correct that only one of cxl_pci or vfio-cxl binds the function at a time, so there is no race between them. The collision is one layer down, inside the vfio-cxl bind itself. - vfio-pci-core claims the whole BAR for every populated BAR when it maps them in vfio_pci_core_map_bars(). - vfio-cxl then reuses the cxl-core helper cxl_pci_setup_regs() to locate and map the sub-blocks from component space. - The helper routines default path claims each sub-block with devm_request_mem_region(); Those sub-blocks sit inside the BAR vfio already owns, so the nested request returns -EBUSY and the mapping fails. cxl_pci passes false because its native flow never requests the whole BAR; it relies on exactly those per-sub-block requests as its ownership, and leaving the rest of the BAR unclaimed. v5 will drop the bool argument as per Alex's earlier feedback. The caller will register the resource it owns, and cxl-core will decide per sub-block with resource_contains(), skipping the request when the target already lies inside a caller-owned resource. Thanks, Manish > > > > Signed-off-by: Manish Honap <[email protected]> > > --- > > drivers/cxl/core/pci.c | 9 ++++++++- drivers/cxl/core/regs.c | 32 > > ++++++++++++++++++++++---------- > > drivers/cxl/pci.c | 4 ++-- > > include/cxl/cxl.h | 3 +++ > > include/cxl/pci.h | 2 +- > > 5 files changed, 36 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index > > 9d807c1a002c..08d4c955137d 100644 > > --- a/drivers/cxl/core/pci.c > > +++ b/drivers/cxl/core/pci.c > > @@ -720,7 +720,7 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev > > *pdev, } > > > > int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > > - struct cxl_register_map *map) > > + struct cxl_register_map *map, bool bar_owned) > > { > > int rc; > > > > @@ -750,6 +750,13 @@ int cxl_pci_setup_regs(struct pci_dev *pdev, enum > cxl_regloc_type type, > > return rc; > > } > > > > + /* > > + * A caller that owns the whole register BAR (for example vfio-cxl) > > + * maps the sub-blocks without claiming them, so the later > > + * cxl_map_component_regs() does not collide with the full-BAR > > request. > > + */ > > + map->skip_sub_bar_request = bar_owned; > > + > > return cxl_setup_regs(map); > > } > > EXPORT_SYMBOL_NS_GPL(cxl_pci_setup_regs, "CXL"); diff --git > > a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index > > 989e79383b99..90dd85f322f5 100644 > > --- a/drivers/cxl/core/regs.c > > +++ b/drivers/cxl/core/regs.c > > @@ -177,35 +177,47 @@ void cxl_probe_device_regs(struct device *dev, > > void __iomem *base, } EXPORT_SYMBOL_NS_GPL(cxl_probe_device_regs, > > "CXL"); > > > > -void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t > addr, > > - resource_size_t length) > > +static void __iomem *__cxl_iomap_block(struct device *dev, > resource_size_t addr, > > + resource_size_t length, bool > > +request) > > { > > + resource_size_t end = addr + length - 1; > > void __iomem *ret_val; > > struct resource *res; > > > > if (WARN_ON_ONCE(addr == CXL_RESOURCE_NONE)) > > return NULL; > > > > - res = devm_request_mem_region(dev, addr, length, dev_name(dev)); > > - if (!res) { > > - resource_size_t end = addr + length - 1; > > - > > - dev_err(dev, "Failed to request region %pa-%pa\n", &addr, > > &end); > > - return NULL; > > + /* > > + * An owning driver (e.g. vfio-cxl) may already hold the whole BAR; a > > + * sub-block request would then collide with that claim. > > + */ > > + if (request) { > > + res = devm_request_mem_region(dev, addr, length, > dev_name(dev)); > > + if (!res) { > > + dev_err(dev, "Failed to request region %pa-%pa\n", > > &addr, > &end); > > + return NULL; > > + } > > } > > > > ret_val = devm_ioremap(dev, addr, length); > > if (!ret_val) > > - dev_err(dev, "Failed to map region %pr\n", res); > > + dev_err(dev, "Failed to map region %pa-%pa\n", &addr, > > + &end); > > > > return ret_val; > > } > > + > > +void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t > addr, > > + resource_size_t length) { > > + return __cxl_iomap_block(dev, addr, length, true); } > > EXPORT_SYMBOL_NS_GPL(devm_cxl_iomap_block, "CXL"); > > > > int cxl_map_component_regs(const struct cxl_register_map *map, > > struct cxl_component_regs *regs, > > unsigned long map_mask) { > > + bool request = !map->skip_sub_bar_request; > > struct device *host = map->host; > > struct mapinfo { > > const struct cxl_reg_map *rmap; @@ -227,7 +239,7 @@ int > > cxl_map_component_regs(const struct cxl_register_map *map, > > continue; > > addr = map->resource + mi->rmap->offset; > > length = mi->rmap->size; > > - *(mi->addr) = devm_cxl_iomap_block(host, addr, length); > > + *(mi->addr) = __cxl_iomap_block(host, addr, length, > > + request); > > if (!*(mi->addr)) > > return -ENOMEM; > > } > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index > > bb892dbfdd6d..ec1693c21f61 100644 > > --- a/drivers/cxl/pci.c > > +++ b/drivers/cxl/pci.c > > @@ -816,7 +816,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, > > const struct pci_device_id *id) > > > > cxlds->rcd = is_cxl_restricted(pdev); > > > > - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > > + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, > > + false); > > if (rc) > > return rc; > > > > @@ -829,7 +829,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const > struct pci_device_id *id) > > * still be useful for management functions so don't return an error. > > */ > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, > > - &cxlds->reg_map); > > + &cxlds->reg_map, false); > > if (rc) > > dev_warn(&pdev->dev, "No component registers (%d)\n", rc); > > else if (!cxlds->reg_map.component_map.ras.valid) > > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h index > > efb8ac97bd92..f8e8fddba152 100644 > > --- a/include/cxl/cxl.h > > +++ b/include/cxl/cxl.h > > @@ -213,6 +213,8 @@ struct cxl_pmu_reg_map { > > * @resource: physical resource base of the register block > > * @max_size: maximum mapping size to perform register search > > * @reg_type: see enum cxl_regloc_type > > + * @skip_sub_bar_request: set via cxl_pci_setup_regs(); the owning driver > holds > > + * the BAR, so skip the sub-block claim > > * @component_map: cxl_reg_map for component registers > > * @device_map: cxl_reg_maps for device registers > > * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units @@ > > -223,6 +225,7 @@ struct cxl_register_map { > > resource_size_t resource; > > resource_size_t max_size; > > u8 reg_type; > > + bool skip_sub_bar_request; > > union { > > struct cxl_component_reg_map component_map; > > struct cxl_device_reg_map device_map; diff --git > > a/include/cxl/pci.h b/include/cxl/pci.h index > > 3e0000015871..81a32004a604 100644 > > --- a/include/cxl/pci.h > > +++ b/include/cxl/pci.h > > @@ -18,5 +18,5 @@ struct cxl_register_map; struct pci_dev; > > > > int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > > - struct cxl_register_map *map); > > + struct cxl_register_map *map, bool bar_owned); > > #endif
