| Message ID | 167564536041.847146.11330354943211409793.stgit@dwillia2-xfh.jf.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | CXL RAM and the 'Soft Reserved' => 'System RAM' default | expand |
On Sun, 05 Feb 2023 17:02:40 -0800 Dan Williams <dan.j.williams@intel.com> wrote: > In preparation for a new region type, "ram" regions, add a mode > attribute to clarify the mode of the decoders that can be added to a > region. Share the internals of mode_show() (for decoders) with the > region case. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> I guess the DEAD decoder cases is an odd enough path that it is fine to not worry that it is presented as mixed (none might be more appropriate as you can't put any memory behind it). Other than that this is straight forwards exposure of internal data that already existed... Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 11 +++++++++++ > drivers/cxl/core/port.c | 12 +----------- > drivers/cxl/core/region.c | 10 ++++++++++ > drivers/cxl/cxl.h | 14 ++++++++++++++ > 4 files changed, 36 insertions(+), 11 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 5be032313e29..058b0c45001f 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -358,6 +358,17 @@ Description: > results in the same address being allocated. > > > +What: /sys/bus/cxl/devices/regionZ/mode > +Date: January, 2023 > +KernelVersion: v6.3 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RO) The mode of a region is established at region creation time > + and dictates the mode of the endpoint decoder that comprise the > + region. For more details on the possible modes see > + /sys/bus/cxl/devices/decoderX.Y/mode > + > + > What: /sys/bus/cxl/devices/regionZ/resource > Date: May, 2022 > KernelVersion: v6.0 > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 410c036c09fa..8566451cb22f 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr, > { > struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev); > > - switch (cxled->mode) { > - case CXL_DECODER_RAM: > - return sysfs_emit(buf, "ram\n"); > - case CXL_DECODER_PMEM: > - return sysfs_emit(buf, "pmem\n"); > - case CXL_DECODER_NONE: > - return sysfs_emit(buf, "none\n"); > - case CXL_DECODER_MIXED: > - default: > - return sysfs_emit(buf, "mixed\n"); > - } > + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled->mode)); > } > > static ssize_t mode_store(struct device *dev, struct device_attribute *attr, > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 60828d01972a..17d2d0c12725 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr, > } > static DEVICE_ATTR_RO(resource); > > +static ssize_t mode_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_region *cxlr = to_cxl_region(dev); > + > + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode)); > +} > +static DEVICE_ATTR_RO(mode); > + > static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); > @@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = { > &dev_attr_interleave_granularity.attr, > &dev_attr_resource.attr, > &dev_attr_size.attr, > + &dev_attr_mode.attr, > NULL, > }; > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index aa3af3bb73b2..ca76879af1de 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -320,6 +320,20 @@ enum cxl_decoder_mode { > CXL_DECODER_DEAD, > }; > > +static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) > +{ > + static const char * const names[] = { > + [CXL_DECODER_NONE] = "none", > + [CXL_DECODER_RAM] = "ram", > + [CXL_DECODER_PMEM] = "pmem", > + [CXL_DECODER_MIXED] = "mixed", > + }; > + > + if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED) > + return names[mode]; > + return "mixed"; > +} > + > /** > * struct cxl_endpoint_decoder - Endpoint / SPA to DPA decoder > * @cxld: base cxl_decoder_object >
On Sun, Feb 05, 2023 at 05:02:40PM -0800, Dan Williams wrote: > In preparation for a new region type, "ram" regions, add a mode > attribute to clarify the mode of the decoders that can be added to a > region. Share the internals of mode_show() (for decoders) with the > region case. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- Reviewed-by: Gregory Price <gregory.price@memverge.com>
Jonathan Cameron wrote: > On Sun, 05 Feb 2023 17:02:40 -0800 > Dan Williams <dan.j.williams@intel.com> wrote: > > > In preparation for a new region type, "ram" regions, add a mode > > attribute to clarify the mode of the decoders that can be added to a > > region. Share the internals of mode_show() (for decoders) with the > > region case. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > I guess the DEAD decoder cases is an odd enough path that it is > fine to not worry that it is presented as mixed (none might be > more appropriate as you can't put any memory behind it). Yeah, I went back and forth on what's the best state to expose in the small window between the transition to DEAD and the removal of the attribute from sysfs. Settled on "mixed" as the best indicator of "ambiguous". From an administrator point of the view the decoder is still very much alive in hardware, but the kernel has lost track of its setting. > > Other than that this is straight forwards exposure of internal data > that already existed... > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> >
Dan Williams wrote: > In preparation for a new region type, "ram" regions, add a mode > attribute to clarify the mode of the decoders that can be added to a > region. Share the internals of mode_show() (for decoders) with the > region case. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
On 2/5/23 6:02 PM, Dan Williams wrote: > In preparation for a new region type, "ram" regions, add a mode > attribute to clarify the mode of the decoders that can be added to a > region. Share the internals of mode_show() (for decoders) with the > region case. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 11 +++++++++++ > drivers/cxl/core/port.c | 12 +----------- > drivers/cxl/core/region.c | 10 ++++++++++ > drivers/cxl/cxl.h | 14 ++++++++++++++ > 4 files changed, 36 insertions(+), 11 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 5be032313e29..058b0c45001f 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -358,6 +358,17 @@ Description: > results in the same address being allocated. > > > +What: /sys/bus/cxl/devices/regionZ/mode > +Date: January, 2023 > +KernelVersion: v6.3 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RO) The mode of a region is established at region creation time > + and dictates the mode of the endpoint decoder that comprise the > + region. For more details on the possible modes see > + /sys/bus/cxl/devices/decoderX.Y/mode > + > + > What: /sys/bus/cxl/devices/regionZ/resource > Date: May, 2022 > KernelVersion: v6.0 > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 410c036c09fa..8566451cb22f 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr, > { > struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev); > > - switch (cxled->mode) { > - case CXL_DECODER_RAM: > - return sysfs_emit(buf, "ram\n"); > - case CXL_DECODER_PMEM: > - return sysfs_emit(buf, "pmem\n"); > - case CXL_DECODER_NONE: > - return sysfs_emit(buf, "none\n"); > - case CXL_DECODER_MIXED: > - default: > - return sysfs_emit(buf, "mixed\n"); > - } > + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled->mode)); > } > > static ssize_t mode_store(struct device *dev, struct device_attribute *attr, > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 60828d01972a..17d2d0c12725 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr, > } > static DEVICE_ATTR_RO(resource); > > +static ssize_t mode_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct cxl_region *cxlr = to_cxl_region(dev); > + > + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode)); > +} > +static DEVICE_ATTR_RO(mode); > + > static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); > @@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = { > &dev_attr_interleave_granularity.attr, > &dev_attr_resource.attr, > &dev_attr_size.attr, > + &dev_attr_mode.attr, > NULL, > }; > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index aa3af3bb73b2..ca76879af1de 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -320,6 +320,20 @@ enum cxl_decoder_mode { > CXL_DECODER_DEAD, > }; > > +static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) > +{ > + static const char * const names[] = { > + [CXL_DECODER_NONE] = "none", > + [CXL_DECODER_RAM] = "ram", > + [CXL_DECODER_PMEM] = "pmem", > + [CXL_DECODER_MIXED] = "mixed", > + }; > + > + if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED) > + return names[mode]; > + return "mixed"; > +} > + > /** > * struct cxl_endpoint_decoder - Endpoint / SPA to DPA decoder > * @cxld: base cxl_decoder_object >
On Sun, 2023-02-05 at 17:02 -0800, Dan Williams wrote: > In preparation for a new region type, "ram" regions, add a mode > attribute to clarify the mode of the decoders that can be added to a > region. Share the internals of mode_show() (for decoders) with the > region case. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 11 +++++++++++ > drivers/cxl/core/port.c | 12 +----------- > drivers/cxl/core/region.c | 10 ++++++++++ > drivers/cxl/cxl.h | 14 ++++++++++++++ > 4 files changed, 36 insertions(+), 11 deletions(-) Looks good, thanks for adding this - makes libcxl's job much easier! Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl > b/Documentation/ABI/testing/sysfs-bus-cxl > index 5be032313e29..058b0c45001f 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -358,6 +358,17 @@ Description: > results in the same address being allocated. > > > +What: /sys/bus/cxl/devices/regionZ/mode > +Date: January, 2023 > +KernelVersion: v6.3 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RO) The mode of a region is established at region > creation time > + and dictates the mode of the endpoint decoder that > comprise the > + region. For more details on the possible modes see > + /sys/bus/cxl/devices/decoderX.Y/mode > + > + > What: /sys/bus/cxl/devices/regionZ/resource > Date: May, 2022 > KernelVersion: v6.0 > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 410c036c09fa..8566451cb22f 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, > struct device_attribute *attr, > { > struct cxl_endpoint_decoder *cxled = > to_cxl_endpoint_decoder(dev); > > - switch (cxled->mode) { > - case CXL_DECODER_RAM: > - return sysfs_emit(buf, "ram\n"); > - case CXL_DECODER_PMEM: > - return sysfs_emit(buf, "pmem\n"); > - case CXL_DECODER_NONE: > - return sysfs_emit(buf, "none\n"); > - case CXL_DECODER_MIXED: > - default: > - return sysfs_emit(buf, "mixed\n"); > - } > + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled- > >mode)); > } > > static ssize_t mode_store(struct device *dev, struct > device_attribute *attr, > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 60828d01972a..17d2d0c12725 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, > struct device_attribute *attr, > } > static DEVICE_ATTR_RO(resource); > > +static ssize_t mode_show(struct device *dev, struct device_attribute > *attr, > + char *buf) > +{ > + struct cxl_region *cxlr = to_cxl_region(dev); > + > + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr- > >mode)); > +} > +static DEVICE_ATTR_RO(mode); > + > static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr- > >dev.parent); > @@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = { > &dev_attr_interleave_granularity.attr, > &dev_attr_resource.attr, > &dev_attr_size.attr, > + &dev_attr_mode.attr, > NULL, > }; > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index aa3af3bb73b2..ca76879af1de 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -320,6 +320,20 @@ enum cxl_decoder_mode { > CXL_DECODER_DEAD, > }; > > +static inline const char *cxl_decoder_mode_name(enum > cxl_decoder_mode mode) > +{ > + static const char * const names[] = { > + [CXL_DECODER_NONE] = "none", > + [CXL_DECODER_RAM] = "ram", > + [CXL_DECODER_PMEM] = "pmem", > + [CXL_DECODER_MIXED] = "mixed", > + }; > + > + if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED) > + return names[mode]; > + return "mixed"; > +} > + > /** > * struct cxl_endpoint_decoder - Endpoint / SPA to DPA decoder > * @cxld: base cxl_decoder_object > >
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl index 5be032313e29..058b0c45001f 100644 --- a/Documentation/ABI/testing/sysfs-bus-cxl +++ b/Documentation/ABI/testing/sysfs-bus-cxl @@ -358,6 +358,17 @@ Description: results in the same address being allocated. +What: /sys/bus/cxl/devices/regionZ/mode +Date: January, 2023 +KernelVersion: v6.3 +Contact: linux-cxl@vger.kernel.org +Description: + (RO) The mode of a region is established at region creation time + and dictates the mode of the endpoint decoder that comprise the + region. For more details on the possible modes see + /sys/bus/cxl/devices/decoderX.Y/mode + + What: /sys/bus/cxl/devices/regionZ/resource Date: May, 2022 KernelVersion: v6.0 diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 410c036c09fa..8566451cb22f 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr, { struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev); - switch (cxled->mode) { - case CXL_DECODER_RAM: - return sysfs_emit(buf, "ram\n"); - case CXL_DECODER_PMEM: - return sysfs_emit(buf, "pmem\n"); - case CXL_DECODER_NONE: - return sysfs_emit(buf, "none\n"); - case CXL_DECODER_MIXED: - default: - return sysfs_emit(buf, "mixed\n"); - } + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled->mode)); } static ssize_t mode_store(struct device *dev, struct device_attribute *attr, diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 60828d01972a..17d2d0c12725 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(resource); +static ssize_t mode_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct cxl_region *cxlr = to_cxl_region(dev); + + return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode)); +} +static DEVICE_ATTR_RO(mode); + static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) { struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); @@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = { &dev_attr_interleave_granularity.attr, &dev_attr_resource.attr, &dev_attr_size.attr, + &dev_attr_mode.attr, NULL, }; diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index aa3af3bb73b2..ca76879af1de 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -320,6 +320,20 @@ enum cxl_decoder_mode { CXL_DECODER_DEAD, }; +static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode) +{ + static const char * const names[] = { + [CXL_DECODER_NONE] = "none", + [CXL_DECODER_RAM] = "ram", + [CXL_DECODER_PMEM] = "pmem", + [CXL_DECODER_MIXED] = "mixed", + }; + + if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED) + return names[mode]; + return "mixed"; +} + /** * struct cxl_endpoint_decoder - Endpoint / SPA to DPA decoder * @cxld: base cxl_decoder_object
In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Documentation/ABI/testing/sysfs-bus-cxl | 11 +++++++++++ drivers/cxl/core/port.c | 12 +----------- drivers/cxl/core/region.c | 10 ++++++++++ drivers/cxl/cxl.h | 14 ++++++++++++++ 4 files changed, 36 insertions(+), 11 deletions(-)