NVDIMM Device and Persistent Memory development
 help / color / mirror / Atom feed
diff for duplicates of <20250414153519.0000677a@huawei.com>

diff --git a/a/1.txt b/N1/1.txt
index 2b13a6c..a8fa82a 100644
--- a/a/1.txt
+++ b/N1/1.txt
@@ -27,6 +27,16 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > 
 > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
 > 
+Hi Ira,
+
+This ended up with a slightly odd mix of the nice flexible code which
+we had before to handle multiple regions and just handing one.
+
+Whilst I don't mind keeping the multiple region handling you could further
+simplify this if you didn't...
+
+Jonathan
+
 > ---
 > Changes:
 > [iweiny: rebase]
@@ -50,6 +60,12 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 >  
 > +static int cxl_dc_check(struct device *dev, struct cxl_dc_partition_info *part_array,
 > +			u8 index, struct cxl_dc_partition *dev_part)
+
+I'd be tempted to pass in both this part and the previous one (or NULL) directly rather
+than passing in the array.  Seems like it would end up slightly simpler in here.
+Mind you we only support the first one anyway so maybe we don't need the prev_part
+stuff for now...
+
 > +{
 > +	size_t blk_size, len;
 > +
@@ -58,9 +74,19 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > +	part_array[index].size *= CXL_CAPACITY_MULTIPLIER;
 > +	len = le64_to_cpu(dev_part->length);
 > +	blk_size = le64_to_cpu(dev_part->block_size);
+
+For these, might as well do it at declaration and save a line.
+
+	size_t blk_size = le64_to_cpu(dev_part->length);
+	size_t len = le64_to_cpu(dev_part->length);
+
 > +
 > +	/* Check partitions are in increasing DPA order */
 > +	if (index > 0) {
+
+If you pass the prev_part in as a parameter, this just becomes
+	if (prev_part)
+
 > +		struct cxl_dc_partition_info *prev_part = &part_array[index - 1];
 > +
 > +		if ((prev_part->start + prev_part->size) >
@@ -71,33 +97,9 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > +			return -EINVAL;
 > +		}
 > +	}
-> +
-> +	if (!IS_ALIGNED(part_array[index].start, SZ_256M) ||
-> +	    !IS_ALIGNED(part_array[index].start, blk_size)) {
-> +		dev_err(dev, "DC partition %d invalid start %zu blk size %zu\n",
-> +			index, part_array[index].start, blk_size);
-> +		return -EINVAL;
-> +	}
-> +
-> +	if (part_array[index].size == 0 || len == 0 ||
-> +	    part_array[index].size < len || !IS_ALIGNED(len, blk_size)) {
-> +		dev_err(dev, "DC partition %d invalid length; size %zu len %zu blk size %zu\n",
-> +			index, part_array[index].size, len, blk_size);
-> +		return -EINVAL;
-> +	}
-> +
-> +	if (blk_size == 0 || blk_size % CXL_DCD_BLOCK_LINE_SIZE ||
-> +	    !is_power_of_2(blk_size)) {
-> +		dev_err(dev, "DC partition %d invalid block size; %zu\n",
-> +			index, blk_size);
-> +		return -EINVAL;
-> +	}
-> +
-> +	dev_dbg(dev, "DC partition %d start %zu start %zu size %zu\n",
-> +		index, part_array[index].start, part_array[index].size,
-> +		blk_size);
-> +
-> +	return 0;
+
+Rest of the checks look good to me.
+
 > +}
 > +
 > +/* Returns the number of partitions in dc_resp or -ERRNO */
@@ -116,6 +118,11 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > +		.size_out = dc_resp_size,
 > +		.payload_out = dc_resp,
 > +		.min_out = 1,
+
+Why 1?  If a device oddly supported 0 regions I think it would still be 8
+to cover the first two fields and the reserved space before region configuration
+structure.
+
 > +	};
 > +	int rc;
 > +
@@ -151,12 +158,20 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > +
 > +	struct cxl_mbox_get_dc_config_out *dc_resp __free(kfree) =
 > +					kvmalloc(dc_resp_size, GFP_KERNEL);
+
+Could we size this one for max possible? (i.e. 8 partitions) with a struct
+size and avoid needing vmalloc.  Maybe it is worth the bother.
+
 > +	if (!dc_resp)
 > +		return -ENOMEM;
 > +
 > +	/* Read and check all partition information for validity and potential
+
+Multi line comment syntax isn't this for this file.
+
 > +	 * debugging; see debug output in cxl_dc_check() */
 > +	start_partition = 0;
+Could set at declaration  (up to you)
 > +	do {
 > +		int rc, i, j;
 > +
@@ -167,6 +182,9 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > +		}
 > +
 > +		num_partitions += rc;
+
+Initialization missing I think.
+
 > +
 > +		if (num_partitions < 1 || num_partitions > CXL_MAX_DC_PARTITIONS) {
 > +			dev_err(dev, "Invalid num of dynamic capacity partitions %d\n",
@@ -188,180 +206,31 @@ Ira Weiny <ira.weiny@intel.com> wrote:
 > +	/* Return 1st partition */
 > +	dc_info->start = partitions[0].start;
 > +	dc_info->size = partitions[0].size;
+
+I'm not against keeping the complexity above but if all we are going to do is
+use the first partition, maybe just ask for that in the first place?
+We don't need to check for issues in things we aren't turning on.
+
 > +	dev_dbg(dev, "Returning partition 0 %zu size %zu\n",
 > +		dc_info->start, dc_info->size);
 > +
 > +	return 0;
 > +}
 > +EXPORT_SYMBOL_NS_GPL(cxl_dev_dc_identify, "CXL");
-> +
->  static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_partition_mode mode)
->  {
->  	int i = info->nr_partitions;
-> @@ -1383,6 +1530,38 @@ int cxl_get_dirty_count(struct cxl_memdev_state *mds, u32 *count)
->  }
->  EXPORT_SYMBOL_NS_GPL(cxl_get_dirty_count, "CXL");
->  
-> +void cxl_configure_dcd(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)
-> +{
-> +	struct cxl_dc_partition_info dc_info = { 0 };
-Trivial bit of c stuff that surprised me in another thread the other day that doesn't
-apply here because of packed nature of structure but...
 
- = {}; is defined in c23 (and probably before that in practice) as
-the "empty initializer"
-> +	struct device *dev = mds->cxlds.dev;
-> +	size_t skip;
-> +	int rc;
-> +
-> +	rc = cxl_dev_dc_identify(&mds->cxlds.cxl_mbox, &dc_info);
-> +	if (rc) {
-> +		dev_warn(dev,
-> +			 "Failed to read Dynamic Capacity config: %d\n", rc);
-> +		cxl_disable_dcd(mds);
-> +		return;
-> +	}
-> +
-> +	/* Skips between pmem and the dynamic partition are not supported */
-> +	skip = dc_info.start - info->size;
-> +	if (skip) {
-> +		dev_warn(dev,
-> +			 "Dynamic Capacity skip from pmem not supported: %zu\n",
-> +			 skip);
-> +		cxl_disable_dcd(mds);
-> +		return;
-> +	}
-> +
-> +	info->size += dc_info.size;
-> +	dev_dbg(dev, "Adding dynamic ram partition A; %zu size %zu\n",
-> +		dc_info.start, dc_info.size);
-> +	add_part(info, dc_info.start, dc_info.size, CXL_PARTMODE_DYNAMIC_RAM_A);
-> +}
-> +EXPORT_SYMBOL_NS_GPL(cxl_configure_dcd, "CXL");
-> +
->  int cxl_arm_dirty_shutdown(struct cxl_memdev_state *mds)
->  {
->  	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
-> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
-> index be8a7dc77719..a9d42210e8a3 100644
-> --- a/drivers/cxl/cxl.h
-> +++ b/drivers/cxl/cxl.h
-> @@ -485,6 +485,7 @@ struct cxl_region_params {
->  enum cxl_partition_mode {
->  	CXL_PARTMODE_RAM,
->  	CXL_PARTMODE_PMEM,
-> +	CXL_PARTMODE_DYNAMIC_RAM_A,
->  };
->  
->  /*
+
+
 > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
 > index 394a776954f4..057933128d2c 100644
 > --- a/drivers/cxl/cxlmem.h
 > +++ b/drivers/cxl/cxlmem.h
-> @@ -97,7 +97,7 @@ int devm_cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
->  			 resource_size_t base, resource_size_t len,
->  			 resource_size_t skipped);
->  
-> -#define CXL_NR_PARTITIONS_MAX 2
-> +#define CXL_NR_PARTITIONS_MAX 3
->  
->  struct cxl_dpa_info {
->  	u64 size;
-> @@ -380,6 +380,7 @@ enum cxl_devtype {
->  	CXL_DEVTYPE_CLASSMEM,
->  };
->  
-> +#define CXL_MAX_DC_PARTITIONS 8
->  /**
->   * struct cxl_dpa_perf - DPA performance property entry
->   * @dpa_range: range for DPA address
-> @@ -722,6 +723,31 @@ struct cxl_mbox_set_shutdown_state_in {
->  	u8 state;
->  } __packed;
->  
-> +/* See CXL 3.2 Table 8-178 get dynamic capacity config Input Payload */
-> +struct cxl_mbox_get_dc_config_in {
-> +	u8 partition_count;
-> +	u8 start_partition_index;
-> +} __packed;
-> +
-> +/* See CXL 3.2 Table 8-179 get dynamic capacity config Output Payload */
-> +struct cxl_mbox_get_dc_config_out {
-> +	u8 avail_partition_count;
-> +	u8 partitions_returned;
-> +	u8 rsvd[6];
-> +	/* See CXL 3.2 Table 8-180 */
-> +	struct cxl_dc_partition {
-> +		__le64 base;
-> +		__le64 decode_length;
-> +		__le64 length;
-> +		__le64 block_size;
-> +		__le32 dsmad_handle;
-> +		u8 flags;
-> +		u8 rsvd[3];
-> +	} __packed partition[] __counted_by(partitions_returned);
-> +	/* Trailing fields unused */
-> +} __packed;
-> +#define CXL_DCD_BLOCK_LINE_SIZE 0x40
-> +
->  /* Set Timestamp CXL 3.0 Spec 8.2.9.4.2 */
->  struct cxl_mbox_set_timestamp_in {
->  	__le64 timestamp;
-> @@ -845,9 +871,24 @@ enum {
->  int cxl_internal_send_cmd(struct cxl_mailbox *cxl_mbox,
->  			  struct cxl_mbox_cmd *cmd);
->  int cxl_dev_state_identify(struct cxl_memdev_state *mds);
 > +
 > +struct cxl_mem_dev_info {
 > +	u64 total_bytes;
 > +	u64 volatile_bytes;
 > +	u64 persistent_bytes;
 > +};
-> +
-> +struct cxl_dc_partition_info {
-> +	size_t start;
-> +	size_t size;
-> +};
-> +
-> +int cxl_dev_dc_identify(struct cxl_mailbox *mbox,
-> +			struct cxl_dc_partition_info *dc_info);
->  int cxl_await_media_ready(struct cxl_dev_state *cxlds);
->  int cxl_enumerate_cmds(struct cxl_memdev_state *mds);
->  int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);
-> +void cxl_configure_dcd(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);
->  struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev);
->  void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
->  				unsigned long *cmds);
-> @@ -860,6 +901,17 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
->  			    const uuid_t *uuid, union cxl_event *evt);
->  int cxl_get_dirty_count(struct cxl_memdev_state *mds, u32 *count);
->  int cxl_arm_dirty_shutdown(struct cxl_memdev_state *mds);
-> +
-> +static inline bool cxl_dcd_supported(struct cxl_memdev_state *mds)
-> +{
-> +	return mds->dcd_supported;
-> +}
-> +
-> +static inline void cxl_disable_dcd(struct cxl_memdev_state *mds)
-> +{
-> +	mds->dcd_supported = false;
-> +}
-> +
->  int cxl_set_timestamp(struct cxl_memdev_state *mds);
->  int cxl_poison_state_init(struct cxl_memdev_state *mds);
->  int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
-> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
-> index 7b14a154463c..bc40cf6e2fe9 100644
-> --- a/drivers/cxl/pci.c
-> +++ b/drivers/cxl/pci.c
-> @@ -998,6 +998,9 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
->  	if (rc)
->  		return rc;
->  
-> +	if (cxl_dcd_supported(mds))
-> +		cxl_configure_dcd(mds, &range_info);
-> +
->  	rc = cxl_dpa_setup(cxlds, &range_info);
->  	if (rc)
->  		return rc;
->
+
+So far I'm not seeing any use of this. Left over from previous patch
+or something that gets used later in the series and so should get
+introduced with first use?
diff --git a/a/content_digest b/N1/content_digest
index 8b71356..da968c9 100644
--- a/a/content_digest
+++ b/N1/content_digest
@@ -2,7 +2,7 @@
  "ref\020250413-dcd-type2-upstream-v9-2-1d4911a0b365@intel.com\0"
  "From\0Jonathan Cameron <Jonathan.Cameron@huawei.com>\0"
  "Subject\0Re: [PATCH v9 02/19] cxl/mem: Read dynamic capacity configuration from the device\0"
- "Date\0Mon, 14 Apr 2025 15:35:19 +0100\0"
+ "Date\0Mon, 14 Apr 2025 16:20:40 +0100\0"
  "To\0Ira Weiny <ira.weiny@intel.com>\0"
  "Cc\0Dave Jiang <dave.jiang@intel.com>"
   Fan Ni <fan.ni@samsung.com>
@@ -44,6 +44,16 @@
  "> \n"
  "> Signed-off-by: Ira Weiny <ira.weiny@intel.com>\n"
  "> \n"
+ "Hi Ira,\n"
+ "\n"
+ "This ended up with a slightly odd mix of the nice flexible code which\n"
+ "we had before to handle multiple regions and just handing one.\n"
+ "\n"
+ "Whilst I don't mind keeping the multiple region handling you could further\n"
+ "simplify this if you didn't...\n"
+ "\n"
+ "Jonathan\n"
+ "\n"
  "> ---\n"
  "> Changes:\n"
  "> [iweiny: rebase]\n"
@@ -67,6 +77,12 @@
  ">  \n"
  "> +static int cxl_dc_check(struct device *dev, struct cxl_dc_partition_info *part_array,\n"
  "> +\t\t\tu8 index, struct cxl_dc_partition *dev_part)\n"
+ "\n"
+ "I'd be tempted to pass in both this part and the previous one (or NULL) directly rather\n"
+ "than passing in the array.  Seems like it would end up slightly simpler in here.\n"
+ "Mind you we only support the first one anyway so maybe we don't need the prev_part\n"
+ "stuff for now...\n"
+ "\n"
  "> +{\n"
  "> +\tsize_t blk_size, len;\n"
  "> +\n"
@@ -75,9 +91,19 @@
  "> +\tpart_array[index].size *= CXL_CAPACITY_MULTIPLIER;\n"
  "> +\tlen = le64_to_cpu(dev_part->length);\n"
  "> +\tblk_size = le64_to_cpu(dev_part->block_size);\n"
+ "\n"
+ "For these, might as well do it at declaration and save a line.\n"
+ "\n"
+ "\tsize_t blk_size = le64_to_cpu(dev_part->length);\n"
+ "\tsize_t len = le64_to_cpu(dev_part->length);\n"
+ "\n"
  "> +\n"
  "> +\t/* Check partitions are in increasing DPA order */\n"
  "> +\tif (index > 0) {\n"
+ "\n"
+ "If you pass the prev_part in as a parameter, this just becomes\n"
+ "\tif (prev_part)\n"
+ "\n"
  "> +\t\tstruct cxl_dc_partition_info *prev_part = &part_array[index - 1];\n"
  "> +\n"
  "> +\t\tif ((prev_part->start + prev_part->size) >\n"
@@ -88,33 +114,9 @@
  "> +\t\t\treturn -EINVAL;\n"
  "> +\t\t}\n"
  "> +\t}\n"
- "> +\n"
- "> +\tif (!IS_ALIGNED(part_array[index].start, SZ_256M) ||\n"
- "> +\t    !IS_ALIGNED(part_array[index].start, blk_size)) {\n"
- "> +\t\tdev_err(dev, \"DC partition %d invalid start %zu blk size %zu\\n\",\n"
- "> +\t\t\tindex, part_array[index].start, blk_size);\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tif (part_array[index].size == 0 || len == 0 ||\n"
- "> +\t    part_array[index].size < len || !IS_ALIGNED(len, blk_size)) {\n"
- "> +\t\tdev_err(dev, \"DC partition %d invalid length; size %zu len %zu blk size %zu\\n\",\n"
- "> +\t\t\tindex, part_array[index].size, len, blk_size);\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tif (blk_size == 0 || blk_size % CXL_DCD_BLOCK_LINE_SIZE ||\n"
- "> +\t    !is_power_of_2(blk_size)) {\n"
- "> +\t\tdev_err(dev, \"DC partition %d invalid block size; %zu\\n\",\n"
- "> +\t\t\tindex, blk_size);\n"
- "> +\t\treturn -EINVAL;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tdev_dbg(dev, \"DC partition %d start %zu start %zu size %zu\\n\",\n"
- "> +\t\tindex, part_array[index].start, part_array[index].size,\n"
- "> +\t\tblk_size);\n"
- "> +\n"
- "> +\treturn 0;\n"
+ "\n"
+ "Rest of the checks look good to me.\n"
+ "\n"
  "> +}\n"
  "> +\n"
  "> +/* Returns the number of partitions in dc_resp or -ERRNO */\n"
@@ -133,6 +135,11 @@
  "> +\t\t.size_out = dc_resp_size,\n"
  "> +\t\t.payload_out = dc_resp,\n"
  "> +\t\t.min_out = 1,\n"
+ "\n"
+ "Why 1?  If a device oddly supported 0 regions I think it would still be 8\n"
+ "to cover the first two fields and the reserved space before region configuration\n"
+ "structure.\n"
+ "\n"
  "> +\t};\n"
  "> +\tint rc;\n"
  "> +\n"
@@ -168,12 +175,20 @@
  "> +\n"
  "> +\tstruct cxl_mbox_get_dc_config_out *dc_resp __free(kfree) =\n"
  "> +\t\t\t\t\tkvmalloc(dc_resp_size, GFP_KERNEL);\n"
+ "\n"
+ "Could we size this one for max possible? (i.e. 8 partitions) with a struct\n"
+ "size and avoid needing vmalloc.  Maybe it is worth the bother.\n"
+ "\n"
  "> +\tif (!dc_resp)\n"
  "> +\t\treturn -ENOMEM;\n"
  "> +\n"
  "> +\t/* Read and check all partition information for validity and potential\n"
+ "\n"
+ "Multi line comment syntax isn't this for this file.\n"
+ "\n"
  "> +\t * debugging; see debug output in cxl_dc_check() */\n"
  "> +\tstart_partition = 0;\n"
+ "Could set at declaration  (up to you)\n"
  "> +\tdo {\n"
  "> +\t\tint rc, i, j;\n"
  "> +\n"
@@ -184,6 +199,9 @@
  "> +\t\t}\n"
  "> +\n"
  "> +\t\tnum_partitions += rc;\n"
+ "\n"
+ "Initialization missing I think.\n"
+ "\n"
  "> +\n"
  "> +\t\tif (num_partitions < 1 || num_partitions > CXL_MAX_DC_PARTITIONS) {\n"
  "> +\t\t\tdev_err(dev, \"Invalid num of dynamic capacity partitions %d\\n\",\n"
@@ -205,182 +223,33 @@
  "> +\t/* Return 1st partition */\n"
  "> +\tdc_info->start = partitions[0].start;\n"
  "> +\tdc_info->size = partitions[0].size;\n"
+ "\n"
+ "I'm not against keeping the complexity above but if all we are going to do is\n"
+ "use the first partition, maybe just ask for that in the first place?\n"
+ "We don't need to check for issues in things we aren't turning on.\n"
+ "\n"
  "> +\tdev_dbg(dev, \"Returning partition 0 %zu size %zu\\n\",\n"
  "> +\t\tdc_info->start, dc_info->size);\n"
  "> +\n"
  "> +\treturn 0;\n"
  "> +}\n"
  "> +EXPORT_SYMBOL_NS_GPL(cxl_dev_dc_identify, \"CXL\");\n"
- "> +\n"
- ">  static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_partition_mode mode)\n"
- ">  {\n"
- ">  \tint i = info->nr_partitions;\n"
- "> @@ -1383,6 +1530,38 @@ int cxl_get_dirty_count(struct cxl_memdev_state *mds, u32 *count)\n"
- ">  }\n"
- ">  EXPORT_SYMBOL_NS_GPL(cxl_get_dirty_count, \"CXL\");\n"
- ">  \n"
- "> +void cxl_configure_dcd(struct cxl_memdev_state *mds, struct cxl_dpa_info *info)\n"
- "> +{\n"
- "> +\tstruct cxl_dc_partition_info dc_info = { 0 };\n"
- "Trivial bit of c stuff that surprised me in another thread the other day that doesn't\n"
- "apply here because of packed nature of structure but...\n"
  "\n"
- " = {}; is defined in c23 (and probably before that in practice) as\n"
- "the \"empty initializer\"\n"
- "> +\tstruct device *dev = mds->cxlds.dev;\n"
- "> +\tsize_t skip;\n"
- "> +\tint rc;\n"
- "> +\n"
- "> +\trc = cxl_dev_dc_identify(&mds->cxlds.cxl_mbox, &dc_info);\n"
- "> +\tif (rc) {\n"
- "> +\t\tdev_warn(dev,\n"
- "> +\t\t\t \"Failed to read Dynamic Capacity config: %d\\n\", rc);\n"
- "> +\t\tcxl_disable_dcd(mds);\n"
- "> +\t\treturn;\n"
- "> +\t}\n"
- "> +\n"
- "> +\t/* Skips between pmem and the dynamic partition are not supported */\n"
- "> +\tskip = dc_info.start - info->size;\n"
- "> +\tif (skip) {\n"
- "> +\t\tdev_warn(dev,\n"
- "> +\t\t\t \"Dynamic Capacity skip from pmem not supported: %zu\\n\",\n"
- "> +\t\t\t skip);\n"
- "> +\t\tcxl_disable_dcd(mds);\n"
- "> +\t\treturn;\n"
- "> +\t}\n"
- "> +\n"
- "> +\tinfo->size += dc_info.size;\n"
- "> +\tdev_dbg(dev, \"Adding dynamic ram partition A; %zu size %zu\\n\",\n"
- "> +\t\tdc_info.start, dc_info.size);\n"
- "> +\tadd_part(info, dc_info.start, dc_info.size, CXL_PARTMODE_DYNAMIC_RAM_A);\n"
- "> +}\n"
- "> +EXPORT_SYMBOL_NS_GPL(cxl_configure_dcd, \"CXL\");\n"
- "> +\n"
- ">  int cxl_arm_dirty_shutdown(struct cxl_memdev_state *mds)\n"
- ">  {\n"
- ">  \tstruct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;\n"
- "> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h\n"
- "> index be8a7dc77719..a9d42210e8a3 100644\n"
- "> --- a/drivers/cxl/cxl.h\n"
- "> +++ b/drivers/cxl/cxl.h\n"
- "> @@ -485,6 +485,7 @@ struct cxl_region_params {\n"
- ">  enum cxl_partition_mode {\n"
- ">  \tCXL_PARTMODE_RAM,\n"
- ">  \tCXL_PARTMODE_PMEM,\n"
- "> +\tCXL_PARTMODE_DYNAMIC_RAM_A,\n"
- ">  };\n"
- ">  \n"
- ">  /*\n"
+ "\n"
+ "\n"
  "> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h\n"
  "> index 394a776954f4..057933128d2c 100644\n"
  "> --- a/drivers/cxl/cxlmem.h\n"
  "> +++ b/drivers/cxl/cxlmem.h\n"
- "> @@ -97,7 +97,7 @@ int devm_cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,\n"
- ">  \t\t\t resource_size_t base, resource_size_t len,\n"
- ">  \t\t\t resource_size_t skipped);\n"
- ">  \n"
- "> -#define CXL_NR_PARTITIONS_MAX 2\n"
- "> +#define CXL_NR_PARTITIONS_MAX 3\n"
- ">  \n"
- ">  struct cxl_dpa_info {\n"
- ">  \tu64 size;\n"
- "> @@ -380,6 +380,7 @@ enum cxl_devtype {\n"
- ">  \tCXL_DEVTYPE_CLASSMEM,\n"
- ">  };\n"
- ">  \n"
- "> +#define CXL_MAX_DC_PARTITIONS 8\n"
- ">  /**\n"
- ">   * struct cxl_dpa_perf - DPA performance property entry\n"
- ">   * @dpa_range: range for DPA address\n"
- "> @@ -722,6 +723,31 @@ struct cxl_mbox_set_shutdown_state_in {\n"
- ">  \tu8 state;\n"
- ">  } __packed;\n"
- ">  \n"
- "> +/* See CXL 3.2 Table 8-178 get dynamic capacity config Input Payload */\n"
- "> +struct cxl_mbox_get_dc_config_in {\n"
- "> +\tu8 partition_count;\n"
- "> +\tu8 start_partition_index;\n"
- "> +} __packed;\n"
- "> +\n"
- "> +/* See CXL 3.2 Table 8-179 get dynamic capacity config Output Payload */\n"
- "> +struct cxl_mbox_get_dc_config_out {\n"
- "> +\tu8 avail_partition_count;\n"
- "> +\tu8 partitions_returned;\n"
- "> +\tu8 rsvd[6];\n"
- "> +\t/* See CXL 3.2 Table 8-180 */\n"
- "> +\tstruct cxl_dc_partition {\n"
- "> +\t\t__le64 base;\n"
- "> +\t\t__le64 decode_length;\n"
- "> +\t\t__le64 length;\n"
- "> +\t\t__le64 block_size;\n"
- "> +\t\t__le32 dsmad_handle;\n"
- "> +\t\tu8 flags;\n"
- "> +\t\tu8 rsvd[3];\n"
- "> +\t} __packed partition[] __counted_by(partitions_returned);\n"
- "> +\t/* Trailing fields unused */\n"
- "> +} __packed;\n"
- "> +#define CXL_DCD_BLOCK_LINE_SIZE 0x40\n"
- "> +\n"
- ">  /* Set Timestamp CXL 3.0 Spec 8.2.9.4.2 */\n"
- ">  struct cxl_mbox_set_timestamp_in {\n"
- ">  \t__le64 timestamp;\n"
- "> @@ -845,9 +871,24 @@ enum {\n"
- ">  int cxl_internal_send_cmd(struct cxl_mailbox *cxl_mbox,\n"
- ">  \t\t\t  struct cxl_mbox_cmd *cmd);\n"
- ">  int cxl_dev_state_identify(struct cxl_memdev_state *mds);\n"
  "> +\n"
  "> +struct cxl_mem_dev_info {\n"
  "> +\tu64 total_bytes;\n"
  "> +\tu64 volatile_bytes;\n"
  "> +\tu64 persistent_bytes;\n"
  "> +};\n"
- "> +\n"
- "> +struct cxl_dc_partition_info {\n"
- "> +\tsize_t start;\n"
- "> +\tsize_t size;\n"
- "> +};\n"
- "> +\n"
- "> +int cxl_dev_dc_identify(struct cxl_mailbox *mbox,\n"
- "> +\t\t\tstruct cxl_dc_partition_info *dc_info);\n"
- ">  int cxl_await_media_ready(struct cxl_dev_state *cxlds);\n"
- ">  int cxl_enumerate_cmds(struct cxl_memdev_state *mds);\n"
- ">  int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);\n"
- "> +void cxl_configure_dcd(struct cxl_memdev_state *mds, struct cxl_dpa_info *info);\n"
- ">  struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev);\n"
- ">  void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,\n"
- ">  \t\t\t\tunsigned long *cmds);\n"
- "> @@ -860,6 +901,17 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,\n"
- ">  \t\t\t    const uuid_t *uuid, union cxl_event *evt);\n"
- ">  int cxl_get_dirty_count(struct cxl_memdev_state *mds, u32 *count);\n"
- ">  int cxl_arm_dirty_shutdown(struct cxl_memdev_state *mds);\n"
- "> +\n"
- "> +static inline bool cxl_dcd_supported(struct cxl_memdev_state *mds)\n"
- "> +{\n"
- "> +\treturn mds->dcd_supported;\n"
- "> +}\n"
- "> +\n"
- "> +static inline void cxl_disable_dcd(struct cxl_memdev_state *mds)\n"
- "> +{\n"
- "> +\tmds->dcd_supported = false;\n"
- "> +}\n"
- "> +\n"
- ">  int cxl_set_timestamp(struct cxl_memdev_state *mds);\n"
- ">  int cxl_poison_state_init(struct cxl_memdev_state *mds);\n"
- ">  int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,\n"
- "> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c\n"
- "> index 7b14a154463c..bc40cf6e2fe9 100644\n"
- "> --- a/drivers/cxl/pci.c\n"
- "> +++ b/drivers/cxl/pci.c\n"
- "> @@ -998,6 +998,9 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)\n"
- ">  \tif (rc)\n"
- ">  \t\treturn rc;\n"
- ">  \n"
- "> +\tif (cxl_dcd_supported(mds))\n"
- "> +\t\tcxl_configure_dcd(mds, &range_info);\n"
- "> +\n"
- ">  \trc = cxl_dpa_setup(cxlds, &range_info);\n"
- ">  \tif (rc)\n"
- ">  \t\treturn rc;\n"
- >
+ "\n"
+ "So far I'm not seeing any use of this. Left over from previous patch\n"
+ "or something that gets used later in the series and so should get\n"
+ introduced with first use?
 
-dfe9eb387f0dc1a1a97f3ae3978e122ca452b9ca00d93ca82a40c55158c3cc80
+9fc82a5a39489c048f646a8c019ddbee6c28cbc0c94ade6e8db8c62c431da541

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox