Re: [PATCH 03/26] cxl/mem: Read dynamic capacity configuration from the device

Ira Weiny
2024-04-05T18:09:34Z

Jørgen Hansen wrote:
> On 3/25/24 00:18, ira.weiny@intel.com wrote:
>
> > From: Navneet Singh <navneet.singh@intel.com>
> >

[snip]

> > /**
> > * struct cxl_memdev_state - Generic Type-3 Memory Device Class driver data
> > *
> > @@ -467,6 +482,8 @@ struct cxl_dev_state {
> > * @enabled_cmds: Hardware commands found enabled in CEL.
> > * @exclusive_cmds: Commands that are kernel-internal only
> > * @total_bytes: sum of all possible capacities
> > + * @static_cap: Sum of static RAM and PMEM capacities
> > + * @dynamic_cap: Complete DPA range occupied by DC regions
>
> How about naming these total_range, static_cap and dynamic_range to make
> it clear that the DPA range occupied by DC regions isn't necessarily
> usable capacity (as opposed to the static_cap where the spec defines it
> as usable capacity).

I thought this was a good idea but on second thought these are not range
variables at all. They really represent the various lengths of the
resources.

For total_bytes the documentation already says 'sum of all __possible__
capacities'.

I think you have a point for the new fields though. They should all be
named in some consistent manner and documented as such.

So I propose:

diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 94531af018f8..9c18b229f69a 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -481,9 +481,9 @@ struct cxl_dc_region_info {
* @dcd_cmds: List of DCD commands implemented by memory device
* @enabled_cmds: Hardware commands found enabled in CEL.
* @exclusive_cmds: Commands that are kernel-internal only
- * @total_bytes: sum of all possible capacities
- * @static_cap: Sum of static RAM and PMEM capacities
- * @dynamic_cap: Complete DPA range occupied by DC regions
+ * @total_bytes: length of all possible capacities
+ * @static_bytes: length of possible static RAM and PMEM partitions
+ * @dynamic_bytes: length of possible DC partitions (DC Regions)
* @volatile_only_bytes: hard volatile capacity
* @persistent_only_bytes: hard persistent capacity
* @partition_align_bytes: alignment size for partition-able capacity
@@ -515,8 +515,8 @@ struct cxl_memdev_state {
DECLARE_BITMAP(exclusive_cmds, CXL_MEM_COMMAND_ID_MAX);

u64 total_bytes;
- u64 static_cap;
- u64 dynamic_cap;
+ u64 static_bytes;
+ u64 dynamic_bytes;
u64 volatile_only_bytes;
u64 persistent_only_bytes;
u64 partition_align_bytes;