Re: [PATCH v3 07/25] cxl/core: Separate region mode from decoder mode

2024-09-02 Thread Li, Ming4
On 8/16/2024 10:44 PM, ira.we...@intel.com wrote:
> From: Navneet Singh 
>
> Until now region modes and decoder modes were equivalent in that both
> modes were either PMEM or RAM.  The addition of Dynamic
> Capacity partitions defines up to 8 DC partitions per device.
>
> The region mode is thus no longer equivalent to the endpoint decoder
> mode.  IOW the endpoint decoders may have modes of DC0-DC7 while the
> region mode is simply DC.
>
> Define a new region mode enumeration which applies to regions separate
> from the decoder mode.  Adjust the code to process these modes
> independently.
>
> There is no equal to decoder mode dead in region modes.  Avoid
> constructing regions with decoders which have been flagged as dead.
>
> Suggested-by: Jonathan Cameron 
> Signed-off-by: Navneet Singh 
> Co-developed-by: Ira Weiny 
> Signed-off-by: Ira Weiny 
Reviewed-by: Li Ming 



Re: [PATCH v3 07/25] cxl/core: Separate region mode from decoder mode

2024-08-23 Thread Jonathan Cameron
On Fri, 16 Aug 2024 09:44:15 -0500
ira.we...@intel.com wrote:

> From: Navneet Singh 
> 
> Until now region modes and decoder modes were equivalent in that both
> modes were either PMEM or RAM.  The addition of Dynamic
> Capacity partitions defines up to 8 DC partitions per device.
> 
> The region mode is thus no longer equivalent to the endpoint decoder
> mode.  IOW the endpoint decoders may have modes of DC0-DC7 while the
> region mode is simply DC.
> 
> Define a new region mode enumeration which applies to regions separate
> from the decoder mode.  Adjust the code to process these modes
> independently.
> 
> There is no equal to decoder mode dead in region modes.  Avoid
> constructing regions with decoders which have been flagged as dead.
> 
> Suggested-by: Jonathan Cameron 
> Signed-off-by: Navneet Singh 
> Co-developed-by: Ira Weiny 
> Signed-off-by: Ira Weiny 
Reviewed-by: Jonathan Cameron 



Re: [PATCH v3 07/25] cxl/core: Separate region mode from decoder mode

2024-08-16 Thread Dave Jiang



On 8/16/24 7:44 AM, ira.we...@intel.com wrote:
> From: Navneet Singh 
> 
> Until now region modes and decoder modes were equivalent in that both
> modes were either PMEM or RAM.  The addition of Dynamic
> Capacity partitions defines up to 8 DC partitions per device.
> 
> The region mode is thus no longer equivalent to the endpoint decoder
> mode.  IOW the endpoint decoders may have modes of DC0-DC7 while the
> region mode is simply DC.
> 
> Define a new region mode enumeration which applies to regions separate
> from the decoder mode.  Adjust the code to process these modes
> independently.
> 
> There is no equal to decoder mode dead in region modes.  Avoid
> constructing regions with decoders which have been flagged as dead.
> 
> Suggested-by: Jonathan Cameron 
> Signed-off-by: Navneet Singh 
> Co-developed-by: Ira Weiny 
> Signed-off-by: Ira Weiny 
Reviewed-by: Dave Jiang 

> 
> ---
> Changes:
> [iweiny: rebase]
> [Jonathan: remove dead code]
> [Jonathan: clarify commit message]
> ---
>  drivers/cxl/core/region.c | 75 
> ++-
>  drivers/cxl/cxl.h | 26 ++--
>  2 files changed, 79 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 971a314b6b0e..796e5a791e44 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -144,7 +144,7 @@ static ssize_t uuid_show(struct device *dev, struct 
> device_attribute *attr,
>   rc = down_read_interruptible(&cxl_region_rwsem);
>   if (rc)
>   return rc;
> - if (cxlr->mode != CXL_DECODER_PMEM)
> + if (cxlr->mode != CXL_REGION_PMEM)
>   rc = sysfs_emit(buf, "\n");
>   else
>   rc = sysfs_emit(buf, "%pUb\n", &p->uuid);
> @@ -457,7 +457,7 @@ static umode_t cxl_region_visible(struct kobject *kobj, 
> struct attribute *a,
>* Support tooling that expects to find a 'uuid' attribute for all
>* regions regardless of mode.
>*/
> - if (a == &dev_attr_uuid.attr && cxlr->mode != CXL_DECODER_PMEM)
> + if (a == &dev_attr_uuid.attr && cxlr->mode != CXL_REGION_PMEM)
>   return 0444;
>   return a->mode;
>  }
> @@ -620,7 +620,7 @@ static ssize_t mode_show(struct device *dev, struct 
> device_attribute *attr,
>  {
>   struct cxl_region *cxlr = to_cxl_region(dev);
>  
> - return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode));
> + return sysfs_emit(buf, "%s\n", cxl_region_mode_name(cxlr->mode));
>  }
>  static DEVICE_ATTR_RO(mode);
>  
> @@ -646,7 +646,7 @@ static int alloc_hpa(struct cxl_region *cxlr, 
> resource_size_t size)
>  
>   /* ways, granularity and uuid (if PMEM) need to be set before HPA */
>   if (!p->interleave_ways || !p->interleave_granularity ||
> - (cxlr->mode == CXL_DECODER_PMEM && uuid_is_null(&p->uuid)))
> + (cxlr->mode == CXL_REGION_PMEM && uuid_is_null(&p->uuid)))
>   return -ENXIO;
>  
>   div64_u64_rem(size, (u64)SZ_256M * p->interleave_ways, &remainder);
> @@ -1863,6 +1863,17 @@ static int cxl_region_sort_targets(struct cxl_region 
> *cxlr)
>   return rc;
>  }
>  
> +static bool cxl_modes_compatible(enum cxl_region_mode rmode,
> +  enum cxl_decoder_mode dmode)
> +{
> + if (rmode == CXL_REGION_RAM && dmode == CXL_DECODER_RAM)
> + return true;
> + if (rmode == CXL_REGION_PMEM && dmode == CXL_DECODER_PMEM)
> + return true;
> +
> + return false;
> +}
> +
>  static int cxl_region_attach(struct cxl_region *cxlr,
>struct cxl_endpoint_decoder *cxled, int pos)
>  {
> @@ -1882,9 +1893,11 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>   return rc;
>   }
>  
> - if (cxled->mode != cxlr->mode) {
> - dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
> - dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
> + if (!cxl_modes_compatible(cxlr->mode, cxled->mode)) {
> + dev_dbg(&cxlr->dev, "%s region mode: %s mismatch decoder: %s\n",
> + dev_name(&cxled->cxld.dev),
> + cxl_region_mode_name(cxlr->mode),
> + cxl_decoder_mode_name(cxled->mode));
>   return -EINVAL;
>   }
>  
> @@ -2447,7 +2460,7 @@ static int cxl_region_calculate_adistance(struct 
> notifier_block *nb,
>   * devm_cxl_add_region - Adds a region to a decoder
>   * @cxlrd: root decoder
>   * @id: memregion id to create, or memregion_free() on failure
> - * @mode: mode for the endpoint decoders of this region
> + * @mode: mode of this region
>   * @type: select whether this is an expander or accelerator (type-2 or 
> type-3)
>   *
>   * This is the second step of region initialization. Regions exist within an
> @@ -2458,7 +2471,7 @@ static int cxl_region_calculate_adistance(struct 
> notifier_block *nb,
>   */
>  static struct cxl_region *devm_cxl_add_region(stru