linux-cxl.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/3] Query and use Partition Info
@ 2021-06-17 22:16 ira.weiny
  2021-06-17 22:16 ` [PATCH V2 1/3] cxl/pci: Store memory capacity values ira.weiny
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: ira.weiny @ 2021-06-17 22:16 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ira Weiny, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

From: Ira Weiny <ira.weiny@intel.com>

Three small patches which query and use Partition Info.  Specifically this
correctly accounts for partitionable space should a device support it.

Changes for V2:
	Feed back from Ben, Dan, and Jonathan
	Remove sysfs patch in favor of using ndctl to query values in the
	future.
	Add patch to create the correct ram/pmem range entries per the spec.

Ira Weiny (3):
  cxl/pci: Store memory capacity values
  cxl/mem: Account for partitionable space in ram/pmem ranges
  cxl/mem: Adjust ram/pmem range to represent DPA ranges

 drivers/cxl/mem.h |   9 ++++
 drivers/cxl/pci.c | 134 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 134 insertions(+), 9 deletions(-)

-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V2 1/3] cxl/pci: Store memory capacity values
  2021-06-17 22:16 [PATCH V2 0/3] Query and use Partition Info ira.weiny
@ 2021-06-17 22:16 ` ira.weiny
  2021-06-18 13:40   ` Jonathan Cameron
  2021-06-17 22:16 ` [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges ira.weiny
  2021-06-17 22:16 ` [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges ira.weiny
  2 siblings, 1 reply; 14+ messages in thread
From: ira.weiny @ 2021-06-17 22:16 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ira Weiny, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

From: Ira Weiny <ira.weiny@intel.com>

The Identify Memory Device command returns information about the
volatile only and persistent only memory capacities.  Store those values
in the cxl_mem structure for later use.  While at it, reuse those
calculations to calculate the ram and pmem ranges.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes for V2:
	Ben
		Change names to be more clear
			total_bytes
			volatile_only_bytes
			persistent_only_bytes
---
 drivers/cxl/mem.h |  4 ++++
 drivers/cxl/pci.c | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
index 13868ff7cadf..9dc34418db36 100644
--- a/drivers/cxl/mem.h
+++ b/drivers/cxl/mem.h
@@ -75,5 +75,9 @@ struct cxl_mem {
 
 	struct range pmem_range;
 	struct range ram_range;
+	u64 total_bytes;
+	u64 volatile_only_bytes;
+	u64 persistent_only_bytes;
+	u64 partition_align_bytes;
 };
 #endif /* __CXL_MEM_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 5a1705b52278..94b7ee08ef67 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -57,6 +57,15 @@ enum opcode {
 	CXL_MBOX_OP_MAX			= 0x10000
 };
 
+/*
+ * CXL 2.0 - Memory capacity multiplier
+ * See Section 8.2.9.5
+ *
+ * Volatile, Persistent, and Partition capacities are specified to be in
+ * multiples of 256MB - define a multiplier to convert to/from bytes.
+ */
+#define CXL_CAPACITY_MULTIPLIER SZ_256M
+
 /**
  * struct mbox_cmd - A command to be submitted to hardware.
  * @opcode: (input) The command set and command submitted to hardware.
@@ -1542,16 +1551,37 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
 	if (rc < 0)
 		return rc;
 
+	cxlm->total_bytes = le64_to_cpu(id.total_capacity);
+	cxlm->total_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	cxlm->volatile_only_bytes = le64_to_cpu(id.volatile_capacity);
+	cxlm->volatile_only_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	cxlm->persistent_only_bytes = le64_to_cpu(id.persistent_capacity);
+	cxlm->persistent_only_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	cxlm->partition_align_bytes = le64_to_cpu(id.partition_align);
+	cxlm->partition_align_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	dev_dbg(&cxlm->pdev->dev, "Identify Memory Device\n"
+		"     total_bytes = %#llx\n"
+		"     volatile_only_bytes = %#llx\n"
+		"     persistent_only_bytes = %#llx\n"
+		"     partition_align_bytes = %#llx\n",
+			cxlm->total_bytes,
+			cxlm->volatile_only_bytes,
+			cxlm->persistent_only_bytes,
+			cxlm->partition_align_bytes);
+
 	/*
 	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
 	 * For now, only the capacity is exported in sysfs
 	 */
 	cxlm->ram_range.start = 0;
-	cxlm->ram_range.end = le64_to_cpu(id.volatile_capacity) * SZ_256M - 1;
+	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
 
 	cxlm->pmem_range.start = 0;
-	cxlm->pmem_range.end =
-		le64_to_cpu(id.persistent_capacity) * SZ_256M - 1;
+	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
 
 	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
 	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-17 22:16 [PATCH V2 0/3] Query and use Partition Info ira.weiny
  2021-06-17 22:16 ` [PATCH V2 1/3] cxl/pci: Store memory capacity values ira.weiny
@ 2021-06-17 22:16 ` ira.weiny
  2021-06-18 13:59   ` Jonathan Cameron
  2021-06-18 16:31   ` [PATCH V3] " ira.weiny
  2021-06-17 22:16 ` [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges ira.weiny
  2 siblings, 2 replies; 14+ messages in thread
From: ira.weiny @ 2021-06-17 22:16 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ira Weiny, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

From: Ira Weiny <ira.weiny@intel.com>

Memory devices may specify volatile only, persistent only, and
partitionable space which when added together result in a total capacity.

If Identify Memory Device.Partition Alignment != 0 the device supports
partitionable space.  This partitionable space can be split between
volatile and persistent space.  The total volatile and persistent sizes
are reported in Get Partition Info.  ie

	active volatile memory = volatile only + partitionable volatile
	active persistent memory = persistent only + partitionable persistent

Define cxl_mem_get_partition(), check for partitionable support, and use
cxl_mem_get_partition() if applicable.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes for V2:
	Jonathan
		Account for Get Partition Info being optional
		Fix documentation
	Dan
		Store the active capacities in struct cxl_mem
		Make cxl_mem_get_partition() static until there is a
		need to export it.
---
 drivers/cxl/mem.h |  5 +++
 drivers/cxl/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
index 9dc34418db36..8d7bd966a298 100644
--- a/drivers/cxl/mem.h
+++ b/drivers/cxl/mem.h
@@ -79,5 +79,10 @@ struct cxl_mem {
 	u64 volatile_only_bytes;
 	u64 persistent_only_bytes;
 	u64 partition_align_bytes;
+
+	u64 active_volatile_bytes;
+	u64 active_persistent_bytes;
+	u64 next_volatile_bytes;
+	u64 next_persistent_bytes;
 };
 #endif /* __CXL_MEM_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 94b7ee08ef67..341885345b53 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1455,6 +1455,55 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
 	return ret;
 }
 
+/**
+ * cxl_mem_get_partition_info - Get partition info
+ * @cxlm: The device to act on
+ * @active_volatile_bytes: returned active volatile capacity; in bytes
+ * @active_persistent_bytes: returned active persistent capacity; in bytes
+ * @next_volatile_bytes: return next volatile capacity; in bytes
+ * @next_persistent_bytes: return next persistent capacity; in bytes
+ *
+ * Retrieve the current partition info for the device specified.  The active
+ * values are the current capacity in bytes.  If not 0, the 'next' values are
+ * the pending values, in bytes, which take affect on next cold reset.
+ *
+ * Return: 0 if no error: or the result of the mailbox command.
+ *
+ * See CXL @8.2.9.5.2.1 Get Partition Info
+ */
+static int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
+				      u64 *active_volatile_bytes,
+				      u64 *active_persistent_bytes,
+				      u64 *next_volatile_bytes,
+				      u64 *next_persistent_bytes)
+{
+	struct cxl_mbox_get_partition_info {
+		u64 active_volatile_cap;
+		u64 active_persistent_cap;
+		u64 next_volatile_cap;
+		u64 next_persistent_cap;
+	} __packed pi;
+	int rc;
+
+	rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
+				   NULL, 0, &pi, sizeof(pi));
+
+	if (rc)
+		return rc;
+
+	*active_volatile_bytes = le64_to_cpu(pi.active_volatile_cap);
+	*active_persistent_bytes = le64_to_cpu(pi.active_persistent_cap);
+	*next_volatile_bytes = le64_to_cpu(pi.next_volatile_cap);
+	*next_persistent_bytes = le64_to_cpu(pi.next_volatile_cap);
+
+	*active_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*active_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*next_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*next_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	return 0;
+}
+
 /**
  * cxl_mem_enumerate_cmds() - Enumerate commands for a device.
  * @cxlm: The device.
@@ -1573,18 +1622,53 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
 			cxlm->persistent_only_bytes,
 			cxlm->partition_align_bytes);
 
+	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
+	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
+
+	return 0;
+}
+
+static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
+{
+	int rc;
+
+	if (cxlm->partition_align_bytes == 0) {
+		cxlm->ram_range.start = 0;
+		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
+		cxlm->pmem_range.start = 0;
+		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
+		return 0;
+	}
+
+	rc = cxl_mem_get_partition_info(cxlm,
+					&cxlm->active_volatile_bytes,
+					&cxlm->active_persistent_bytes,
+					&cxlm->next_volatile_bytes,
+					&cxlm->next_persistent_bytes);
+	if (rc < 0) {
+		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
+		return rc;
+	}
+
+	dev_dbg(&cxlm->pdev->dev, "Get Partition Info\n"
+		"     active_volatile_bytes = %#llx\n"
+		"     active_persistent_bytes = %#llx\n"
+		"     next_volatile_bytes = %#llx\n"
+		"     next_persistent_bytes = %#llx\n",
+			cxlm->active_volatile_bytes,
+			cxlm->active_persistent_bytes,
+			cxlm->next_volatile_bytes,
+			cxlm->next_persistent_bytes);
+
 	/*
 	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
 	 * For now, only the capacity is exported in sysfs
 	 */
 	cxlm->ram_range.start = 0;
-	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
+	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
 
 	cxlm->pmem_range.start = 0;
-	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
-
-	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
-	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
+	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
 
 	return 0;
 }
@@ -1618,6 +1702,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = cxl_mem_create_range_info(cxlm);
+	if (rc)
+		return rc;
+
 	return cxl_mem_add_memdev(cxlm);
 }
 
-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges
  2021-06-17 22:16 [PATCH V2 0/3] Query and use Partition Info ira.weiny
  2021-06-17 22:16 ` [PATCH V2 1/3] cxl/pci: Store memory capacity values ira.weiny
  2021-06-17 22:16 ` [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges ira.weiny
@ 2021-06-17 22:16 ` ira.weiny
  2021-06-18 14:03   ` Jonathan Cameron
  2021-06-21 19:54   ` [PATCH V3] " ira.weiny
  2 siblings, 2 replies; 14+ messages in thread
From: ira.weiny @ 2021-06-17 22:16 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ira Weiny, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

From: Ira Weiny <ira.weiny@intel.com>

CXL spec defines the volatile DPA range to be 0 to Volatile memory size.
It further defines the persistent DPA range to follow directly after the
end of the Volatile DPA through the persistent memory size.  Essentially

Volatile DPA range   = [0, Volatile size)
Persistent DPA range = [Volatile size, Volatile size + Persistent size)

Adjust the pmem_range start to reflect this and remote the TODO.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/cxl/pci.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 341885345b53..a1fd7923dfb9 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1635,8 +1635,9 @@ static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
 	if (cxlm->partition_align_bytes == 0) {
 		cxlm->ram_range.start = 0;
 		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
-		cxlm->pmem_range.start = 0;
-		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
+		cxlm->pmem_range.start = cxlm->volatile_only_bytes;
+		cxlm->pmem_range.end = cxlm->volatile_only_bytes +
+					cxlm->persistent_only_bytes - 1;
 		return 0;
 	}
 
@@ -1660,15 +1661,12 @@ static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
 			cxlm->next_volatile_bytes,
 			cxlm->next_persistent_bytes);
 
-	/*
-	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
-	 * For now, only the capacity is exported in sysfs
-	 */
 	cxlm->ram_range.start = 0;
 	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
 
-	cxlm->pmem_range.start = 0;
-	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
+	cxlm->pmem_range.start = cxlm->active_volatile_bytes;
+	cxlm->pmem_range.end = cxlm->active_volatile_bytes +
+				cxlm->active_persistent_bytes - 1;
 
 	return 0;
 }
-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 1/3] cxl/pci: Store memory capacity values
  2021-06-17 22:16 ` [PATCH V2 1/3] cxl/pci: Store memory capacity values ira.weiny
@ 2021-06-18 13:40   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2021-06-18 13:40 UTC (permalink / raw)
  To: ira.weiny
  Cc: Dan Williams, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

On Thu, 17 Jun 2021 15:16:18 -0700
<ira.weiny@intel.com> wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> The Identify Memory Device command returns information about the
> volatile only and persistent only memory capacities.  Store those values
> in the cxl_mem structure for later use.  While at it, reuse those
> calculations to calculate the ram and pmem ranges.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
New naming is indeed clearer.
FWIW
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> Changes for V2:
> 	Ben
> 		Change names to be more clear
> 			total_bytes
> 			volatile_only_bytes
> 			persistent_only_bytes
> ---
>  drivers/cxl/mem.h |  4 ++++
>  drivers/cxl/pci.c | 36 +++++++++++++++++++++++++++++++++---
>  2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
> index 13868ff7cadf..9dc34418db36 100644
> --- a/drivers/cxl/mem.h
> +++ b/drivers/cxl/mem.h
> @@ -75,5 +75,9 @@ struct cxl_mem {
>  
>  	struct range pmem_range;
>  	struct range ram_range;
> +	u64 total_bytes;
> +	u64 volatile_only_bytes;
> +	u64 persistent_only_bytes;
> +	u64 partition_align_bytes;
>  };
>  #endif /* __CXL_MEM_H__ */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 5a1705b52278..94b7ee08ef67 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -57,6 +57,15 @@ enum opcode {
>  	CXL_MBOX_OP_MAX			= 0x10000
>  };
>  
> +/*
> + * CXL 2.0 - Memory capacity multiplier
> + * See Section 8.2.9.5
> + *
> + * Volatile, Persistent, and Partition capacities are specified to be in
> + * multiples of 256MB - define a multiplier to convert to/from bytes.
> + */
> +#define CXL_CAPACITY_MULTIPLIER SZ_256M
> +
>  /**
>   * struct mbox_cmd - A command to be submitted to hardware.
>   * @opcode: (input) The command set and command submitted to hardware.
> @@ -1542,16 +1551,37 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
>  	if (rc < 0)
>  		return rc;
>  
> +	cxlm->total_bytes = le64_to_cpu(id.total_capacity);
> +	cxlm->total_bytes *= CXL_CAPACITY_MULTIPLIER;
> +
> +	cxlm->volatile_only_bytes = le64_to_cpu(id.volatile_capacity);
> +	cxlm->volatile_only_bytes *= CXL_CAPACITY_MULTIPLIER;
> +
> +	cxlm->persistent_only_bytes = le64_to_cpu(id.persistent_capacity);
> +	cxlm->persistent_only_bytes *= CXL_CAPACITY_MULTIPLIER;
> +
> +	cxlm->partition_align_bytes = le64_to_cpu(id.partition_align);
> +	cxlm->partition_align_bytes *= CXL_CAPACITY_MULTIPLIER;
> +
> +	dev_dbg(&cxlm->pdev->dev, "Identify Memory Device\n"
> +		"     total_bytes = %#llx\n"
> +		"     volatile_only_bytes = %#llx\n"
> +		"     persistent_only_bytes = %#llx\n"
> +		"     partition_align_bytes = %#llx\n",
> +			cxlm->total_bytes,
> +			cxlm->volatile_only_bytes,
> +			cxlm->persistent_only_bytes,
> +			cxlm->partition_align_bytes);
> +
>  	/*
>  	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
>  	 * For now, only the capacity is exported in sysfs
>  	 */
>  	cxlm->ram_range.start = 0;
> -	cxlm->ram_range.end = le64_to_cpu(id.volatile_capacity) * SZ_256M - 1;
> +	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
>  
>  	cxlm->pmem_range.start = 0;
> -	cxlm->pmem_range.end =
> -		le64_to_cpu(id.persistent_capacity) * SZ_256M - 1;
> +	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
>  
>  	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
>  	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-17 22:16 ` [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges ira.weiny
@ 2021-06-18 13:59   ` Jonathan Cameron
  2021-06-18 16:30     ` Ira Weiny
  2021-06-18 16:31   ` [PATCH V3] " ira.weiny
  1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2021-06-18 13:59 UTC (permalink / raw)
  To: ira.weiny
  Cc: Dan Williams, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

On Thu, 17 Jun 2021 15:16:19 -0700
<ira.weiny@intel.com> wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> Memory devices may specify volatile only, persistent only, and
> partitionable space which when added together result in a total capacity.
> 
> If Identify Memory Device.Partition Alignment != 0 the device supports
> partitionable space.  This partitionable space can be split between
> volatile and persistent space.  The total volatile and persistent sizes
> are reported in Get Partition Info.  ie
> 
> 	active volatile memory = volatile only + partitionable volatile
> 	active persistent memory = persistent only + partitionable persistent
> 
> Define cxl_mem_get_partition(), check for partitionable support, and use
> cxl_mem_get_partition() if applicable.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Trivial editorial stuff inline.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> Changes for V2:
> 	Jonathan
> 		Account for Get Partition Info being optional
> 		Fix documentation
> 	Dan
> 		Store the active capacities in struct cxl_mem
> 		Make cxl_mem_get_partition() static until there is a
> 		need to export it.
> ---
>  drivers/cxl/mem.h |  5 +++
>  drivers/cxl/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 98 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
> index 9dc34418db36..8d7bd966a298 100644
> --- a/drivers/cxl/mem.h
> +++ b/drivers/cxl/mem.h
> @@ -79,5 +79,10 @@ struct cxl_mem {
>  	u64 volatile_only_bytes;
>  	u64 persistent_only_bytes;
>  	u64 partition_align_bytes;
> +
> +	u64 active_volatile_bytes;
> +	u64 active_persistent_bytes;
> +	u64 next_volatile_bytes;
> +	u64 next_persistent_bytes;
>  };
>  #endif /* __CXL_MEM_H__ */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 94b7ee08ef67..341885345b53 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1455,6 +1455,55 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
>  	return ret;
>  }
>  
> +/**
> + * cxl_mem_get_partition_info - Get partition info
> + * @cxlm: The device to act on
> + * @active_volatile_bytes: returned active volatile capacity; in bytes

Perhaps over documented?  Would be very very odd if _bytes attribute wasn't
in bytes so maybe don't need to state it in the comment?

> + * @active_persistent_bytes: returned active persistent capacity; in bytes
> + * @next_volatile_bytes: return next volatile capacity; in bytes
> + * @next_persistent_bytes: return next persistent capacity; in bytes
> + *
> + * Retrieve the current partition info for the device specified.  The active
> + * values are the current capacity in bytes.  If not 0, the 'next' values are

Definitely don't need to say they are in bytes again.

> + * the pending values, in bytes, which take affect on next cold reset.
> + *
> + * Return: 0 if no error: or the result of the mailbox command.
> + *
> + * See CXL @8.2.9.5.2.1 Get Partition Info
> + */
> +static int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
> +				      u64 *active_volatile_bytes,
> +				      u64 *active_persistent_bytes,
> +				      u64 *next_volatile_bytes,
> +				      u64 *next_persistent_bytes)
> +{
> +	struct cxl_mbox_get_partition_info {
> +		u64 active_volatile_cap;
> +		u64 active_persistent_cap;
> +		u64 next_volatile_cap;
> +		u64 next_persistent_cap;
> +	} __packed pi;
> +	int rc;
> +
> +	rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
> +				   NULL, 0, &pi, sizeof(pi));
> +

In my view, slightly better to not have a blank line between statement
and error handling.

> +	if (rc)
> +		return rc;
> +
> +	*active_volatile_bytes = le64_to_cpu(pi.active_volatile_cap);
> +	*active_persistent_bytes = le64_to_cpu(pi.active_persistent_cap);
> +	*next_volatile_bytes = le64_to_cpu(pi.next_volatile_cap);
> +	*next_persistent_bytes = le64_to_cpu(pi.next_volatile_cap);
> +
> +	*active_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
> +	*active_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
> +	*next_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
> +	*next_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
> +
> +	return 0;
> +}
> +
>  /**
>   * cxl_mem_enumerate_cmds() - Enumerate commands for a device.
>   * @cxlm: The device.
> @@ -1573,18 +1622,53 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
>  			cxlm->persistent_only_bytes,
>  			cxlm->partition_align_bytes);
>  
> +	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
> +	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
> +
> +	return 0;
> +}
> +
> +static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
> +{
> +	int rc;
> +
> +	if (cxlm->partition_align_bytes == 0) {
> +		cxlm->ram_range.start = 0;
> +		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> +		cxlm->pmem_range.start = 0;
> +		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> +		return 0;
> +	}
> +
> +	rc = cxl_mem_get_partition_info(cxlm,
> +					&cxlm->active_volatile_bytes,
> +					&cxlm->active_persistent_bytes,
> +					&cxlm->next_volatile_bytes,
> +					&cxlm->next_persistent_bytes);
> +	if (rc < 0) {
> +		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
> +		return rc;
> +	}
> +
> +	dev_dbg(&cxlm->pdev->dev, "Get Partition Info\n"
> +		"     active_volatile_bytes = %#llx\n"
> +		"     active_persistent_bytes = %#llx\n"
> +		"     next_volatile_bytes = %#llx\n"
> +		"     next_persistent_bytes = %#llx\n",
> +			cxlm->active_volatile_bytes,
> +			cxlm->active_persistent_bytes,
> +			cxlm->next_volatile_bytes,
> +			cxlm->next_persistent_bytes);
> +
>  	/*
>  	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
>  	 * For now, only the capacity is exported in sysfs
>  	 */
>  	cxlm->ram_range.start = 0;
> -	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> +	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
>  
>  	cxlm->pmem_range.start = 0;
> -	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> -
> -	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
> -	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
> +	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
>  
>  	return 0;
>  }
> @@ -1618,6 +1702,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> +	rc = cxl_mem_create_range_info(cxlm);
> +	if (rc)
> +		return rc;
> +
>  	return cxl_mem_add_memdev(cxlm);
>  }
>  


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges
  2021-06-17 22:16 ` [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges ira.weiny
@ 2021-06-18 14:03   ` Jonathan Cameron
  2021-06-21 19:54   ` [PATCH V3] " ira.weiny
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2021-06-18 14:03 UTC (permalink / raw)
  To: ira.weiny
  Cc: Dan Williams, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

On Thu, 17 Jun 2021 15:16:20 -0700
<ira.weiny@intel.com> wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> CXL spec defines the volatile DPA range to be 0 to Volatile memory size.
> It further defines the persistent DPA range to follow directly after the
> end of the Volatile DPA through the persistent memory size.

Reference would be good for anyone sanity checking against the spec. I think it's
Table 176 that says this.

> Essentially
> 
> Volatile DPA range   = [0, Volatile size)
> Persistent DPA range = [Volatile size, Volatile size + Persistent size)
> 
> Adjust the pmem_range start to reflect this and remote the TODO.

remove

> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/pci.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 341885345b53..a1fd7923dfb9 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1635,8 +1635,9 @@ static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
>  	if (cxlm->partition_align_bytes == 0) {
>  		cxlm->ram_range.start = 0;
>  		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> -		cxlm->pmem_range.start = 0;
> -		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> +		cxlm->pmem_range.start = cxlm->volatile_only_bytes;
> +		cxlm->pmem_range.end = cxlm->volatile_only_bytes +
> +					cxlm->persistent_only_bytes - 1;
>  		return 0;
>  	}
>  
> @@ -1660,15 +1661,12 @@ static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
>  			cxlm->next_volatile_bytes,
>  			cxlm->next_persistent_bytes);
>  
> -	/*
> -	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
> -	 * For now, only the capacity is exported in sysfs
> -	 */
>  	cxlm->ram_range.start = 0;
>  	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
>  
> -	cxlm->pmem_range.start = 0;
> -	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
> +	cxlm->pmem_range.start = cxlm->active_volatile_bytes;
> +	cxlm->pmem_range.end = cxlm->active_volatile_bytes +
> +				cxlm->active_persistent_bytes - 1;
>  
>  	return 0;
>  }


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-18 13:59   ` Jonathan Cameron
@ 2021-06-18 16:30     ` Ira Weiny
  0 siblings, 0 replies; 14+ messages in thread
From: Ira Weiny @ 2021-06-18 16:30 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Dan Williams, Alison Schofield, Vishal Verma, Ben Widawsky, linux-cxl

On Fri, Jun 18, 2021 at 02:59:02PM +0100, Jonathan Cameron wrote:
> On Thu, 17 Jun 2021 15:16:19 -0700
> <ira.weiny@intel.com> wrote:
> 
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > Memory devices may specify volatile only, persistent only, and
> > partitionable space which when added together result in a total capacity.
> > 
> > If Identify Memory Device.Partition Alignment != 0 the device supports
> > partitionable space.  This partitionable space can be split between
> > volatile and persistent space.  The total volatile and persistent sizes
> > are reported in Get Partition Info.  ie
> > 
> > 	active volatile memory = volatile only + partitionable volatile
> > 	active persistent memory = persistent only + partitionable persistent
> > 
> > Define cxl_mem_get_partition(), check for partitionable support, and use
> > cxl_mem_get_partition() if applicable.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> Trivial editorial stuff inline.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks!

> 
> > 
> > ---
> > Changes for V2:
> > 	Jonathan
> > 		Account for Get Partition Info being optional
> > 		Fix documentation
> > 	Dan
> > 		Store the active capacities in struct cxl_mem
> > 		Make cxl_mem_get_partition() static until there is a
> > 		need to export it.
> > ---
> >  drivers/cxl/mem.h |  5 +++
> >  drivers/cxl/pci.c | 98 ++++++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 98 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
> > index 9dc34418db36..8d7bd966a298 100644
> > --- a/drivers/cxl/mem.h
> > +++ b/drivers/cxl/mem.h
> > @@ -79,5 +79,10 @@ struct cxl_mem {
> >  	u64 volatile_only_bytes;
> >  	u64 persistent_only_bytes;
> >  	u64 partition_align_bytes;
> > +
> > +	u64 active_volatile_bytes;
> > +	u64 active_persistent_bytes;
> > +	u64 next_volatile_bytes;
> > +	u64 next_persistent_bytes;
> >  };
> >  #endif /* __CXL_MEM_H__ */
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index 94b7ee08ef67..341885345b53 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -1455,6 +1455,55 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
> >  	return ret;
> >  }
> >  
> > +/**
> > + * cxl_mem_get_partition_info - Get partition info
> > + * @cxlm: The device to act on
> > + * @active_volatile_bytes: returned active volatile capacity; in bytes
> 
> Perhaps over documented?  Would be very very odd if _bytes attribute wasn't
> in bytes so maybe don't need to state it in the comment?

Ok...  I keep saying this to myself because all the table values are in 256MB
increments so I just kept typing it!!!  :-D

I'll remove the redundancies...

> 
> > + * @active_persistent_bytes: returned active persistent capacity; in bytes
> > + * @next_volatile_bytes: return next volatile capacity; in bytes
> > + * @next_persistent_bytes: return next persistent capacity; in bytes
> > + *
> > + * Retrieve the current partition info for the device specified.  The active
> > + * values are the current capacity in bytes.  If not 0, the 'next' values are
> 
> Definitely don't need to say they are in bytes again.

Sure...

> 
> > + * the pending values, in bytes, which take affect on next cold reset.
> > + *
> > + * Return: 0 if no error: or the result of the mailbox command.
> > + *
> > + * See CXL @8.2.9.5.2.1 Get Partition Info
> > + */
> > +static int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
> > +				      u64 *active_volatile_bytes,
> > +				      u64 *active_persistent_bytes,
> > +				      u64 *next_volatile_bytes,
> > +				      u64 *next_persistent_bytes)
> > +{
> > +	struct cxl_mbox_get_partition_info {
> > +		u64 active_volatile_cap;
> > +		u64 active_persistent_cap;
> > +		u64 next_volatile_cap;
> > +		u64 next_persistent_cap;
> > +	} __packed pi;
> > +	int rc;
> > +
> > +	rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
> > +				   NULL, 0, &pi, sizeof(pi));
> > +
> 
> In my view, slightly better to not have a blank line between statement
> and error handling.

Fair enough.  I go back and forth depending on how the code looks.

All changed, v3 comming,
Ira

> 
> > +	if (rc)
> > +		return rc;
> > +
> > +	*active_volatile_bytes = le64_to_cpu(pi.active_volatile_cap);
> > +	*active_persistent_bytes = le64_to_cpu(pi.active_persistent_cap);
> > +	*next_volatile_bytes = le64_to_cpu(pi.next_volatile_cap);
> > +	*next_persistent_bytes = le64_to_cpu(pi.next_volatile_cap);
> > +
> > +	*active_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +	*active_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +	*next_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +	*next_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * cxl_mem_enumerate_cmds() - Enumerate commands for a device.
> >   * @cxlm: The device.
> > @@ -1573,18 +1622,53 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
> >  			cxlm->persistent_only_bytes,
> >  			cxlm->partition_align_bytes);
> >  
> > +	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
> > +	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
> > +
> > +	return 0;
> > +}
> > +
> > +static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
> > +{
> > +	int rc;
> > +
> > +	if (cxlm->partition_align_bytes == 0) {
> > +		cxlm->ram_range.start = 0;
> > +		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> > +		cxlm->pmem_range.start = 0;
> > +		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> > +		return 0;
> > +	}
> > +
> > +	rc = cxl_mem_get_partition_info(cxlm,
> > +					&cxlm->active_volatile_bytes,
> > +					&cxlm->active_persistent_bytes,
> > +					&cxlm->next_volatile_bytes,
> > +					&cxlm->next_persistent_bytes);
> > +	if (rc < 0) {
> > +		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
> > +		return rc;
> > +	}
> > +
> > +	dev_dbg(&cxlm->pdev->dev, "Get Partition Info\n"
> > +		"     active_volatile_bytes = %#llx\n"
> > +		"     active_persistent_bytes = %#llx\n"
> > +		"     next_volatile_bytes = %#llx\n"
> > +		"     next_persistent_bytes = %#llx\n",
> > +			cxlm->active_volatile_bytes,
> > +			cxlm->active_persistent_bytes,
> > +			cxlm->next_volatile_bytes,
> > +			cxlm->next_persistent_bytes);
> > +
> >  	/*
> >  	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
> >  	 * For now, only the capacity is exported in sysfs
> >  	 */
> >  	cxlm->ram_range.start = 0;
> > -	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> > +	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
> >  
> >  	cxlm->pmem_range.start = 0;
> > -	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> > -
> > -	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
> > -	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
> > +	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
> >  
> >  	return 0;
> >  }
> > @@ -1618,6 +1702,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >  	if (rc)
> >  		return rc;
> >  
> > +	rc = cxl_mem_create_range_info(cxlm);
> > +	if (rc)
> > +		return rc;
> > +
> >  	return cxl_mem_add_memdev(cxlm);
> >  }
> >  
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-17 22:16 ` [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges ira.weiny
  2021-06-18 13:59   ` Jonathan Cameron
@ 2021-06-18 16:31   ` ira.weiny
  2021-06-18 16:58     ` Ben Widawsky
  2021-08-11  1:49     ` [PATCH v4 2/3] " Dan Williams
  1 sibling, 2 replies; 14+ messages in thread
From: ira.weiny @ 2021-06-18 16:31 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ira Weiny, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ben Widawsky, linux-cxl

From: Ira Weiny <ira.weiny@intel.com>

Memory devices may specify volatile only, persistent only, and
partitionable space which when added together result in a total capacity.

If Identify Memory Device.Partition Alignment != 0 the device supports
partitionable space.  This partitionable space can be split between
volatile and persistent space.  The total volatile and persistent sizes
are reported in Get Partition Info.  ie

	active volatile memory = volatile only + partitionable volatile
	active persistent memory = persistent only + partitionable persistent

Define cxl_mem_get_partition(), check for partitionable support, and use
cxl_mem_get_partition() if applicable.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes for V3:
	Johnathan
		Stop over documenting 'in bytes'...  :-D

Changes for V2:
	Jonathan
		Account for Get Partition Info being optional
		Fix documentation
	Dan
		Store the active capacities in struct cxl_mem
		Make cxl_mem_get_partition() static until there is a
		need to export it.
---
 drivers/cxl/mem.h |  5 +++
 drivers/cxl/pci.c | 96 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
index 9dc34418db36..8d7bd966a298 100644
--- a/drivers/cxl/mem.h
+++ b/drivers/cxl/mem.h
@@ -79,5 +79,10 @@ struct cxl_mem {
 	u64 volatile_only_bytes;
 	u64 persistent_only_bytes;
 	u64 partition_align_bytes;
+
+	u64 active_volatile_bytes;
+	u64 active_persistent_bytes;
+	u64 next_volatile_bytes;
+	u64 next_persistent_bytes;
 };
 #endif /* __CXL_MEM_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 94b7ee08ef67..3c8b7aa8c273 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1455,6 +1455,53 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
 	return ret;
 }
 
+/**
+ * cxl_mem_get_partition_info - Get partition info
+ * @cxlm: The device to act on
+ * @active_volatile_bytes: returned active volatile capacity
+ * @active_persistent_bytes: returned active persistent capacity
+ * @next_volatile_bytes: return next volatile capacity
+ * @next_persistent_bytes: return next persistent capacity
+ *
+ * Retrieve the current partition info for the device specified.  If not 0, the
+ * 'next' values are pending and take affect on next cold reset.
+ *
+ * Return: 0 if no error: or the result of the mailbox command.
+ *
+ * See CXL @8.2.9.5.2.1 Get Partition Info
+ */
+static int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
+				      u64 *active_volatile_bytes,
+				      u64 *active_persistent_bytes,
+				      u64 *next_volatile_bytes,
+				      u64 *next_persistent_bytes)
+{
+	struct cxl_mbox_get_partition_info {
+		u64 active_volatile_cap;
+		u64 active_persistent_cap;
+		u64 next_volatile_cap;
+		u64 next_persistent_cap;
+	} __packed pi;
+	int rc;
+
+	rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
+				   NULL, 0, &pi, sizeof(pi));
+	if (rc)
+		return rc;
+
+	*active_volatile_bytes = le64_to_cpu(pi.active_volatile_cap);
+	*active_persistent_bytes = le64_to_cpu(pi.active_persistent_cap);
+	*next_volatile_bytes = le64_to_cpu(pi.next_volatile_cap);
+	*next_persistent_bytes = le64_to_cpu(pi.next_volatile_cap);
+
+	*active_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*active_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*next_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*next_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	return 0;
+}
+
 /**
  * cxl_mem_enumerate_cmds() - Enumerate commands for a device.
  * @cxlm: The device.
@@ -1573,18 +1620,53 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
 			cxlm->persistent_only_bytes,
 			cxlm->partition_align_bytes);
 
+	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
+	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
+
+	return 0;
+}
+
+static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
+{
+	int rc;
+
+	if (cxlm->partition_align_bytes == 0) {
+		cxlm->ram_range.start = 0;
+		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
+		cxlm->pmem_range.start = 0;
+		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
+		return 0;
+	}
+
+	rc = cxl_mem_get_partition_info(cxlm,
+					&cxlm->active_volatile_bytes,
+					&cxlm->active_persistent_bytes,
+					&cxlm->next_volatile_bytes,
+					&cxlm->next_persistent_bytes);
+	if (rc < 0) {
+		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
+		return rc;
+	}
+
+	dev_dbg(&cxlm->pdev->dev, "Get Partition Info\n"
+		"     active_volatile_bytes = %#llx\n"
+		"     active_persistent_bytes = %#llx\n"
+		"     next_volatile_bytes = %#llx\n"
+		"     next_persistent_bytes = %#llx\n",
+			cxlm->active_volatile_bytes,
+			cxlm->active_persistent_bytes,
+			cxlm->next_volatile_bytes,
+			cxlm->next_persistent_bytes);
+
 	/*
 	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
 	 * For now, only the capacity is exported in sysfs
 	 */
 	cxlm->ram_range.start = 0;
-	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
+	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
 
 	cxlm->pmem_range.start = 0;
-	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
-
-	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
-	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
+	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
 
 	return 0;
 }
@@ -1618,6 +1700,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = cxl_mem_create_range_info(cxlm);
+	if (rc)
+		return rc;
+
 	return cxl_mem_add_memdev(cxlm);
 }
 
-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-18 16:31   ` [PATCH V3] " ira.weiny
@ 2021-06-18 16:58     ` Ben Widawsky
  2021-06-18 18:48       ` Ira Weiny
  2021-08-11  1:49     ` [PATCH v4 2/3] " Dan Williams
  1 sibling, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2021-06-18 16:58 UTC (permalink / raw)
  To: ira.weiny
  Cc: Dan Williams, Jonathan Cameron, Alison Schofield, Vishal Verma,
	linux-cxl

On 21-06-18 09:31:29, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Memory devices may specify volatile only, persistent only, and
> partitionable space which when added together result in a total capacity.
> 
> If Identify Memory Device.Partition Alignment != 0 the device supports
> partitionable space.  This partitionable space can be split between
> volatile and persistent space.  The total volatile and persistent sizes
> are reported in Get Partition Info.  ie
> 
> 	active volatile memory = volatile only + partitionable volatile
> 	active persistent memory = persistent only + partitionable persistent
> 
> Define cxl_mem_get_partition(), check for partitionable support, and use
> cxl_mem_get_partition() if applicable.

This doesn't look right to me, but I'll happily stand corrected.

> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes for V3:
> 	Johnathan
> 		Stop over documenting 'in bytes'...  :-D
> 
> Changes for V2:
> 	Jonathan
> 		Account for Get Partition Info being optional
> 		Fix documentation
> 	Dan
> 		Store the active capacities in struct cxl_mem
> 		Make cxl_mem_get_partition() static until there is a
> 		need to export it.
> ---
>  drivers/cxl/mem.h |  5 +++
>  drivers/cxl/pci.c | 96 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 96 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h
> index 9dc34418db36..8d7bd966a298 100644
> --- a/drivers/cxl/mem.h
> +++ b/drivers/cxl/mem.h
> @@ -79,5 +79,10 @@ struct cxl_mem {
>  	u64 volatile_only_bytes;
>  	u64 persistent_only_bytes;
>  	u64 partition_align_bytes;
> +
> +	u64 active_volatile_bytes;
> +	u64 active_persistent_bytes;
> +	u64 next_volatile_bytes;
> +	u64 next_persistent_bytes;
>  };
>  #endif /* __CXL_MEM_H__ */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 94b7ee08ef67..3c8b7aa8c273 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1455,6 +1455,53 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
>  	return ret;
>  }
>  
> +/**
> + * cxl_mem_get_partition_info - Get partition info
> + * @cxlm: The device to act on
> + * @active_volatile_bytes: returned active volatile capacity
> + * @active_persistent_bytes: returned active persistent capacity
> + * @next_volatile_bytes: return next volatile capacity
> + * @next_persistent_bytes: return next persistent capacity
> + *
> + * Retrieve the current partition info for the device specified.  If not 0, the
> + * 'next' values are pending and take affect on next cold reset.
> + *
> + * Return: 0 if no error: or the result of the mailbox command.
> + *
> + * See CXL @8.2.9.5.2.1 Get Partition Info
> + */
> +static int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
> +				      u64 *active_volatile_bytes,
> +				      u64 *active_persistent_bytes,
> +				      u64 *next_volatile_bytes,
> +				      u64 *next_persistent_bytes)
> +{
> +	struct cxl_mbox_get_partition_info {
> +		u64 active_volatile_cap;
> +		u64 active_persistent_cap;
> +		u64 next_volatile_cap;
> +		u64 next_persistent_cap;
> +	} __packed pi;
> +	int rc;
> +
> +	rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
> +				   NULL, 0, &pi, sizeof(pi));
> +	if (rc)
> +		return rc;
> +
> +	*active_volatile_bytes = le64_to_cpu(pi.active_volatile_cap);
> +	*active_persistent_bytes = le64_to_cpu(pi.active_persistent_cap);
> +	*next_volatile_bytes = le64_to_cpu(pi.next_volatile_cap);
> +	*next_persistent_bytes = le64_to_cpu(pi.next_volatile_cap);
> +
> +	*active_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
> +	*active_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
> +	*next_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
> +	*next_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
> +
> +	return 0;
> +}
> +
>  /**
>   * cxl_mem_enumerate_cmds() - Enumerate commands for a device.
>   * @cxlm: The device.
> @@ -1573,18 +1620,53 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
>  			cxlm->persistent_only_bytes,
>  			cxlm->partition_align_bytes);
>  
> +	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
> +	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
> +
> +	return 0;
> +}
> +
> +static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
> +{
> +	int rc;
> +
> +	if (cxlm->partition_align_bytes == 0) {
> +		cxlm->ram_range.start = 0;
> +		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> +		cxlm->pmem_range.start = 0;
> +		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> +		return 0;
> +	}
> +
> +	rc = cxl_mem_get_partition_info(cxlm,
> +					&cxlm->active_volatile_bytes,
> +					&cxlm->active_persistent_bytes,
> +					&cxlm->next_volatile_bytes,
> +					&cxlm->next_persistent_bytes);
> +	if (rc < 0) {
> +		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
> +		return rc;
> +	}

If the command is not supported, you're going to get back -ENXIO here and fail.
I think what you want to do also is use IDENTIFY if PARTITION_INFO doesn't
exist.

> +
> +	dev_dbg(&cxlm->pdev->dev, "Get Partition Info\n"
> +		"     active_volatile_bytes = %#llx\n"
> +		"     active_persistent_bytes = %#llx\n"
> +		"     next_volatile_bytes = %#llx\n"
> +		"     next_persistent_bytes = %#llx\n",
> +			cxlm->active_volatile_bytes,
> +			cxlm->active_persistent_bytes,
> +			cxlm->next_volatile_bytes,
> +			cxlm->next_persistent_bytes);
> +
>  	/*
>  	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
>  	 * For now, only the capacity is exported in sysfs
>  	 */
>  	cxlm->ram_range.start = 0;
> -	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> +	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
>  
>  	cxlm->pmem_range.start = 0;
> -	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> -
> -	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
> -	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
> +	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
>  
>  	return 0;
>  }
> @@ -1618,6 +1700,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> +	rc = cxl_mem_create_range_info(cxlm);
> +	if (rc)
> +		return rc;
> +

Assuming my assessment is correct, I think it's time to make
cxl_mem_mbox_send_cmd grow a little bit better error handling. I'd recommend
something like this and then checking for -ENOTSUPP.

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index b1a5a18dba92..3b7d8a905393 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -92,6 +92,7 @@ struct mbox_cmd {
        size_t size_out;
        u16 return_code;
 #define CXL_MBOX_SUCCESS 0
+#define CXL_MBOX_UNSUPPORTED 0x15
 };

 static int cxl_mem_major;
@@ -876,9 +877,15 @@ static int cxl_mem_mbox_send_cmd(struct cxl_mem *cxlm, u16 opcode,
        if (rc)
                return rc;

-       /* TODO: Map return code to proper kernel style errno */
-       if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
+       switch (mbox_cmd.return_code) {
+       case CXL_MBOX_SUCCESS:
+               break;
+       case CXL_MBOX_UNSUPPORTED:
+               return -ENOTSUPP;
+       default:
+               /* TODO: Map return code to proper kernel style errno */
                return -ENXIO;
+       }

        /*
         * Variable sized commands can't be validated and so it's up to the


>  	return cxl_mem_add_memdev(cxlm);
>  }

>  
> -- 
> 2.28.0.rc0.12.gb6a658bd00c9
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-18 16:58     ` Ben Widawsky
@ 2021-06-18 18:48       ` Ira Weiny
  2021-06-18 19:32         ` Ben Widawsky
  0 siblings, 1 reply; 14+ messages in thread
From: Ira Weiny @ 2021-06-18 18:48 UTC (permalink / raw)
  To: Ben Widawsky
  Cc: Dan Williams, Jonathan Cameron, Alison Schofield, Vishal Verma,
	linux-cxl

On Fri, Jun 18, 2021 at 09:58:33AM -0700, Widawsky, Ben wrote:
> On 21-06-18 09:31:29, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > Memory devices may specify volatile only, persistent only, and
> > partitionable space which when added together result in a total capacity.
> > 
> > If Identify Memory Device.Partition Alignment != 0 the device supports
> > partitionable space.  This partitionable space can be split between
> > volatile and persistent space.  The total volatile and persistent sizes
> > are reported in Get Partition Info.  ie
> > 
> > 	active volatile memory = volatile only + partitionable volatile
> > 	active persistent memory = persistent only + partitionable persistent
> > 
> > Define cxl_mem_get_partition(), check for partitionable support, and use
> > cxl_mem_get_partition() if applicable.
> 
> This doesn't look right to me, but I'll happily stand corrected.
> 

[snip]

> > +
> > +static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
> > +{
> > +	int rc;
> > +
> > +	if (cxlm->partition_align_bytes == 0) {
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This...

> > +		cxlm->ram_range.start = 0;
> > +		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> > +		cxlm->pmem_range.start = 0;
> > +		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> > +		return 0;
> > +	}
> > +
> > +	rc = cxl_mem_get_partition_info(cxlm,
> > +					&cxlm->active_volatile_bytes,
> > +					&cxlm->active_persistent_bytes,
> > +					&cxlm->next_volatile_bytes,
> > +					&cxlm->next_persistent_bytes);
> > +	if (rc < 0) {
> > +		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
> > +		return rc;
> > +	}
> 
> If the command is not supported, you're going to get back -ENXIO here and fail.
> I think what you want to do also is use IDENTIFY if PARTITION_INFO doesn't
> exist.

... should handle the situation where Get Partition Info is not supported.

I did not see it explicitly stated but I believe if Partition Align Bytes is
non-0 then partitioning is supported and Get/Set Partition Info should work.

> 
> Assuming my assessment is correct, I think it's time to make
> cxl_mem_mbox_send_cmd grow a little bit better error handling. I'd recommend
> something like this and then checking for -ENOTSUPP.
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index b1a5a18dba92..3b7d8a905393 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -92,6 +92,7 @@ struct mbox_cmd {
>         size_t size_out;
>         u16 return_code;
>  #define CXL_MBOX_SUCCESS 0
> +#define CXL_MBOX_UNSUPPORTED 0x15
>  };
> 
>  static int cxl_mem_major;
> @@ -876,9 +877,15 @@ static int cxl_mem_mbox_send_cmd(struct cxl_mem *cxlm, u16 opcode,
>         if (rc)
>                 return rc;
> 
> -       /* TODO: Map return code to proper kernel style errno */
> -       if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
> +       switch (mbox_cmd.return_code) {
> +       case CXL_MBOX_SUCCESS:
> +               break;
> +       case CXL_MBOX_UNSUPPORTED:
> +               return -ENOTSUPP;
> +       default:
> +               /* TODO: Map return code to proper kernel style errno */
>                 return -ENXIO;
> +       }


I'm not opposed to adding this though.

Ira

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-18 18:48       ` Ira Weiny
@ 2021-06-18 19:32         ` Ben Widawsky
  0 siblings, 0 replies; 14+ messages in thread
From: Ben Widawsky @ 2021-06-18 19:32 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Dan Williams, Jonathan Cameron, Alison Schofield, Vishal Verma,
	linux-cxl

On 21-06-18 11:48:32, Ira Weiny wrote:
> On Fri, Jun 18, 2021 at 09:58:33AM -0700, Widawsky, Ben wrote:
> > On 21-06-18 09:31:29, ira.weiny@intel.com wrote:
> > > From: Ira Weiny <ira.weiny@intel.com>
> > > 
> > > Memory devices may specify volatile only, persistent only, and
> > > partitionable space which when added together result in a total capacity.
> > > 
> > > If Identify Memory Device.Partition Alignment != 0 the device supports
> > > partitionable space.  This partitionable space can be split between
> > > volatile and persistent space.  The total volatile and persistent sizes
> > > are reported in Get Partition Info.  ie
> > > 
> > > 	active volatile memory = volatile only + partitionable volatile
> > > 	active persistent memory = persistent only + partitionable persistent
> > > 
> > > Define cxl_mem_get_partition(), check for partitionable support, and use
> > > cxl_mem_get_partition() if applicable.
> > 
> > This doesn't look right to me, but I'll happily stand corrected.
> > 
> 
> [snip]
> 
> > > +
> > > +static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
> > > +{
> > > +	int rc;
> > > +
> > > +	if (cxlm->partition_align_bytes == 0) {
>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This...
> 
> > > +		cxlm->ram_range.start = 0;
> > > +		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
> > > +		cxlm->pmem_range.start = 0;
> > > +		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
> > > +		return 0;
> > > +	}
> > > +
> > > +	rc = cxl_mem_get_partition_info(cxlm,
> > > +					&cxlm->active_volatile_bytes,
> > > +					&cxlm->active_persistent_bytes,
> > > +					&cxlm->next_volatile_bytes,
> > > +					&cxlm->next_persistent_bytes);
> > > +	if (rc < 0) {
> > > +		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
> > > +		return rc;
> > > +	}
> > 
> > If the command is not supported, you're going to get back -ENXIO here and fail.
> > I think what you want to do also is use IDENTIFY if PARTITION_INFO doesn't
> > exist.
> 
> ... should handle the situation where Get Partition Info is not supported.
> 
> I did not see it explicitly stated but I believe if Partition Align Bytes is
> non-0 then partitioning is supported and Get/Set Partition Info should work.

Ah. I didn't notice that. Perhaps a comment with a spec reference might help the
next person who comes along to look at that. My reading of it is the same
(though not explicit). I wonder why they chose to put that in IDENTIFY...

> 
> > 
> > Assuming my assessment is correct, I think it's time to make
> > cxl_mem_mbox_send_cmd grow a little bit better error handling. I'd recommend
> > something like this and then checking for -ENOTSUPP.
> > 
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index b1a5a18dba92..3b7d8a905393 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -92,6 +92,7 @@ struct mbox_cmd {
> >         size_t size_out;
> >         u16 return_code;
> >  #define CXL_MBOX_SUCCESS 0
> > +#define CXL_MBOX_UNSUPPORTED 0x15
> >  };
> > 
> >  static int cxl_mem_major;
> > @@ -876,9 +877,15 @@ static int cxl_mem_mbox_send_cmd(struct cxl_mem *cxlm, u16 opcode,
> >         if (rc)
> >                 return rc;
> > 
> > -       /* TODO: Map return code to proper kernel style errno */
> > -       if (mbox_cmd.return_code != CXL_MBOX_SUCCESS)
> > +       switch (mbox_cmd.return_code) {
> > +       case CXL_MBOX_SUCCESS:
> > +               break;
> > +       case CXL_MBOX_UNSUPPORTED:
> > +               return -ENOTSUPP;
> > +       default:
> > +               /* TODO: Map return code to proper kernel style errno */
> >                 return -ENXIO;
> > +       }
> 
> 
> I'm not opposed to adding this though.

You've resolved my concern, so it's up to you. The other option would be to
check the commands if the command got enabled in cxlm->enabled_cmds.

> 
> Ira

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3] cxl/mem: Adjust ram/pmem range to represent DPA ranges
  2021-06-17 22:16 ` [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges ira.weiny
  2021-06-18 14:03   ` Jonathan Cameron
@ 2021-06-21 19:54   ` ira.weiny
  1 sibling, 0 replies; 14+ messages in thread
From: ira.weiny @ 2021-06-21 19:54 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ira Weiny, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ben Widawsky, linux-cxl

From: Ira Weiny <ira.weiny@intel.com>

Table 176 of the CXL spec defines the volatile DPA range to be 0 to
Volatile memory size.  It further defines the persistent DPA range to
follow directly after the end of the Volatile DPA through the persistent
memory size.  Essentially

Volatile DPA range   = [0, Volatile size)
Persistent DPA range = [Volatile size, Volatile size + Persistent size)

Adjust the pmem_range start to reflect this and remove the TODO.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
For some reason I fixed this but failed to send it out...

Changes for V3
	Jonathan -- Clean up commit message

---
 drivers/cxl/pci.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 3c8b7aa8c273..592374baa005 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1633,8 +1633,9 @@ static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
 	if (cxlm->partition_align_bytes == 0) {
 		cxlm->ram_range.start = 0;
 		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
-		cxlm->pmem_range.start = 0;
-		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
+		cxlm->pmem_range.start = cxlm->volatile_only_bytes;
+		cxlm->pmem_range.end = cxlm->volatile_only_bytes +
+					cxlm->persistent_only_bytes - 1;
 		return 0;
 	}
 
@@ -1658,15 +1659,12 @@ static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
 			cxlm->next_volatile_bytes,
 			cxlm->next_persistent_bytes);
 
-	/*
-	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
-	 * For now, only the capacity is exported in sysfs
-	 */
 	cxlm->ram_range.start = 0;
 	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
 
-	cxlm->pmem_range.start = 0;
-	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
+	cxlm->pmem_range.start = cxlm->active_volatile_bytes;
+	cxlm->pmem_range.end = cxlm->active_volatile_bytes +
+				cxlm->active_persistent_bytes - 1;
 
 	return 0;
 }
-- 
2.28.0.rc0.12.gb6a658bd00c9


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges
  2021-06-18 16:31   ` [PATCH V3] " ira.weiny
  2021-06-18 16:58     ` Ben Widawsky
@ 2021-08-11  1:49     ` Dan Williams
  1 sibling, 0 replies; 14+ messages in thread
From: Dan Williams @ 2021-08-11  1:49 UTC (permalink / raw)
  To: linux-cxl; +Cc: Jonathan Cameron, Ira Weiny, kernel test robot

From: Ira Weiny <ira.weiny@intel.com>

Memory devices may specify volatile only, persistent only, and
partitionable space which when added together result in a total capacity.

If Identify Memory Device.Partition Alignment != 0 the device supports
partitionable space.  This partitionable space can be split between
volatile and persistent space.  The total volatile and persistent sizes
are reported in Get Partition Info.  ie

	active volatile memory = volatile only + partitionable volatile
	active persistent memory = persistent only + partitionable persistent

Define cxl_mem_get_partition(), check for partitionable support, and use
cxl_mem_get_partition() if applicable.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Changes since v3:
- Fix "sparse: cast to restricted __le64" warnings, mark the members of
  struct cxl_mbox_get_partition_info as __le64.

 drivers/cxl/cxlmem.h |    5 +++
 drivers/cxl/pci.c    |   96 +++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 22344fda8ca5..6c0b1e2ea97c 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -99,5 +99,10 @@ struct cxl_mem {
 	u64 volatile_only_bytes;
 	u64 persistent_only_bytes;
 	u64 partition_align_bytes;
+
+	u64 active_volatile_bytes;
+	u64 active_persistent_bytes;
+	u64 next_volatile_bytes;
+	u64 next_persistent_bytes;
 };
 #endif /* __CXL_MEM_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index cf4f593f426e..3f5db8960098 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1263,6 +1263,53 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
 	return ret;
 }
 
+/**
+ * cxl_mem_get_partition_info - Get partition info
+ * @cxlm: The device to act on
+ * @active_volatile_bytes: returned active volatile capacity
+ * @active_persistent_bytes: returned active persistent capacity
+ * @next_volatile_bytes: return next volatile capacity
+ * @next_persistent_bytes: return next persistent capacity
+ *
+ * Retrieve the current partition info for the device specified.  If not 0, the
+ * 'next' values are pending and take affect on next cold reset.
+ *
+ * Return: 0 if no error: or the result of the mailbox command.
+ *
+ * See CXL @8.2.9.5.2.1 Get Partition Info
+ */
+static int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
+				      u64 *active_volatile_bytes,
+				      u64 *active_persistent_bytes,
+				      u64 *next_volatile_bytes,
+				      u64 *next_persistent_bytes)
+{
+	struct cxl_mbox_get_partition_info {
+		__le64 active_volatile_cap;
+		__le64 active_persistent_cap;
+		__le64 next_volatile_cap;
+		__le64 next_persistent_cap;
+	} __packed pi;
+	int rc;
+
+	rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
+				   NULL, 0, &pi, sizeof(pi));
+	if (rc)
+		return rc;
+
+	*active_volatile_bytes = le64_to_cpu(pi.active_volatile_cap);
+	*active_persistent_bytes = le64_to_cpu(pi.active_persistent_cap);
+	*next_volatile_bytes = le64_to_cpu(pi.next_volatile_cap);
+	*next_persistent_bytes = le64_to_cpu(pi.next_volatile_cap);
+
+	*active_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*active_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*next_volatile_bytes *= CXL_CAPACITY_MULTIPLIER;
+	*next_persistent_bytes *= CXL_CAPACITY_MULTIPLIER;
+
+	return 0;
+}
+
 /**
  * cxl_mem_enumerate_cmds() - Enumerate commands for a device.
  * @cxlm: The device.
@@ -1381,18 +1428,53 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
 			cxlm->persistent_only_bytes,
 			cxlm->partition_align_bytes);
 
+	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
+	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
+
+	return 0;
+}
+
+static int cxl_mem_create_range_info(struct cxl_mem *cxlm)
+{
+	int rc;
+
+	if (cxlm->partition_align_bytes == 0) {
+		cxlm->ram_range.start = 0;
+		cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
+		cxlm->pmem_range.start = 0;
+		cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
+		return 0;
+	}
+
+	rc = cxl_mem_get_partition_info(cxlm,
+					&cxlm->active_volatile_bytes,
+					&cxlm->active_persistent_bytes,
+					&cxlm->next_volatile_bytes,
+					&cxlm->next_persistent_bytes);
+	if (rc < 0) {
+		dev_err(&cxlm->pdev->dev, "Failed to query partition information\n");
+		return rc;
+	}
+
+	dev_dbg(&cxlm->pdev->dev, "Get Partition Info\n"
+		"     active_volatile_bytes = %#llx\n"
+		"     active_persistent_bytes = %#llx\n"
+		"     next_volatile_bytes = %#llx\n"
+		"     next_persistent_bytes = %#llx\n",
+			cxlm->active_volatile_bytes,
+			cxlm->active_persistent_bytes,
+			cxlm->next_volatile_bytes,
+			cxlm->next_persistent_bytes);
+
 	/*
 	 * TODO: enumerate DPA map, as 'ram' and 'pmem' do not alias.
 	 * For now, only the capacity is exported in sysfs
 	 */
 	cxlm->ram_range.start = 0;
-	cxlm->ram_range.end = cxlm->volatile_only_bytes - 1;
+	cxlm->ram_range.end = cxlm->active_volatile_bytes - 1;
 
 	cxlm->pmem_range.start = 0;
-	cxlm->pmem_range.end = cxlm->persistent_only_bytes - 1;
-
-	cxlm->lsa_size = le32_to_cpu(id.lsa_size);
-	memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
+	cxlm->pmem_range.end = cxlm->active_persistent_bytes - 1;
 
 	return 0;
 }
@@ -1427,6 +1509,10 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
+	rc = cxl_mem_create_range_info(cxlm);
+	if (rc)
+		return rc;
+
 	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlm, &cxl_memdev_fops);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-08-11  1:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 22:16 [PATCH V2 0/3] Query and use Partition Info ira.weiny
2021-06-17 22:16 ` [PATCH V2 1/3] cxl/pci: Store memory capacity values ira.weiny
2021-06-18 13:40   ` Jonathan Cameron
2021-06-17 22:16 ` [PATCH V2 2/3] cxl/mem: Account for partitionable space in ram/pmem ranges ira.weiny
2021-06-18 13:59   ` Jonathan Cameron
2021-06-18 16:30     ` Ira Weiny
2021-06-18 16:31   ` [PATCH V3] " ira.weiny
2021-06-18 16:58     ` Ben Widawsky
2021-06-18 18:48       ` Ira Weiny
2021-06-18 19:32         ` Ben Widawsky
2021-08-11  1:49     ` [PATCH v4 2/3] " Dan Williams
2021-06-17 22:16 ` [PATCH V2 3/3] cxl/mem: Adjust ram/pmem range to represent DPA ranges ira.weiny
2021-06-18 14:03   ` Jonathan Cameron
2021-06-21 19:54   ` [PATCH V3] " ira.weiny

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