| Message ID | 20210611002224.1594913-4-ira.weiny@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Query and use Partition Info | expand |
On Thu, 10 Jun 2021 17:22:24 -0700 <ira.weiny@intel.com> wrote: > From: Ira Weiny <ira.weiny@intel.com> > > Partitionable space is added to the ram and pmem size sysfs attributes > but this does not show the entire story. > > Add full partition information about the device under <sysfs>/partition. > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > drivers/cxl/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index bcc2829e4475..7e4e9605e4ed 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -1226,6 +1226,42 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr, > static struct device_attribute dev_attr_pmem_size = > __ATTR(size, 0444, pmem_size_show, NULL); > > +static ssize_t part_vo_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_mem *cxlm = cxlmd->cxlm; > + > + return sysfs_emit(buf, "%#llx\n", cxlm->volatile_cap_bytes); > +} > + > +static struct device_attribute dev_attr_part_vo = > + __ATTR(volatile_only, 0444, part_vo_show, NULL); With a bit of tweaking could we use DEVICE_ATTR_RO for these and have even less boilerplate? > + > +static ssize_t part_po_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_mem *cxlm = cxlmd->cxlm; > + > + return sysfs_emit(buf, "%#llx\n", cxlm->persistent_cap_bytes); > +} > + > +static struct device_attribute dev_attr_part_po = > + __ATTR(persistent_only, 0444, part_po_show, NULL); > + > +static ssize_t part_total_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_mem *cxlm = cxlmd->cxlm; > + > + return sysfs_emit(buf, "%#llx\n", cxlm->total_cap_bytes); > +} > + > +static struct device_attribute dev_attr_part_total = > + __ATTR(total, 0444, part_total_show, NULL); > + > static struct attribute *cxl_memdev_attributes[] = { > &dev_attr_firmware_version.attr, > &dev_attr_payload_max.attr, > @@ -1243,6 +1279,13 @@ static struct attribute *cxl_memdev_ram_attributes[] = { > NULL, > }; > > +static struct attribute *cxl_memdev_part_attributes[] = { > + &dev_attr_part_vo.attr, > + &dev_attr_part_po.attr, > + &dev_attr_part_total.attr, > + NULL, > +}; > + > static struct attribute_group cxl_memdev_attribute_group = { > .attrs = cxl_memdev_attributes, > }; > @@ -1257,10 +1300,16 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = { > .attrs = cxl_memdev_pmem_attributes, > }; > > +static struct attribute_group cxl_memdev_part_attribute_group = { > + .name = "partition", > + .attrs = cxl_memdev_part_attributes, > +}; > + > static const struct attribute_group *cxl_memdev_attribute_groups[] = { > &cxl_memdev_attribute_group, > &cxl_memdev_ram_attribute_group, > &cxl_memdev_pmem_attribute_group, > + &cxl_memdev_part_attribute_group, > NULL, > }; >
On 21-06-10 17:22:24, ira.weiny@intel.com wrote: > From: Ira Weiny <ira.weiny@intel.com> > > Partitionable space is added to the ram and pmem size sysfs attributes > but this does not show the entire story. > > Add full partition information about the device under <sysfs>/partition. Please add these to: Documentation/ABI/testing/sysfs-bus-cxl It might be time to break that file up... maybe a syfs-bus-cxl-memdev? I'll also pose the original question here, what's the justification for exposing this via sysfs versus having userspace executing the mailbox command itself? Are you planning to have these attributes be writable at some point to do the partition programming? > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > drivers/cxl/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index bcc2829e4475..7e4e9605e4ed 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -1226,6 +1226,42 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr, > static struct device_attribute dev_attr_pmem_size = > __ATTR(size, 0444, pmem_size_show, NULL); > > +static ssize_t part_vo_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_mem *cxlm = cxlmd->cxlm; > + > + return sysfs_emit(buf, "%#llx\n", cxlm->volatile_cap_bytes); > +} > + > +static struct device_attribute dev_attr_part_vo = > + __ATTR(volatile_only, 0444, part_vo_show, NULL); > + > +static ssize_t part_po_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_mem *cxlm = cxlmd->cxlm; > + > + return sysfs_emit(buf, "%#llx\n", cxlm->persistent_cap_bytes); > +} > + > +static struct device_attribute dev_attr_part_po = > + __ATTR(persistent_only, 0444, part_po_show, NULL); > + > +static ssize_t part_total_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_mem *cxlm = cxlmd->cxlm; > + > + return sysfs_emit(buf, "%#llx\n", cxlm->total_cap_bytes); > +} > + > +static struct device_attribute dev_attr_part_total = > + __ATTR(total, 0444, part_total_show, NULL); > + > static struct attribute *cxl_memdev_attributes[] = { > &dev_attr_firmware_version.attr, > &dev_attr_payload_max.attr, > @@ -1243,6 +1279,13 @@ static struct attribute *cxl_memdev_ram_attributes[] = { > NULL, > }; > > +static struct attribute *cxl_memdev_part_attributes[] = { > + &dev_attr_part_vo.attr, > + &dev_attr_part_po.attr, > + &dev_attr_part_total.attr, > + NULL, > +}; > + > static struct attribute_group cxl_memdev_attribute_group = { > .attrs = cxl_memdev_attributes, > }; > @@ -1257,10 +1300,16 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = { > .attrs = cxl_memdev_pmem_attributes, > }; > > +static struct attribute_group cxl_memdev_part_attribute_group = { > + .name = "partition", > + .attrs = cxl_memdev_part_attributes, > +}; > + > static const struct attribute_group *cxl_memdev_attribute_groups[] = { > &cxl_memdev_attribute_group, > &cxl_memdev_ram_attribute_group, > &cxl_memdev_pmem_attribute_group, > + &cxl_memdev_part_attribute_group, > NULL, > }; > > -- > 2.28.0.rc0.12.gb6a658bd00c9 >
On Fri, Jun 11, 2021 at 10:26:15AM -0700, Widawsky, Ben wrote: > On 21-06-10 17:22:24, ira.weiny@intel.com wrote: > > From: Ira Weiny <ira.weiny@intel.com> > > > > Partitionable space is added to the ram and pmem size sysfs attributes > > but this does not show the entire story. > > > > Add full partition information about the device under <sysfs>/partition. > > Please add these to: > Documentation/ABI/testing/sysfs-bus-cxl Good point... Thanks! > > It might be time to break that file up... maybe a syfs-bus-cxl-memdev? > > I'll also pose the original question here, what's the justification for exposing > this via sysfs versus having userspace executing the mailbox command itself? Are > you planning to have these attributes be writable at some point to do the > partition programming? Well... if we have ram/size and pmem/size this is nice to have to know how it is configured... For me I like not having to have to execute mailbox commands for everything but I understand having sysfs 'explosion'... not really sure were to draw the line. For now this helps me to see what the emulators are returning. Ira > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > > drivers/cxl/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > > index bcc2829e4475..7e4e9605e4ed 100644 > > --- a/drivers/cxl/pci.c > > +++ b/drivers/cxl/pci.c > > @@ -1226,6 +1226,42 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr, > > static struct device_attribute dev_attr_pmem_size = > > __ATTR(size, 0444, pmem_size_show, NULL); > > > > +static ssize_t part_vo_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > > + struct cxl_mem *cxlm = cxlmd->cxlm; > > + > > + return sysfs_emit(buf, "%#llx\n", cxlm->volatile_cap_bytes); > > +} > > + > > +static struct device_attribute dev_attr_part_vo = > > + __ATTR(volatile_only, 0444, part_vo_show, NULL); > > + > > +static ssize_t part_po_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > > + struct cxl_mem *cxlm = cxlmd->cxlm; > > + > > + return sysfs_emit(buf, "%#llx\n", cxlm->persistent_cap_bytes); > > +} > > + > > +static struct device_attribute dev_attr_part_po = > > + __ATTR(persistent_only, 0444, part_po_show, NULL); > > + > > +static ssize_t part_total_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > > + struct cxl_mem *cxlm = cxlmd->cxlm; > > + > > + return sysfs_emit(buf, "%#llx\n", cxlm->total_cap_bytes); > > +} > > + > > +static struct device_attribute dev_attr_part_total = > > + __ATTR(total, 0444, part_total_show, NULL); > > + > > static struct attribute *cxl_memdev_attributes[] = { > > &dev_attr_firmware_version.attr, > > &dev_attr_payload_max.attr, > > @@ -1243,6 +1279,13 @@ static struct attribute *cxl_memdev_ram_attributes[] = { > > NULL, > > }; > > > > +static struct attribute *cxl_memdev_part_attributes[] = { > > + &dev_attr_part_vo.attr, > > + &dev_attr_part_po.attr, > > + &dev_attr_part_total.attr, > > + NULL, > > +}; > > + > > static struct attribute_group cxl_memdev_attribute_group = { > > .attrs = cxl_memdev_attributes, > > }; > > @@ -1257,10 +1300,16 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = { > > .attrs = cxl_memdev_pmem_attributes, > > }; > > > > +static struct attribute_group cxl_memdev_part_attribute_group = { > > + .name = "partition", > > + .attrs = cxl_memdev_part_attributes, > > +}; > > + > > static const struct attribute_group *cxl_memdev_attribute_groups[] = { > > &cxl_memdev_attribute_group, > > &cxl_memdev_ram_attribute_group, > > &cxl_memdev_pmem_attribute_group, > > + &cxl_memdev_part_attribute_group, > > NULL, > > }; > > > > -- > > 2.28.0.rc0.12.gb6a658bd00c9 > >
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index bcc2829e4475..7e4e9605e4ed 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -1226,6 +1226,42 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr, static struct device_attribute dev_attr_pmem_size = __ATTR(size, 0444, pmem_size_show, NULL); +static ssize_t part_vo_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); + struct cxl_mem *cxlm = cxlmd->cxlm; + + return sysfs_emit(buf, "%#llx\n", cxlm->volatile_cap_bytes); +} + +static struct device_attribute dev_attr_part_vo = + __ATTR(volatile_only, 0444, part_vo_show, NULL); + +static ssize_t part_po_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); + struct cxl_mem *cxlm = cxlmd->cxlm; + + return sysfs_emit(buf, "%#llx\n", cxlm->persistent_cap_bytes); +} + +static struct device_attribute dev_attr_part_po = + __ATTR(persistent_only, 0444, part_po_show, NULL); + +static ssize_t part_total_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); + struct cxl_mem *cxlm = cxlmd->cxlm; + + return sysfs_emit(buf, "%#llx\n", cxlm->total_cap_bytes); +} + +static struct device_attribute dev_attr_part_total = + __ATTR(total, 0444, part_total_show, NULL); + static struct attribute *cxl_memdev_attributes[] = { &dev_attr_firmware_version.attr, &dev_attr_payload_max.attr, @@ -1243,6 +1279,13 @@ static struct attribute *cxl_memdev_ram_attributes[] = { NULL, }; +static struct attribute *cxl_memdev_part_attributes[] = { + &dev_attr_part_vo.attr, + &dev_attr_part_po.attr, + &dev_attr_part_total.attr, + NULL, +}; + static struct attribute_group cxl_memdev_attribute_group = { .attrs = cxl_memdev_attributes, }; @@ -1257,10 +1300,16 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = { .attrs = cxl_memdev_pmem_attributes, }; +static struct attribute_group cxl_memdev_part_attribute_group = { + .name = "partition", + .attrs = cxl_memdev_part_attributes, +}; + static const struct attribute_group *cxl_memdev_attribute_groups[] = { &cxl_memdev_attribute_group, &cxl_memdev_ram_attribute_group, &cxl_memdev_pmem_attribute_group, + &cxl_memdev_part_attribute_group, NULL, };