>-----Original Message-----
>From: fan <nifan.cxl@xxxxxxxxx>
>Sent: 24 April 2024 21:26
>To: Shiju Jose <shiju.jose@xxxxxxxxxx>
>Cc: linux-cxl@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; linux-
>mm@xxxxxxxxx; dan.j.williams@xxxxxxxxx; dave@xxxxxxxxxxxx; Jonathan
>Cameron <jonathan.cameron@xxxxxxxxxx>; dave.jiang@xxxxxxxxx;
>alison.schofield@xxxxxxxxx; vishal.l.verma@xxxxxxxxx; ira.weiny@xxxxxxxxx;
>linux-edac@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; david@xxxxxxxxxx;
>Vilas.Sridharan@xxxxxxx; leo.duran@xxxxxxx; Yazen.Ghannam@xxxxxxx;
>rientjes@xxxxxxxxxx; jiaqiyan@xxxxxxxxxx; tony.luck@xxxxxxxxx;
>Jon.Grimm@xxxxxxx; dave.hansen@xxxxxxxxxxxxxxx; rafael@xxxxxxxxxx;
>lenb@xxxxxxxxxx; naoya.horiguchi@xxxxxxx; james.morse@xxxxxxx;
>jthoughton@xxxxxxxxxx; somasundaram.a@xxxxxxx;
>erdemaktas@xxxxxxxxxx; pgonda@xxxxxxxxxx; duenwen@xxxxxxxxxx;
>mike.malvestuto@xxxxxxxxx; gthelen@xxxxxxxxxx;
>wschwartz@xxxxxxxxxxxxxxxxxxx; dferguson@xxxxxxxxxxxxxxxxxxx;
>wbs@xxxxxxxxxxxxxxxxxxxxxx; nifan.cxl@xxxxxxxxx; tanxiaofei
><tanxiaofei@xxxxxxxxxx>; Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>;
>kangkang.shen@xxxxxxxxxxxxx; wanghuiqiang <wanghuiqiang@xxxxxxxxxx>;
>Linuxarm <linuxarm@xxxxxxxxxx>
>Subject: Re: [RFC PATCH v8 01/10] ras: scrub: Add scrub subsystem
>
>On Sat, Apr 20, 2024 at 12:47:10AM +0800, shiju.jose@xxxxxxxxxx wrote:
>> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>>
>> Add scrub subsystem supports configuring the memory scrubbers in the
>> system. The scrub subsystem provides the interface for registering the
>> scrub devices. The scrub control attributes are provided to the user
>> in /sys/class/ras/rasX/scrub
>>
>> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>> Signed-off-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
>> ---
>> .../ABI/testing/sysfs-class-scrub-configure | 47 +++
>> drivers/ras/Kconfig | 7 +
>> drivers/ras/Makefile | 1 +
>> drivers/ras/memory_scrub.c | 271 ++++++++++++++++++
>> include/linux/memory_scrub.h | 37 +++
>> 5 files changed, 363 insertions(+)
>> create mode 100644
>> Documentation/ABI/testing/sysfs-class-scrub-configure
>> create mode 100755 drivers/ras/memory_scrub.c create mode 100755
>> include/linux/memory_scrub.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure
>> b/Documentation/ABI/testing/sysfs-class-scrub-configure
>> new file mode 100644
>> index 000000000000..3ed77dbb00ad
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
>> @@ -0,0 +1,47 @@
>> +What: /sys/class/ras/
>> +Date: March 2024
>> +KernelVersion: 6.9
>> +Contact: linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> + The ras/ class subdirectory belongs to the
>> + common ras features such as scrub subsystem.
>> +
>> +What: /sys/class/ras/rasX/scrub/
>> +Date: March 2024
>> +KernelVersion: 6.9
>> +Contact: linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> + The /sys/class/ras/ras{0,1,2,3,...}/scrub directories
>> + correspond to each scrub device registered with the
>> + scrub subsystem.
>> +
>> +What: /sys/class/ras/rasX/scrub/name
>> +Date: March 2024
>> +KernelVersion: 6.9
>> +Contact: linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> + (RO) name of the memory scrubber
>> +
>> +What: /sys/class/ras/rasX/scrub/enable_background
>> +Date: March 2024
>> +KernelVersion: 6.9
>> +Contact: linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> + (RW) Enable/Disable background(patrol) scrubbing if supported.
>> +
>> +What: /sys/class/ras/rasX/scrub/rate_available
>> +Date: March 2024
>> +KernelVersion: 6.9
>> +Contact: linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> + (RO) Supported range for the scrub rate by the scrubber.
>> + The scrub rate represents in hours.
>> +
>> +What: /sys/class/ras/rasX/scrub/rate
>> +Date: March 2024
>> +KernelVersion: 6.9
>> +Contact: linux-kernel@xxxxxxxxxxxxxxx
>> +Description:
>> + (RW) The scrub rate specified and it must be with in the
>> + supported range by the scrubber.
>> + The scrub rate represents in hours.
>> diff --git a/drivers/ras/Kconfig b/drivers/ras/Kconfig index
>> fc4f4bb94a4c..181701479564 100644
>> --- a/drivers/ras/Kconfig
>> +++ b/drivers/ras/Kconfig
>> @@ -46,4 +46,11 @@ config RAS_FMPM
>> Memory will be retired during boot time and run time depending on
>> platform-specific policies.
>>
>> +config SCRUB
>> + tristate "Memory scrub driver"
>> + help
>> + This option selects the memory scrub subsystem, supports
>> + configuring the parameters of underlying scrubbers in the
>> + system for the DRAM memories.
>> +
>> endif
>> diff --git a/drivers/ras/Makefile b/drivers/ras/Makefile index
>> 11f95d59d397..89bcf0d84355 100644
>> --- a/drivers/ras/Makefile
>> +++ b/drivers/ras/Makefile
>> @@ -2,6 +2,7 @@
>> obj-$(CONFIG_RAS) += ras.o
>> obj-$(CONFIG_DEBUG_FS) += debugfs.o
>> obj-$(CONFIG_RAS_CEC) += cec.o
>> +obj-$(CONFIG_SCRUB) += memory_scrub.o
>>
>> obj-$(CONFIG_RAS_FMPM) += amd/fmpm.o
>> obj-y += amd/atl/
>> diff --git a/drivers/ras/memory_scrub.c b/drivers/ras/memory_scrub.c
>> new file mode 100755 index 000000000000..7e995380ec3a
>> --- /dev/null
>> +++ b/drivers/ras/memory_scrub.c
>> @@ -0,0 +1,271 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Memory scrub subsystem supports configuring the registered
>> + * memory scrubbers.
>> + *
>> + * Copyright (c) 2024 HiSilicon Limited.
>> + */
>> +
>> +#define pr_fmt(fmt) "MEM SCRUB: " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/kfifo.h>
>> +#include <linux/memory_scrub.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>> +
>> +/* memory scrubber config definitions */ #define SCRUB_ID_PREFIX
>> +"ras"
>> +#define SCRUB_ID_FORMAT SCRUB_ID_PREFIX "%d"
>> +
>> +static DEFINE_IDA(scrub_ida);
>> +
>> +struct scrub_device {
>> + int id;
>> + struct device dev;
>> + const struct scrub_ops *ops;
>> +};
>> +
>> +#define to_scrub_device(d) container_of(d, struct scrub_device, dev)
>> +static ssize_t enable_background_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t len) {
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + bool enable;
>> + int ret;
>> +
>> + ret = kstrtobool(buf, &enable);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = scrub_dev->ops->set_enabled_bg(dev, enable);
>> + if (ret)
>> + return ret;
>> +
>> + return len;
>> +}
>> +
>> +static ssize_t enable_background_show(struct device *dev,
>> + struct device_attribute *attr, char *buf) {
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + bool enable;
>> + int ret;
>> +
>> + ret = scrub_dev->ops->get_enabled_bg(dev, &enable);
>> + if (ret)
>> + return ret;
>> +
>> + return sysfs_emit(buf, "%d\n", enable); }
>> +
>> +static ssize_t name_show(struct device *dev,
>> + struct device_attribute *attr, char *buf) {
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + int ret;
>> +
>> + ret = scrub_dev->ops->get_name(dev, buf);
>> + if (ret)
>> + return ret;
>> +
>> + return strlen(buf);
>> +}
>> +
>> +static ssize_t rate_show(struct device *dev, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + u64 val;
>> + int ret;
>> +
>> + ret = scrub_dev->ops->rate_read(dev, &val);
>> + if (ret)
>> + return ret;
>> +
>> + return sysfs_emit(buf, "0x%llx\n", val); }
>> +
>> +static ssize_t rate_store(struct device *dev, struct device_attribute *attr,
>> + const char *buf, size_t len)
>> +{
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + long val;
>> + int ret;
>> +
>> + ret = kstrtol(buf, 10, &val);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = scrub_dev->ops->rate_write(dev, val);
>> + if (ret)
>> + return ret;
>> +
>> + return len;
>> +}
>> +
>> +static ssize_t rate_available_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + u64 min_sr, max_sr;
>> + int ret;
>> +
>> + ret = scrub_dev->ops->rate_avail_range(dev, &min_sr, &max_sr);
>> + if (ret)
>> + return ret;
>> +
>> + return sysfs_emit(buf, "0x%llx-0x%llx\n", min_sr, max_sr); }
>> +
>> +DEVICE_ATTR_RW(enable_background);
>> +DEVICE_ATTR_RO(name);
>> +DEVICE_ATTR_RW(rate);
>> +DEVICE_ATTR_RO(rate_available);
>> +
>> +static struct attribute *scrub_attrs[] = {
>> + &dev_attr_enable_background.attr,
>> + &dev_attr_name.attr,
>> + &dev_attr_rate.attr,
>> + &dev_attr_rate_available.attr,
>> + NULL
>> +};
>> +
>> +static umode_t scrub_attr_visible(struct kobject *kobj,
>> + struct attribute *a, int attr_id) {
>> + struct device *dev = kobj_to_dev(kobj);
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> + const struct scrub_ops *ops = scrub_dev->ops;
>> +
>> + if (a == &dev_attr_enable_background.attr) {
>> + if (ops->set_enabled_bg && ops->get_enabled_bg)
>> + return a->mode;
>> + if (ops->get_enabled_bg)
>> + return 0444;
>> + return 0;
>> + }
>> + if (a == &dev_attr_name.attr)
>> + return ops->get_name ? a->mode : 0;
>> + if (a == &dev_attr_rate_available.attr)
>> + return ops->rate_avail_range ? a->mode : 0;
>> + if (a == &dev_attr_rate.attr) { /* Write only makes little sense */
>> + if (ops->rate_read && ops->rate_write)
>> + return a->mode;
>> + if (ops->rate_read)
>> + return 0444;
>> + return 0;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct attribute_group scrub_attr_group = {
>> + .name = "scrub",
>> + .attrs = scrub_attrs,
>> + .is_visible = scrub_attr_visible,
>> +};
>> +
>> +static const struct attribute_group *scrub_attr_groups[] = {
>> + &scrub_attr_group,
>> + NULL
>> +};
>> +
>> +static void scrub_dev_release(struct device *dev) {
>> + struct scrub_device *scrub_dev = to_scrub_device(dev);
>> +
>> + ida_free(&scrub_ida, scrub_dev->id);
>> + kfree(scrub_dev);
>> +}
>> +
>> +static struct class scrub_class = {
>> + .name = "ras",
>> + .dev_groups = scrub_attr_groups,
>> + .dev_release = scrub_dev_release,
>> +};
>> +
>> +static struct device *
>> +scrub_device_register(struct device *parent, void *drvdata,
>> + const struct scrub_ops *ops)
>> +{
>> + struct scrub_device *scrub_dev;
>> + struct device *hdev;
>> + int err;
>> +
>> + scrub_dev = kzalloc(sizeof(*scrub_dev), GFP_KERNEL);
>> + if (!scrub_dev)
>> + return ERR_PTR(-ENOMEM);
>> + hdev = &scrub_dev->dev;
>> +
>> + scrub_dev->id = ida_alloc(&scrub_ida, GFP_KERNEL);
>> + if (scrub_dev->id < 0) {
>> + kfree(scrub_dev);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> + scrub_dev->ops = ops;
>> + hdev->class = &scrub_class;
>> + hdev->parent = parent;
>> + dev_set_drvdata(hdev, drvdata);
>> + dev_set_name(hdev, SCRUB_ID_FORMAT, scrub_dev->id);
>
>Need to check the return value of dev_set_name?
Will do, though checking return value of dev_set_name() is not common in the kernel.
>
>fan
>
>> + err = device_register(hdev);
>> + if (err) {
Thanks,
Shiju
