> -----Original Message----- > From: Jonathan Cameron <[email protected]> > Sent: Tuesday, September 22, 2026 7:06 AM > To: Manish Honap <[email protected]> > Cc: [email protected]; [email protected]; Ankit Agrawal <[email protected]>; > [email protected]; [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]; 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 v5 02/27] cxl/regs: Let a BAR-owning driver own the > component register block > > External email: Use caution opening links or attachments > > > On Thu, 17 Sep 2026 00:05:15 +0530 > <[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 already requested the whole > > component register BAR, such as vfio-cxl, would then collide with that > > claim and fail to map the HDM decoder and RAS blocks. > > > > Add cxl_reg_map_add_owned_resource() so such a driver records the > > resource it already owns on the register map, and skip the sub-block > > request when the block falls within an owned resource. > > This seems a slightly odd intermediate point. > > The assumption is that if owned the whole bar is owned - hence only handle > one owned region (no check if there is already one set?) yet the code will > try to > map it if we get a request for something outside of the region mapped (which > can't exist). > > This all makes me a little nervous. >
Agreed; current model is to have one owned resource - the whole component register BAR and I will enforce this in v6. 1. Helper is going to be renamed to cxl_reg_map_set_owned_resource and it will return -EBUSY when resource is already set. 2. cxl_map_component_regs will fail with -ENXIO when a block of an owned map lies outside the owned resource. > > > > Assisted-by: LLM > > Signed-off-by: Manish Honap <[email protected]> > > --- > > drivers/cxl/core/regs.c | 17 +++++++++++++++-- > > include/cxl/cxl.h | 2 ++ > > include/cxl/pci.h | 3 +++ > > 3 files changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index > > d79550dbb484..58a7c5cafb45 100644 > > --- a/drivers/cxl/core/regs.c > > +++ b/drivers/cxl/core/regs.c > > @@ -216,6 +216,13 @@ void __iomem *devm_cxl_iomap_block(struct > device > > *dev, resource_size_t addr, } > > EXPORT_SYMBOL_NS_GPL(devm_cxl_iomap_block, "CXL"); > > > > +void cxl_reg_map_add_owned_resource(struct cxl_register_map *map, > > + struct resource *res) { > > + map->owned = res; > > As above, I think this needs a sanity check that a future driver author > doesn't > think they can just add lots of these. > Absolute minimum is add some Docs. Yes, I will add the documentation. > > > +} > > +EXPORT_SYMBOL_NS_GPL(cxl_reg_map_add_owned_resource, "CXL"); > > + > > int cxl_map_component_regs(const struct cxl_register_map *map, > > struct cxl_component_regs *regs, > > unsigned long map_mask) @@ -234,6 +241,7 @@ > > int cxl_map_component_regs(const struct cxl_register_map *map, > > struct mapinfo *mi = &mapinfo[i]; > > resource_size_t addr; > > resource_size_t length; > > + struct resource res; > > > > if (!mi->rmap->valid) > > continue; > > @@ -241,8 +249,13 @@ 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); > > - if (!*(mi->addr)) > > + res = DEFINE_RES_MEM(addr, length); > > + > > + if (map->owned && resource_contains(map->owned, &res)) > > If assumption is whole bar or none, should we just fail if map->owned is true > but the resource contains fails? Yes, agreed; I will also add this. > > > + *mi->addr = devm_cxl_ioremap_block(host, addr, > > length); > > + else > > + *mi->addr = devm_cxl_iomap_block(host, addr, length); > > + if (!*mi->addr) > > return -ENOMEM; > > } > >
