Re: [PATCH 2/3] cxl/mem: Report correct ram/pmem size in sysfs

[Ira Weiny <ira.weiny@intel.com>]

On Fri, Jun 11, 2021 at 10:14:31AM -0700, Dan Williams wrote:
> On Thu, Jun 10, 2021 at 5:22 PM <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.
> >
> > The partitionable space is configurable between volatile and persistent
> > space.  To account for the dynamic partitionable space the correct ram
> > and pmem size information is reported in the Get Partition Info device
> > command.
> >
> > Define cxl_mem_get_partition() and call it to retrieve the correct
> > ram and pmem ranges sizes.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> >  drivers/cxl/pci.c | 97 +++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index 9995f97d3b28..bcc2829e4475 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -1455,6 +1455,62 @@ static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_mem *cxlm)
> >         return ret;
> >  }
> >
> > +/**
> > + * cxl_mem_get_partition_info - Set partition info
> > + * @cxlm: The device to act on
> > + * @active_volatile_cap_bytes: returned active volatile capacity; in bytes
> > + * @active_persistent_cap_bytes: returned active persistent capacity; in bytes
> > + * @next_volatile_cap_bytes: return next volatile capacity; in bytes
> > + * @next_persistent_cap_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
> > + */
> > +int cxl_mem_get_partition_info(struct cxl_mem *cxlm,
> > +                              u64 *active_volatile_cap_bytes,
> > +                              u64 *active_persistent_cap_bytes,
> > +                              u64 *next_volatile_cap_bytes,
> > +                              u64 *next_persistent_cap_bytes)
> 
> Similar to how cxl_mem_identify() populates data in cxl_mem, I would
> expect this routine to do the same.
> 
> > +{
> > +       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;
> > +
> > +       /* On error report 0 */
> > +       *active_volatile_cap_bytes = 0;
> > +       *active_persistent_cap_bytes = 0;
> > +       *next_volatile_cap_bytes = 0;
> > +       *next_persistent_cap_bytes = 0;
> > +
> > +       rc = cxl_mem_mbox_send_cmd(cxlm, CXL_MBOX_OP_GET_PARTITION_INFO,
> > +                                  NULL, 0, &pi, sizeof(pi));
> > +
> > +       if (rc)
> > +               return rc;
> > +
> > +       *active_volatile_cap_bytes = le64_to_cpu(pi.active_volatile_cap);
> > +       *active_persistent_cap_bytes = le64_to_cpu(pi.active_persistent_cap);
> > +       *next_volatile_cap_bytes = le64_to_cpu(pi.next_volatile_cap);
> > +       *next_persistent_cap_bytes = le64_to_cpu(pi.next_volatile_cap);
> > +
> > +       *active_volatile_cap_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +       *active_persistent_cap_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +       *next_volatile_cap_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +       *next_persistent_cap_bytes *= CXL_CAPACITY_MULTIPLIER;
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(cxl_mem_get_partition_info);
> 
> Why is this exported? Only the PCI driver cares about this.

Once we get to dynamic partitioning I would expect it to need to be exported...
But I think you are correct...  No need to do that until we need it.

Ira