On Wed, May 27, 2026 at 04:37:11PM -0700, Dave Jiang wrote:
> 
> 
> On 5/23/26 2:42 AM, Anisa Su wrote:
> > From: Ira Weiny <[email protected]>
> > 
> > Device partitions have an implied order which is made more complex by
> > the addition of a dynamic partition.
> > 
> > Remove the ram special case information calls in favor of generic calls
> > with a check ahead of time to ensure the preservation of the implied
> > partition order.
> > 
> > Signed-off-by: Ira Weiny <[email protected]>
> > 
> > ---
> > Changes::
> > [anisa: rebase]
> > [davidlohr: core/hdm.c: return -EINVAL instead of 0 in cxl_dpa_setup
> > if partitions are out of order]
> > ---
> >  drivers/cxl/core/hdm.c    | 11 ++++++++++-
> >  drivers/cxl/core/memdev.c | 32 +++++++++-----------------------
> >  drivers/cxl/cxlmem.h      |  9 +++------
> >  drivers/cxl/mem.c         |  2 +-
> >  4 files changed, 23 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 28974adaab75..7a5812971f8f 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -464,6 +464,7 @@ static const char *cxl_mode_name(enum 
> > cxl_partition_mode mode)
> >  int cxl_dpa_setup(struct cxl_dev_state *cxlds, const struct cxl_dpa_info 
> > *info)
> >  {
> >     struct device *dev = cxlds->dev;
> > +   int i;
> >  
> >     guard(rwsem_write)(&cxl_rwsem.dpa);
> >  
> > @@ -476,9 +477,17 @@ int cxl_dpa_setup(struct cxl_dev_state *cxlds, const 
> > struct cxl_dpa_info *info)
> >             return 0;
> >     }
> >  
> > +   /* Verify partitions are in expected order. */
> > +   for (i = 1; i < info->nr_partitions; i++) {
> > +           if (cxlds->part[i].mode < cxlds->part[i-1].mode) {
> 
> I think we need to check info->part[i].mode and not cxlds here. cxlds mode is 
> assigned later in this function.
> 
fixed!
> DJ
> 
Thanks,
Anisa
> 
> > +                   dev_err(dev, "Partition order mismatch\n");
> > +                   return -EINVAL;
> > +           }
> > +   }
> > +
> >     cxlds->dpa_res = DEFINE_RES_MEM(0, info->size);
> >  
> > -   for (int i = 0; i < info->nr_partitions; i++) {
> > +   for (i = 0; i < info->nr_partitions; i++) {
> >             const struct cxl_dpa_part_info *part = &info->part[i];
> >             int rc;
> >  
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > index 80e65690eb77..71602820f896 100644
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> > @@ -75,20 +75,12 @@ static ssize_t label_storage_size_show(struct device 
> > *dev,
> >  }
> >  static DEVICE_ATTR_RO(label_storage_size);
> >  
> > -static resource_size_t cxl_ram_size(struct cxl_dev_state *cxlds)
> > -{
> > -   /* Static RAM is only expected at partition 0. */
> > -   if (cxlds->part[0].mode != CXL_PARTMODE_RAM)
> > -           return 0;
> > -   return resource_size(&cxlds->part[0].res);
> > -}
> > -
> >  static ssize_t ram_size_show(struct device *dev, struct device_attribute 
> > *attr,
> >                          char *buf)
> >  {
> >     struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> >     struct cxl_dev_state *cxlds = cxlmd->cxlds;
> > -   unsigned long long len = cxl_ram_size(cxlds);
> > +   unsigned long long len = cxl_part_size(cxlds, CXL_PARTMODE_RAM);
> >  
> >     return sysfs_emit(buf, "%#llx\n", len);
> >  }
> > @@ -101,7 +93,7 @@ static ssize_t pmem_size_show(struct device *dev, struct 
> > device_attribute *attr,
> >  {
> >     struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> >     struct cxl_dev_state *cxlds = cxlmd->cxlds;
> > -   unsigned long long len = cxl_pmem_size(cxlds);
> > +   unsigned long long len = cxl_part_size(cxlds, CXL_PARTMODE_PMEM);
> >  
> >     return sysfs_emit(buf, "%#llx\n", len);
> >  }
> > @@ -424,10 +416,11 @@ static struct attribute *cxl_memdev_attributes[] = {
> >     NULL,
> >  };
> >  
> > -static struct cxl_dpa_perf *to_pmem_perf(struct cxl_dev_state *cxlds)
> > +static struct cxl_dpa_perf *part_perf(struct cxl_dev_state *cxlds,
> > +                                 enum cxl_partition_mode mode)
> >  {
> >     for (int i = 0; i < cxlds->nr_partitions; i++)
> > -           if (cxlds->part[i].mode == CXL_PARTMODE_PMEM)
> > +           if (cxlds->part[i].mode == mode)
> >                     return &cxlds->part[i].perf;
> >     return NULL;
> >  }
> > @@ -438,7 +431,7 @@ static ssize_t pmem_qos_class_show(struct device *dev,
> >     struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> >     struct cxl_dev_state *cxlds = cxlmd->cxlds;
> >  
> > -   return sysfs_emit(buf, "%d\n", to_pmem_perf(cxlds)->qos_class);
> > +   return sysfs_emit(buf, "%d\n", part_perf(cxlds, 
> > CXL_PARTMODE_PMEM)->qos_class);
> >  }
> >  
> >  static struct device_attribute dev_attr_pmem_qos_class =
> > @@ -450,20 +443,13 @@ static struct attribute *cxl_memdev_pmem_attributes[] 
> > = {
> >     NULL,
> >  };
> >  
> > -static struct cxl_dpa_perf *to_ram_perf(struct cxl_dev_state *cxlds)
> > -{
> > -   if (cxlds->part[0].mode != CXL_PARTMODE_RAM)
> > -           return NULL;
> > -   return &cxlds->part[0].perf;
> > -}
> > -
> >  static ssize_t ram_qos_class_show(struct device *dev,
> >                               struct device_attribute *attr, char *buf)
> >  {
> >     struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> >     struct cxl_dev_state *cxlds = cxlmd->cxlds;
> >  
> > -   return sysfs_emit(buf, "%d\n", to_ram_perf(cxlds)->qos_class);
> > +   return sysfs_emit(buf, "%d\n", part_perf(cxlds, 
> > CXL_PARTMODE_RAM)->qos_class);
> >  }
> >  
> >  static struct device_attribute dev_attr_ram_qos_class =
> > @@ -499,7 +485,7 @@ static umode_t cxl_ram_visible(struct kobject *kobj, 
> > struct attribute *a, int n)
> >  {
> >     struct device *dev = kobj_to_dev(kobj);
> >     struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> > -   struct cxl_dpa_perf *perf = to_ram_perf(cxlmd->cxlds);
> > +   struct cxl_dpa_perf *perf = part_perf(cxlmd->cxlds, CXL_PARTMODE_RAM);
> >  
> >     if (a == &dev_attr_ram_qos_class.attr &&
> >         (!perf || perf->qos_class == CXL_QOS_CLASS_INVALID))
> > @@ -518,7 +504,7 @@ static umode_t cxl_pmem_visible(struct kobject *kobj, 
> > struct attribute *a, int n
> >  {
> >     struct device *dev = kobj_to_dev(kobj);
> >     struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> > -   struct cxl_dpa_perf *perf = to_pmem_perf(cxlmd->cxlds);
> > +   struct cxl_dpa_perf *perf = part_perf(cxlmd->cxlds, CXL_PARTMODE_PMEM);
> >  
> >     if (a == &dev_attr_pmem_qos_class.attr &&
> >         (!perf || perf->qos_class == CXL_QOS_CLASS_INVALID))
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index cee936fb3d03..10175ca3b7ee 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
> > @@ -383,14 +383,11 @@ struct cxl_security_state {
> >  
> >  #define CXL_MAX_DC_PARTITIONS 8
> >  
> > -static inline resource_size_t cxl_pmem_size(struct cxl_dev_state *cxlds)
> > +static inline resource_size_t cxl_part_size(struct cxl_dev_state *cxlds,
> > +                                       enum cxl_partition_mode mode)
> >  {
> > -   /*
> > -    * Static PMEM may be at partition index 0 when there is no static RAM
> > -    * capacity.
> > -    */
> >     for (int i = 0; i < cxlds->nr_partitions; i++)
> > -           if (cxlds->part[i].mode == CXL_PARTMODE_PMEM)
> > +           if (cxlds->part[i].mode == mode)
> >                     return resource_size(&cxlds->part[i].res);
> >     return 0;
> >  }
> > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> > index fcffe24dcb42..f19e08279ec7 100644
> > --- a/drivers/cxl/mem.c
> > +++ b/drivers/cxl/mem.c
> > @@ -114,7 +114,7 @@ static int cxl_mem_probe(struct device *dev)
> >             return -ENXIO;
> >     }
> >  
> > -   if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
> > +   if (cxl_part_size(cxlds, CXL_PARTMODE_PMEM) && 
> > IS_ENABLED(CONFIG_CXL_PMEM)) {
> >             rc = devm_cxl_add_nvdimm(dev, parent_port, cxlmd);
> >             if (rc) {
> >                     if (rc == -ENODEV)
> 

Reply via email to