| Message ID | 20230604-dcd-type2-upstream-v2-6-f740c47e7916@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DCD: Add support for Dynamic Capacity Devices (DCD) | expand |
On Mon, 28 Aug 2023 22:20:57 -0700 Ira Weiny <ira.weiny@intel.com> wrote: > To support Dynamic Capacity Devices (DCD) endpoint decoders will need to > map DC Regions (partitions). Part of this is assigning the size of the > DC Region DPA to the decoder in addition to any skip value from the > previous decoder which exists. This must be done within a continuous > DPA space. Two complications arise with Dynamic Capacity regions which > did not exist with Ram and PMEM partitions. First, gaps in the DPA > space can exist between and around the DC Regions. Second, the Linux > resource tree does not allow a resource to be marked across existing > nodes within a tree. > > For clarity, below is an example of an 60GB device with 10GB of RAM, > 10GB of PMEM and 10GB for each of 2 DC Regions. The desired CXL mapping > is 5GB of RAM, 5GB of PMEM, and all 10GB of DC1. > > DPA RANGE > (dpa_res) > 0GB 10GB 20GB 30GB 40GB 50GB 60GB > |----------|----------|----------|----------|----------|----------| > > RAM PMEM DC0 DC1 > (ram_res) (pmem_res) (dc_res[0]) (dc_res[1]) > |----------|----------| <gap> |----------| <gap> |----------| > > RAM PMEM DC1 > |XXXXX|----|XXXXX|----|----------|----------|----------|XXXXXXXXXX| > 0GB 5GB 10GB 15GB 20GB 30GB 40GB 50GB 60GB > > The previous skip resource between RAM and PMEM was always a child of > the RAM resource and fit nicely (see X below). Because of this > simplicity this skip resource reference was not stored in any CXL state. > On release the skip range could be calculated based on the endpoint > decoders stored values. > > Now when DC1 is being mapped 4 skip resources must be created as > children. One of the PMEM resource (A), two of the parent DPA resource > (B,D), and one more child of the DC0 resource (C). > > 0GB 10GB 20GB 30GB 40GB 50GB 60GB > |----------|----------|----------|----------|----------|----------| > | | > |----------|----------| | |----------| | |----------| > | | | | | > (X) (A) (B) (C) (D) > v v v v v > |XXXXX|----|XXXXX|----|----------|----------|----------|XXXXXXXXXX| > skip skip skip skip skip > > Expand the calculation of DPA freespace and enhance the logic to support > mapping/unmapping DC DPA space. To track the potential of multiple skip > resources an xarray is attached to the endpoint decoder. The existing > algorithm is consolidated with the new one to store a single skip > resource in the same way as multiple skip resources. > > Co-developed-by: Navneet Singh <navneet.singh@intel.com> > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Various minor things noticed inline. Jonathan > > --- > An alternative of using reserve_region_with_split() was considered. > The advantage of that would be keeping all the resource information > stored solely in the resource tree rather than having separate > references to them. However, it would best be implemented with a call > such as release_split_region() [name TBD?] which could find all the leaf > resources in the range and release them. Furthermore, it is not clear > if reserve_region_with_split() is really intended for anything outside > of init code. In the end this algorithm seems straight forward enough. > > Changes for v2: > [iweiny: write commit message] > [iweiny: remove unneeded changes] > [iweiny: split from region creation patch] > [iweiny: Alter skip algorithm to use 'anonymous regions'] > [iweiny: enhance debug messages] > [iweiny: consolidate skip resource creation] > [iweiny: ensure xa_destroy() is called] > [iweiny: consolidate region requests further] > [iweiny: ensure resource is released on xa_insert] > --- > drivers/cxl/core/hdm.c | 188 +++++++++++++++++++++++++++++++++++++++++++----- > drivers/cxl/core/port.c | 2 + > drivers/cxl/cxl.h | 2 + > 3 files changed, 176 insertions(+), 16 deletions(-) > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c > index 3f4af1f5fac8..3cd048677816 100644 > --- a/drivers/cxl/core/hdm.c > +++ b/drivers/cxl/core/hdm.c > + > +static int cxl_reserve_dpa_skip(struct cxl_endpoint_decoder *cxled, > + resource_size_t base, resource_size_t skipped) > +{ > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + struct cxl_port *port = cxled_to_port(cxled); > + struct cxl_dev_state *cxlds = cxlmd->cxlds; > + resource_size_t skip_base = base - skipped; > + resource_size_t size, skip_len = 0; > + struct device *dev = &port->dev; > + int rc, index; > + > + size = resource_size(&cxlds->ram_res); > + if (size && skip_base <= cxlds->ram_res.end) { This size only used in this if statement I'd just put it inline. > + skip_len = cxlds->ram_res.end - skip_base + 1; > + rc = cxl_request_skip(cxled, skip_base, skip_len); > + if (rc) > + return rc; > + skip_base += skip_len; > + } > + > + if (skip_base == base) { > + dev_dbg(dev, "skip done!\n"); Not sure that dbg is much help as other places below where skip also done... > + return 0; > + } > + > + size = resource_size(&cxlds->pmem_res); > + if (size && skip_base <= cxlds->pmem_res.end) { size only used in this if statement. I'd just put the resource_size() bit inline. > + skip_len = cxlds->pmem_res.end - skip_base + 1; > + rc = cxl_request_skip(cxled, skip_base, skip_len); > + if (rc) > + return rc; > + skip_base += skip_len; > + } > + > + index = dc_mode_to_region_index(cxled->mode); > + for (int i = 0; i <= index; i++) { > + struct resource *dcr = &cxlds->dc_res[i]; > + > + if (skip_base < dcr->start) { > + skip_len = dcr->start - skip_base; > + rc = cxl_request_skip(cxled, skip_base, skip_len); > + if (rc) > + return rc; > + skip_base += skip_len; > + } > + > + if (skip_base == base) { > + dev_dbg(dev, "skip done!\n"); As above - perhaps some more info? > + break; > + } > + > + if (resource_size(dcr) && skip_base <= dcr->end) { > + if (skip_base > base) > + dev_err(dev, "Skip error\n"); Not return ? If there is a reason to carry on, I'd like a comment to say what it is. > + > + skip_len = dcr->end - skip_base + 1; > + rc = cxl_request_skip(cxled, skip_base, skip_len); > + if (rc) > + return rc; > + skip_base += skip_len; > + } > + } > + > + return 0; > +} > + > @@ -492,11 +607,13 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, > resource_size_t *start_out, > resource_size_t *skip_out) > { > + resource_size_t free_ram_start, free_pmem_start, free_dc_start; > struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > - resource_size_t free_ram_start, free_pmem_start; > struct cxl_dev_state *cxlds = cxlmd->cxlds; > + struct device *dev = &cxled->cxld.dev; There is one existing (I think) call to dev_dbg(cxled_dev(cxled) ... in this function. So both should use that here, and should convert that one case to using dev. > resource_size_t start, avail, skip; > struct resource *p, *last; > + int index; > > lockdep_assert_held(&cxl_dpa_rwsem); > > @@ -514,6 +631,20 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, > else > free_pmem_start = cxlds->pmem_res.start; > > + /* > + * Limit each decoder to a single DC region to map memory with > + * different DSMAS entry. > + */ > + index = dc_mode_to_region_index(cxled->mode); > + if (index >= 0) { > + if (cxlds->dc_res[index].child) { > + dev_err(dev, "Cannot allocate DPA from DC Region: %d\n", > + index); > + return -EINVAL; > + } > + free_dc_start = cxlds->dc_res[index].start; > + } > + > if (cxled->mode == CXL_DECODER_RAM) { > start = free_ram_start; > avail = cxlds->ram_res.end - start + 1; > @@ -535,6 +666,29 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, > else > skip_end = start - 1; > skip = skip_end - skip_start + 1; > + } else if (cxl_decoder_mode_is_dc(cxled->mode)) { > + resource_size_t skip_start, skip_end; > + > + start = free_dc_start; > + avail = cxlds->dc_res[index].end - start + 1; > + if ((resource_size(&cxlds->pmem_res) == 0) || !cxlds->pmem_res.child) Previous patch used !resource_size() I prefer compare with 0 like you have here, but which ever is chosen, things should be consistent. ...
Jonathan Cameron wrote: > On Mon, 28 Aug 2023 22:20:57 -0700 > Ira Weiny <ira.weiny@intel.com> wrote: > [snip] > > > > Co-developed-by: Navneet Singh <navneet.singh@intel.com> > > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > Various minor things noticed inline. Thanks! [snip] > > > + > > +static int cxl_reserve_dpa_skip(struct cxl_endpoint_decoder *cxled, > > + resource_size_t base, resource_size_t skipped) > > +{ > > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > > + struct cxl_port *port = cxled_to_port(cxled); > > + struct cxl_dev_state *cxlds = cxlmd->cxlds; > > + resource_size_t skip_base = base - skipped; > > + resource_size_t size, skip_len = 0; > > + struct device *dev = &port->dev; > > + int rc, index; > > + > > + size = resource_size(&cxlds->ram_res); > > + if (size && skip_base <= cxlds->ram_res.end) { > > This size only used in this if statement I'd just put it inline. And in the pmem case... > > > + skip_len = cxlds->ram_res.end - skip_base + 1; > > + rc = cxl_request_skip(cxled, skip_base, skip_len); > > + if (rc) > > + return rc; > > + skip_base += skip_len; > > + } > > + > > + if (skip_base == base) { > > + dev_dbg(dev, "skip done!\n"); > > Not sure that dbg is much help as other places below where skip also done... Ok. > > > + return 0; > > + } > > + > > + size = resource_size(&cxlds->pmem_res); > > + if (size && skip_base <= cxlds->pmem_res.end) { > > size only used in this if statement. I'd just put > the resource_size() bit inline. Ah ok. I think the line length was the issue here. I'm ok taking the variable out. > > > + skip_len = cxlds->pmem_res.end - skip_base + 1; > > + rc = cxl_request_skip(cxled, skip_base, skip_len); > > + if (rc) > > + return rc; > > + skip_base += skip_len; > > + } > > + > > + index = dc_mode_to_region_index(cxled->mode); > > + for (int i = 0; i <= index; i++) { > > + struct resource *dcr = &cxlds->dc_res[i]; > > + > > + if (skip_base < dcr->start) { > > + skip_len = dcr->start - skip_base; > > + rc = cxl_request_skip(cxled, skip_base, skip_len); > > + if (rc) > > + return rc; > > + skip_base += skip_len; > > + } > > + > > + if (skip_base == base) { > > + dev_dbg(dev, "skip done!\n"); > > As above - perhaps some more info? Sure. > > > + break; > > + } > > + > > + if (resource_size(dcr) && skip_base <= dcr->end) { > > + if (skip_base > base) > > + dev_err(dev, "Skip error\n"); > > Not return ? If there is a reason to carry on, I'd like a comment to say what it is. Looks like a bug I missed. thanks! > > > + > > + skip_len = dcr->end - skip_base + 1; > > + rc = cxl_request_skip(cxled, skip_base, skip_len); > > + if (rc) > > + return rc; > > + skip_base += skip_len; > > + } > > + } > > + > > + return 0; > > +} > > + > > > > @@ -492,11 +607,13 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, > > resource_size_t *start_out, > > resource_size_t *skip_out) > > { > > + resource_size_t free_ram_start, free_pmem_start, free_dc_start; > > struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > > - resource_size_t free_ram_start, free_pmem_start; > > struct cxl_dev_state *cxlds = cxlmd->cxlds; > > + struct device *dev = &cxled->cxld.dev; > > There is one existing (I think) call to dev_dbg(cxled_dev(cxled) ... > in this function. So both should use that here, and should convert that one > case to using dev. I think the type 2 stuff is lower priority than this series. The main reason I had this series based on that work was due to the split of the memdev state from the device state. Because that patch has landed I've rebased this series on master in hopes of it landing in 6.7 without the type 2 dependency. As such this code got moved to __cxl_dpa_reserve(). > > > resource_size_t start, avail, skip; > > struct resource *p, *last; > > + int index; > > > > lockdep_assert_held(&cxl_dpa_rwsem); > > > > @@ -514,6 +631,20 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, > > else > > free_pmem_start = cxlds->pmem_res.start; > > > > + /* > > + * Limit each decoder to a single DC region to map memory with > > + * different DSMAS entry. > > + */ > > + index = dc_mode_to_region_index(cxled->mode); > > + if (index >= 0) { > > + if (cxlds->dc_res[index].child) { > > + dev_err(dev, "Cannot allocate DPA from DC Region: %d\n", > > + index); > > + return -EINVAL; > > + } > > + free_dc_start = cxlds->dc_res[index].start; > > + } > > + > > if (cxled->mode == CXL_DECODER_RAM) { > > start = free_ram_start; > > avail = cxlds->ram_res.end - start + 1; > > @@ -535,6 +666,29 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, > > else > > skip_end = start - 1; > > skip = skip_end - skip_start + 1; > > + } else if (cxl_decoder_mode_is_dc(cxled->mode)) { > > + resource_size_t skip_start, skip_end; > > + > > + start = free_dc_start; > > + avail = cxlds->dc_res[index].end - start + 1; > > + if ((resource_size(&cxlds->pmem_res) == 0) || !cxlds->pmem_res.child) > > Previous patch used !resource_size() > I prefer compare with 0 like you have here, but which ever is chosen, things should > be consistent. > > ... > Yea good point. I audited the series for this and made the change. Ira
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c index 3f4af1f5fac8..3cd048677816 100644 --- a/drivers/cxl/core/hdm.c +++ b/drivers/cxl/core/hdm.c @@ -222,6 +222,25 @@ void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds) } EXPORT_SYMBOL_NS_GPL(cxl_dpa_debug, CXL); +static void cxl_skip_release(struct cxl_endpoint_decoder *cxled) +{ + struct cxl_dev_state *cxlds = cxled_to_memdev(cxled)->cxlds; + struct cxl_port *port = cxled_to_port(cxled); + struct device *dev = &port->dev; + unsigned long index; + void *entry; + + xa_for_each(&cxled->skip_res, index, entry) { + struct resource *res = entry; + + dev_dbg(dev, "decoder%d.%d: releasing skipped space; %pr\n", + port->id, cxled->cxld.id, res); + __release_region(&cxlds->dpa_res, res->start, + resource_size(res)); + xa_erase(&cxled->skip_res, index); + } +} + /* * Must be called in a context that synchronizes against this decoder's * port ->remove() callback (like an endpoint decoder sysfs attribute) @@ -232,15 +251,11 @@ static void __cxl_dpa_release(struct cxl_endpoint_decoder *cxled) struct cxl_port *port = cxled_to_port(cxled); struct cxl_dev_state *cxlds = cxlmd->cxlds; struct resource *res = cxled->dpa_res; - resource_size_t skip_start; lockdep_assert_held_write(&cxl_dpa_rwsem); - /* save @skip_start, before @res is released */ - skip_start = res->start - cxled->skip; __release_region(&cxlds->dpa_res, res->start, resource_size(res)); - if (cxled->skip) - __release_region(&cxlds->dpa_res, skip_start, cxled->skip); + cxl_skip_release(cxled); cxled->skip = 0; cxled->dpa_res = NULL; put_device(&cxled->cxld.dev); @@ -280,6 +295,98 @@ static int dc_mode_to_region_index(enum cxl_decoder_mode mode) return -EINVAL; } +static int cxl_request_skip(struct cxl_endpoint_decoder *cxled, + resource_size_t skip_base, resource_size_t skip_len) +{ + struct cxl_dev_state *cxlds = cxled_to_memdev(cxled)->cxlds; + const char *name = dev_name(&cxled->cxld.dev); + struct cxl_port *port = cxled_to_port(cxled); + struct resource *dpa_res = &cxlds->dpa_res; + struct device *dev = &port->dev; + struct resource *res; + int rc; + + res = __request_region(dpa_res, skip_base, skip_len, name, 0); + if (!res) + return -EBUSY; + + rc = xa_insert(&cxled->skip_res, skip_base, res, GFP_KERNEL); + if (rc) { + __release_region(dpa_res, skip_base, skip_len); + return rc; + } + + dev_dbg(dev, "decoder%d.%d: skipped space; %pr\n", + port->id, cxled->cxld.id, res); + return 0; +} + +static int cxl_reserve_dpa_skip(struct cxl_endpoint_decoder *cxled, + resource_size_t base, resource_size_t skipped) +{ + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); + struct cxl_port *port = cxled_to_port(cxled); + struct cxl_dev_state *cxlds = cxlmd->cxlds; + resource_size_t skip_base = base - skipped; + resource_size_t size, skip_len = 0; + struct device *dev = &port->dev; + int rc, index; + + size = resource_size(&cxlds->ram_res); + if (size && skip_base <= cxlds->ram_res.end) { + skip_len = cxlds->ram_res.end - skip_base + 1; + rc = cxl_request_skip(cxled, skip_base, skip_len); + if (rc) + return rc; + skip_base += skip_len; + } + + if (skip_base == base) { + dev_dbg(dev, "skip done!\n"); + return 0; + } + + size = resource_size(&cxlds->pmem_res); + if (size && skip_base <= cxlds->pmem_res.end) { + skip_len = cxlds->pmem_res.end - skip_base + 1; + rc = cxl_request_skip(cxled, skip_base, skip_len); + if (rc) + return rc; + skip_base += skip_len; + } + + index = dc_mode_to_region_index(cxled->mode); + for (int i = 0; i <= index; i++) { + struct resource *dcr = &cxlds->dc_res[i]; + + if (skip_base < dcr->start) { + skip_len = dcr->start - skip_base; + rc = cxl_request_skip(cxled, skip_base, skip_len); + if (rc) + return rc; + skip_base += skip_len; + } + + if (skip_base == base) { + dev_dbg(dev, "skip done!\n"); + break; + } + + if (resource_size(dcr) && skip_base <= dcr->end) { + if (skip_base > base) + dev_err(dev, "Skip error\n"); + + skip_len = dcr->end - skip_base + 1; + rc = cxl_request_skip(cxled, skip_base, skip_len); + if (rc) + return rc; + skip_base += skip_len; + } + } + + return 0; +} + static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled, resource_size_t base, resource_size_t len, resource_size_t skipped) @@ -317,13 +424,12 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled, } if (skipped) { - res = __request_region(&cxlds->dpa_res, base - skipped, skipped, - dev_name(&cxled->cxld.dev), 0); - if (!res) { - dev_dbg(dev, - "decoder%d.%d: failed to reserve skipped space\n", - port->id, cxled->cxld.id); - return -EBUSY; + int rc = cxl_reserve_dpa_skip(cxled, base, skipped); + + if (rc) { + dev_dbg(dev, "decoder%d.%d: failed to reserve skipped space; %#llx - %#llx\n", + port->id, cxled->cxld.id, base, skipped); + return rc; } } res = __request_region(&cxlds->dpa_res, base, len, @@ -331,14 +437,20 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled, if (!res) { dev_dbg(dev, "decoder%d.%d: failed to reserve allocation\n", port->id, cxled->cxld.id); - if (skipped) - __release_region(&cxlds->dpa_res, base - skipped, - skipped); + cxl_skip_release(cxled); return -EBUSY; } cxled->dpa_res = res; cxled->skip = skipped; + for (int mode = CXL_DECODER_DC0; mode <= CXL_DECODER_DC7; mode++) { + int index = dc_mode_to_region_index(mode); + + if (resource_contains(&cxlds->dc_res[index], res)) { + cxled->mode = mode; + goto success; + } + } if (resource_contains(&cxlds->pmem_res, res)) cxled->mode = CXL_DECODER_PMEM; else if (resource_contains(&cxlds->ram_res, res)) @@ -349,6 +461,9 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled, cxled->mode = CXL_DECODER_MIXED; } +success: + dev_dbg(dev, "decoder%d.%d: %pr mode: %d\n", port->id, cxled->cxld.id, + cxled->dpa_res, cxled->mode); port->hdm_end++; get_device(&cxled->cxld.dev); return 0; @@ -492,11 +607,13 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, resource_size_t *start_out, resource_size_t *skip_out) { + resource_size_t free_ram_start, free_pmem_start, free_dc_start; struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); - resource_size_t free_ram_start, free_pmem_start; struct cxl_dev_state *cxlds = cxlmd->cxlds; + struct device *dev = &cxled->cxld.dev; resource_size_t start, avail, skip; struct resource *p, *last; + int index; lockdep_assert_held(&cxl_dpa_rwsem); @@ -514,6 +631,20 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, else free_pmem_start = cxlds->pmem_res.start; + /* + * Limit each decoder to a single DC region to map memory with + * different DSMAS entry. + */ + index = dc_mode_to_region_index(cxled->mode); + if (index >= 0) { + if (cxlds->dc_res[index].child) { + dev_err(dev, "Cannot allocate DPA from DC Region: %d\n", + index); + return -EINVAL; + } + free_dc_start = cxlds->dc_res[index].start; + } + if (cxled->mode == CXL_DECODER_RAM) { start = free_ram_start; avail = cxlds->ram_res.end - start + 1; @@ -535,6 +666,29 @@ static resource_size_t cxl_dpa_freespace(struct cxl_endpoint_decoder *cxled, else skip_end = start - 1; skip = skip_end - skip_start + 1; + } else if (cxl_decoder_mode_is_dc(cxled->mode)) { + resource_size_t skip_start, skip_end; + + start = free_dc_start; + avail = cxlds->dc_res[index].end - start + 1; + if ((resource_size(&cxlds->pmem_res) == 0) || !cxlds->pmem_res.child) + skip_start = free_ram_start; + else + skip_start = free_pmem_start; + /* + * If any dc region is already mapped, then that allocation + * already handled the RAM and PMEM skip. Check for DC region + * skip. + */ + for (int i = index - 1; i >= 0 ; i--) { + if (cxlds->dc_res[i].child) { + skip_start = cxlds->dc_res[i].child->end + 1; + break; + } + } + + skip_end = start - 1; + skip = skip_end - skip_start + 1; } else { dev_dbg(cxled_dev(cxled), "mode not set\n"); avail = 0; @@ -572,6 +726,8 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, unsigned long long size) avail = cxl_dpa_freespace(cxled, &start, &skip); + dev_dbg(dev, "DPA Allocation start: %llx len: %llx Skip: %llx\n", + start, size, skip); if (size > avail) { dev_dbg(dev, "%pa exceeds available %s capacity: %pa\n", &size, cxl_decoder_mode_name(cxled->mode), &avail); diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index ce4a66865db3..a5db710a63bc 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -413,6 +413,7 @@ static void cxl_endpoint_decoder_release(struct device *dev) struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev); __cxl_decoder_release(&cxled->cxld); + xa_destroy(&cxled->skip_res); kfree(cxled); } @@ -1769,6 +1770,7 @@ struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port) return ERR_PTR(-ENOMEM); cxled->pos = -1; + xa_init(&cxled->skip_res); cxld = &cxled->cxld; rc = cxl_decoder_init(port, cxld); if (rc) { diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index d41f3f14fbe3..0a225b0c20bf 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -433,6 +433,7 @@ enum cxl_decoder_state { * @cxld: base cxl_decoder_object * @dpa_res: actively claimed DPA span of this decoder * @skip: offset into @dpa_res where @cxld.hpa_range maps + * @skip_res: array of skipped resources from the previous decoder end * @mode: which memory type / access-mode-partition this decoder targets * @state: autodiscovery state * @pos: interleave position in @cxld.region @@ -441,6 +442,7 @@ struct cxl_endpoint_decoder { struct cxl_decoder cxld; struct resource *dpa_res; resource_size_t skip; + struct xarray skip_res; enum cxl_decoder_mode mode; enum cxl_decoder_state state; int pos;
To support Dynamic Capacity Devices (DCD) endpoint decoders will need to map DC Regions (partitions). Part of this is assigning the size of the DC Region DPA to the decoder in addition to any skip value from the previous decoder which exists. This must be done within a continuous DPA space. Two complications arise with Dynamic Capacity regions which did not exist with Ram and PMEM partitions. First, gaps in the DPA space can exist between and around the DC Regions. Second, the Linux resource tree does not allow a resource to be marked across existing nodes within a tree. For clarity, below is an example of an 60GB device with 10GB of RAM, 10GB of PMEM and 10GB for each of 2 DC Regions. The desired CXL mapping is 5GB of RAM, 5GB of PMEM, and all 10GB of DC1. DPA RANGE (dpa_res) 0GB 10GB 20GB 30GB 40GB 50GB 60GB |----------|----------|----------|----------|----------|----------| RAM PMEM DC0 DC1 (ram_res) (pmem_res) (dc_res[0]) (dc_res[1]) |----------|----------| <gap> |----------| <gap> |----------| RAM PMEM DC1 |XXXXX|----|XXXXX|----|----------|----------|----------|XXXXXXXXXX| 0GB 5GB 10GB 15GB 20GB 30GB 40GB 50GB 60GB The previous skip resource between RAM and PMEM was always a child of the RAM resource and fit nicely (see X below). Because of this simplicity this skip resource reference was not stored in any CXL state. On release the skip range could be calculated based on the endpoint decoders stored values. Now when DC1 is being mapped 4 skip resources must be created as children. One of the PMEM resource (A), two of the parent DPA resource (B,D), and one more child of the DC0 resource (C). 0GB 10GB 20GB 30GB 40GB 50GB 60GB |----------|----------|----------|----------|----------|----------| | | |----------|----------| | |----------| | |----------| | | | | | (X) (A) (B) (C) (D) v v v v v |XXXXX|----|XXXXX|----|----------|----------|----------|XXXXXXXXXX| skip skip skip skip skip Expand the calculation of DPA freespace and enhance the logic to support mapping/unmapping DC DPA space. To track the potential of multiple skip resources an xarray is attached to the endpoint decoder. The existing algorithm is consolidated with the new one to store a single skip resource in the same way as multiple skip resources. Co-developed-by: Navneet Singh <navneet.singh@intel.com> Signed-off-by: Navneet Singh <navneet.singh@intel.com> Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- An alternative of using reserve_region_with_split() was considered. The advantage of that would be keeping all the resource information stored solely in the resource tree rather than having separate references to them. However, it would best be implemented with a call such as release_split_region() [name TBD?] which could find all the leaf resources in the range and release them. Furthermore, it is not clear if reserve_region_with_split() is really intended for anything outside of init code. In the end this algorithm seems straight forward enough. Changes for v2: [iweiny: write commit message] [iweiny: remove unneeded changes] [iweiny: split from region creation patch] [iweiny: Alter skip algorithm to use 'anonymous regions'] [iweiny: enhance debug messages] [iweiny: consolidate skip resource creation] [iweiny: ensure xa_destroy() is called] [iweiny: consolidate region requests further] [iweiny: ensure resource is released on xa_insert] --- drivers/cxl/core/hdm.c | 188 +++++++++++++++++++++++++++++++++++++++++++----- drivers/cxl/core/port.c | 2 + drivers/cxl/cxl.h | 2 + 3 files changed, 176 insertions(+), 16 deletions(-)