linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers
@ 2023-09-15 17:28 shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 1/9] memory: scrub: Add scrub driver supports configuring memory scrubbers in the system shiju.jose
                   ` (9 more replies)
  0 siblings, 10 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

This series add,
1. support for ACPI RASF(RAS feature table) PCC interfaces
to communicate with the HW patrol scrubber in the platform,
as per ACPI 5.1 & upwards revision. Section 5.2.20.

2. support for ACPI RAS2(RAS2 feature table), as per
ACPI 6.5 & upwards revision. Section 5.2.21.

3. scrub driver supports configuring parameters of the memory
scrubbers in the system. This driver has been implemented
based on the hwmon subsystem.

The features have tested with RASF and RAS2 emulation in the QEMU.

Previous references to the memory scub and RASF topics.
https://lore.kernel.org/all/20221103155029.2451105-1-jiaqiyan@google.com/
https://patchwork.kernel.org/project/linux-arm-kernel/patch/CS1PR84MB0038718F49DBC0FF03919E1184390@CS1PR84MB0038.NAMPRD84.PROD.OUTLOOK.COM/

A Somasundaram (2):
  ACPI:RASF: Add extract RASF table to register RASF platform devices
  ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces

Shiju Jose (7):
  memory: scrub: Add scrub driver supports configuring memory scrubbers
    in the system
  memory: scrub: sysfs: Add Documentation entries for set of scrub
    attributes
  Documentation/scrub-configure.rst: Add documentation for scrub driver
  memory: RASF: Add memory RASF driver
  ACPICA: ACPI 6.5: Add support for RAS2 table
  ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2)
  memory: RAS2: Add memory RAS2 driver

 .../ABI/testing/sysfs-class-scrub-configure   |  82 ++++
 Documentation/scrub-configure.rst             |  55 +++
 drivers/acpi/Kconfig                          |  15 +
 drivers/acpi/Makefile                         |   1 +
 drivers/acpi/ras2_acpi.c                      |  97 ++++
 drivers/acpi/rasf_acpi.c                      |  71 +++
 drivers/acpi/rasf_acpi_common.c               | 272 +++++++++++
 drivers/memory/Kconfig                        |  15 +
 drivers/memory/Makefile                       |   3 +
 drivers/memory/ras2.c                         | 334 +++++++++++++
 drivers/memory/rasf.c                         | 335 +++++++++++++
 drivers/memory/rasf_common.c                  | 251 ++++++++++
 drivers/memory/scrub/Kconfig                  |  11 +
 drivers/memory/scrub/Makefile                 |   6 +
 drivers/memory/scrub/memory-scrub.c           | 452 ++++++++++++++++++
 include/acpi/actbl2.h                         |  55 +++
 include/acpi/rasf_acpi.h                      |  59 +++
 include/memory/memory-scrub.h                 |  85 ++++
 include/memory/rasf.h                         |  82 ++++
 19 files changed, 2281 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure
 create mode 100644 Documentation/scrub-configure.rst
 create mode 100755 drivers/acpi/ras2_acpi.c
 create mode 100755 drivers/acpi/rasf_acpi.c
 create mode 100755 drivers/acpi/rasf_acpi_common.c
 create mode 100644 drivers/memory/ras2.c
 create mode 100644 drivers/memory/rasf.c
 create mode 100644 drivers/memory/rasf_common.c
 create mode 100644 drivers/memory/scrub/Kconfig
 create mode 100644 drivers/memory/scrub/Makefile
 create mode 100755 drivers/memory/scrub/memory-scrub.c
 create mode 100755 include/acpi/rasf_acpi.h
 create mode 100755 include/memory/memory-scrub.h
 create mode 100755 include/memory/rasf.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [RFC PATCH 1/9] memory: scrub: Add scrub driver supports configuring memory scrubbers in the system
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes shiju.jose
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Add scrub driver supports configuring the memory scrubbers in the
system. The scrub driver provides the interface for registering the
scrub devices and to configure the parameters of memory scrubbers in the
system.
Driver exposes the scrub parameter attributes to the user via sysfs in
/sys/class/scrub/scrubX/regionY/

This driver has been implemented based on the hwmon subsystem.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/memory/Kconfig              |   1 +
 drivers/memory/Makefile             |   1 +
 drivers/memory/scrub/Kconfig        |  11 +
 drivers/memory/scrub/Makefile       |   6 +
 drivers/memory/scrub/memory-scrub.c | 452 ++++++++++++++++++++++++++++
 include/memory/memory-scrub.h       |  85 ++++++
 6 files changed, 556 insertions(+)
 create mode 100644 drivers/memory/scrub/Kconfig
 create mode 100644 drivers/memory/scrub/Makefile
 create mode 100755 drivers/memory/scrub/memory-scrub.c
 create mode 100755 include/memory/memory-scrub.h

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 8efdd1f97139..d2e015c09d83 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -227,5 +227,6 @@ config STM32_FMC2_EBI
 
 source "drivers/memory/samsung/Kconfig"
 source "drivers/memory/tegra/Kconfig"
+source "drivers/memory/scrub/Kconfig"
 
 endif
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index d2e6ca9abbe0..4b37312cb342 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_STM32_FMC2_EBI)	+= stm32-fmc2-ebi.o
 
 obj-$(CONFIG_SAMSUNG_MC)	+= samsung/
 obj-$(CONFIG_TEGRA_MC)		+= tegra/
+obj-$(CONFIG_SCRUB)		+= scrub/
 obj-$(CONFIG_TI_EMIF_SRAM)	+= ti-emif-sram.o
 obj-$(CONFIG_FPGA_DFL_EMIF)	+= dfl-emif.o
 
diff --git a/drivers/memory/scrub/Kconfig b/drivers/memory/scrub/Kconfig
new file mode 100644
index 000000000000..fa7d68f53a69
--- /dev/null
+++ b/drivers/memory/scrub/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Memory scrub driver configurations
+#
+
+config SCRUB
+	bool "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.
diff --git a/drivers/memory/scrub/Makefile b/drivers/memory/scrub/Makefile
new file mode 100644
index 000000000000..1b677132ca13
--- /dev/null
+++ b/drivers/memory/scrub/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for memory scrub drivers
+#
+
+obj-$(CONFIG_SCRUB)		+= memory-scrub.o
diff --git a/drivers/memory/scrub/memory-scrub.c b/drivers/memory/scrub/memory-scrub.c
new file mode 100755
index 000000000000..b3011e7cd062
--- /dev/null
+++ b/drivers/memory/scrub/memory-scrub.c
@@ -0,0 +1,452 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Memory scrub controller driver support to configure
+ * the parameters of the memory scrubbers and enable.
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ */
+
+#define pr_fmt(fmt)     "MEM SCRUB: " fmt
+
+#include <linux/acpi.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/kfifo.h>
+#include <linux/spinlock.h>
+#include <memory/memory-scrub.h>
+
+/* memory scrubber config definitions */
+#define SCRUB_ID_PREFIX "scrub"
+#define SCRUB_ID_FORMAT SCRUB_ID_PREFIX "%d"
+#define SCRUB_DEV_MAX_NAME_LENGTH	128
+
+static DEFINE_IDA(scrub_ida);
+
+struct scrub_device {
+	char name[SCRUB_DEV_MAX_NAME_LENGTH];
+	int id;
+	struct device dev;
+	const struct scrub_source_info *source_info;
+	struct list_head tzdata;
+	char (*region_name)[];
+	struct attribute_group group;
+	int ngroups;
+	struct attribute_group *region_groups;
+	const struct attribute_group **groups;
+};
+
+#define to_scrub_device(d) container_of(d, struct scrub_device, dev)
+#define SCRUB_MAX_SYSFS_ATTR_NAME_LENGTH	64
+
+struct scrub_device_attribute {
+	struct device_attribute dev_attr;
+	const struct scrub_ops *ops;
+	u32 attr;
+	int region_id;
+	char name[SCRUB_MAX_SYSFS_ATTR_NAME_LENGTH];
+};
+
+#define to_scrub_attr(d) \
+	container_of(d, struct scrub_device_attribute, dev_attr)
+#define to_dev_attr(a) container_of(a, struct device_attribute, attr)
+
+static ssize_t name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%s\n", to_scrub_device(dev)->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static struct attribute *scrub_dev_attrs[] = {
+	&dev_attr_name.attr,
+	NULL
+};
+
+static umode_t scrub_dev_attr_is_visible(struct kobject *kobj,
+					    struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct scrub_device *scrub_dev = to_scrub_device(dev);
+
+	if (attr == &dev_attr_name.attr && scrub_dev->name == NULL)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group scrub_dev_attr_group = {
+	.attrs		= scrub_dev_attrs,
+	.is_visible	= scrub_dev_attr_is_visible,
+};
+
+static const struct attribute_group *scrub_dev_attr_groups[] = {
+	&scrub_dev_attr_group,
+	NULL
+};
+
+static void scrub_free_attrs(struct attribute **attrs)
+{
+	int i;
+
+	for (i = 0; attrs[i]; i++) {
+		struct device_attribute *dattr = to_dev_attr(attrs[i]);
+		struct scrub_device_attribute *hattr = to_scrub_attr(dattr);
+
+		kfree(hattr);
+	}
+	kfree(attrs);
+}
+
+static void scrub_dev_release(struct device *dev)
+{
+	int count;
+	struct attribute_group *group;
+	struct scrub_device *scrub_dev = to_scrub_device(dev);
+
+	for (count = 0; count < scrub_dev->ngroups; count++) {
+		group = (struct attribute_group *)scrub_dev->groups[count];
+		if (group)
+			scrub_free_attrs(group->attrs);
+	}
+	kfree(scrub_dev->region_name);
+	kfree(scrub_dev->region_groups);
+	kfree(scrub_dev->groups);
+	ida_free(&scrub_ida, scrub_dev->id);
+	kfree(scrub_dev);
+}
+
+static struct class scrub_class = {
+	.name = "scrub",
+	.dev_groups = scrub_dev_attr_groups,
+	.dev_release = scrub_dev_release,
+};
+
+/* sysfs attribute management */
+
+static ssize_t scrub_attr_show(struct device *dev,
+			       struct device_attribute *devattr, char *buf)
+{
+	int ret;
+	u64 val;
+	struct scrub_device_attribute *hattr = to_scrub_attr(devattr);
+
+	ret = hattr->ops->read(dev, hattr->attr, hattr->region_id, &val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "0x%llx\n", val);
+}
+
+static ssize_t scrub_attr_show_string(struct device *dev,
+				      struct device_attribute *devattr,
+				      char *buf)
+{
+	int ret;
+	struct scrub_device_attribute *hattr = to_scrub_attr(devattr);
+
+	ret = hattr->ops->read_string(dev, hattr->attr, hattr->region_id, buf);
+	if (ret < 0)
+		return ret;
+
+	return strlen(buf);
+}
+
+static ssize_t scrub_attr_store(struct device *dev,
+				struct device_attribute *devattr,
+				const char *buf, size_t count)
+{
+	int ret;
+	long val;
+	struct scrub_device_attribute *hattr = to_scrub_attr(devattr);
+
+	ret = kstrtol(buf, 10, &val);
+	if (ret < 0)
+		return ret;
+
+	ret = hattr->ops->write(dev, hattr->attr, hattr->region_id, val);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static ssize_t scrub_attr_store_hex(struct device *dev,
+				    struct device_attribute *devattr,
+				    const char *buf, size_t count)
+{
+	int ret;
+	u64 val;
+	struct scrub_device_attribute *hattr = to_scrub_attr(devattr);
+
+	ret = kstrtou64(buf, 16, &val);
+	if (ret < 0)
+		return ret;
+
+	ret = hattr->ops->write(dev, hattr->attr, hattr->region_id, val);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static bool is_hex_attr(u32 attr)
+{
+	return	(attr == scrub_addr_base) ||
+		(attr == scrub_addr_size);
+}
+
+static bool is_string_attr(u32 attr)
+{
+	return	attr == scrub_speed_available;
+}
+
+static struct attribute *scrub_genattr(const void *drvdata,
+				       u32 attr,
+				       const char *attrb_name,
+				       const struct scrub_ops *ops,
+				       int region_id)
+{
+	umode_t mode;
+	struct attribute *a;
+	struct device_attribute *dattr;
+	bool is_hex = is_hex_attr(attr);
+	struct scrub_device_attribute *hattr;
+	bool is_string = is_string_attr(attr);
+
+	/* The attribute is invisible if there is no template string */
+	if (!attrb_name)
+		return ERR_PTR(-ENOENT);
+
+	mode = ops->is_visible(drvdata, attr, region_id);
+	if (!mode)
+		return ERR_PTR(-ENOENT);
+
+	if ((mode & 0444) && ((is_string && !ops->read_string) ||
+			      (!is_string && !ops->read)))
+		return ERR_PTR(-EINVAL);
+	if ((mode & 0222) && (!ops->write))
+		return ERR_PTR(-EINVAL);
+
+	hattr = kzalloc(sizeof(*hattr), GFP_KERNEL);
+	if (!hattr)
+		return ERR_PTR(-ENOMEM);
+
+	hattr->attr = attr;
+	hattr->ops = ops;
+	hattr->region_id = region_id;
+
+	dattr = &hattr->dev_attr;
+	dattr->show = is_string ? scrub_attr_show_string : scrub_attr_show;
+	dattr->store = is_hex ? scrub_attr_store_hex : scrub_attr_store;
+
+	a = &dattr->attr;
+	sysfs_attr_init(a);
+	a->name = attrb_name;
+	a->mode = mode;
+
+	return a;
+}
+
+static const char * const scrub_common_attrs[] = {
+	[scrub_addr_base] = "addr_base",
+	[scrub_addr_size] = "addr_size",
+	[scrub_enable] = "enable",
+	[scrub_speed] = "speed",
+	[scrub_speed_available] = "speed_available",
+};
+
+static struct attribute **
+scrub_create_attrs(const void *drvdata, const struct scrub_ops *ops, int region_id)
+{
+	u32 attr;
+	int aindex = 0;
+	struct attribute *a;
+	struct attribute **attrs;
+
+	attrs = kcalloc(max_attrs, sizeof(*attrs), GFP_KERNEL);
+	if (!attrs)
+		return ERR_PTR(-ENOMEM);
+
+	for (attr = 0; attr < max_attrs; attr++) {
+		a = scrub_genattr(drvdata, attr, scrub_common_attrs[attr],
+				  ops, region_id);
+		if (IS_ERR(a)) {
+			if (PTR_ERR(a) != -ENOENT) {
+				scrub_free_attrs(attrs);
+				return ERR_PTR(PTR_ERR(a));
+			}
+			continue;
+		}
+		attrs[aindex++] = a;
+	}
+
+	return attrs;
+}
+
+static struct device *
+scrub_device_register(struct device *dev, const char *name, void *drvdata,
+		      const struct scrub_ops *ops,
+		      int nregions)
+{
+	struct device *hdev;
+	struct attribute **attrs;
+	int err, count, region_id;
+	struct attribute_group *group;
+	struct scrub_device *scrub_dev;
+	char (*region_name)[SCRUB_MAX_SYSFS_ATTR_NAME_LENGTH];
+
+	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) {
+		err = -ENOMEM;
+		goto free_scrub_dev;
+	}
+	int ngroups = 2; /* terminating NULL plus &scrub_dev->groups */
+
+	ngroups += nregions;
+
+	scrub_dev->groups = kcalloc(ngroups, sizeof(struct attribute_group *), GFP_KERNEL);
+	if (!scrub_dev->groups) {
+		err = -ENOMEM;
+		goto free_ida;
+	}
+
+	if (nregions) {
+		scrub_dev->region_groups = kcalloc(nregions, sizeof(struct attribute_group),
+						   GFP_KERNEL);
+		if (!scrub_dev->groups) {
+			err = -ENOMEM;
+			goto free_groups;
+		}
+		scrub_dev->region_name = kcalloc(nregions, SCRUB_MAX_SYSFS_ATTR_NAME_LENGTH,
+						 GFP_KERNEL);
+		if (!scrub_dev->region_name) {
+			err = -ENOMEM;
+			goto free_region_groups;
+		}
+	}
+
+	ngroups = 0;
+	scrub_dev->ngroups = 0;
+	if (nregions) {
+		region_name = scrub_dev->region_name;
+		for (region_id = 0; region_id < nregions; region_id++) {
+			attrs = scrub_create_attrs(drvdata, ops, region_id);
+			if (IS_ERR(attrs)) {
+				err = PTR_ERR(attrs);
+				goto free_attrs;
+			}
+			snprintf((char *)region_name, SCRUB_MAX_SYSFS_ATTR_NAME_LENGTH,
+				 "region%d", region_id);
+			scrub_dev->region_groups[region_id].name = (char *)region_name;
+			scrub_dev->region_groups[region_id].attrs = attrs;
+			region_name++;
+			scrub_dev->groups[ngroups++] = &scrub_dev->region_groups[region_id];
+			scrub_dev->ngroups = ngroups;
+		}
+	} else {
+		attrs = scrub_create_attrs(drvdata, ops, -1);
+		if (IS_ERR(attrs)) {
+			err = PTR_ERR(attrs);
+			goto free_region_name;
+		}
+		scrub_dev->group.attrs = attrs;
+		scrub_dev->groups[ngroups++] = &scrub_dev->group;
+		scrub_dev->ngroups = ngroups;
+	}
+
+	hdev->groups = scrub_dev->groups;
+	hdev->class = &scrub_class;
+	hdev->parent = dev;
+	dev_set_drvdata(hdev, drvdata);
+	dev_set_name(hdev, SCRUB_ID_FORMAT, scrub_dev->id);
+	snprintf(scrub_dev->name, SCRUB_DEV_MAX_NAME_LENGTH, "%s", name);
+	err = device_register(hdev);
+	if (err) {
+		put_device(hdev);
+		return ERR_PTR(err);
+	}
+
+	return hdev;
+
+free_attrs:
+	for (count = 0; count < scrub_dev->ngroups; count++) {
+		group = (struct attribute_group *)scrub_dev->groups[count];
+		if (group)
+			scrub_free_attrs(group->attrs);
+	}
+
+free_region_name:
+	kfree(scrub_dev->region_name);
+
+free_region_groups:
+	kfree(scrub_dev->region_groups);
+
+free_groups:
+	kfree(scrub_dev->groups);
+
+free_ida:
+	ida_free(&scrub_ida, scrub_dev->id);
+
+free_scrub_dev:
+	kfree(scrub_dev);
+	return ERR_PTR(err);
+}
+
+static void devm_scrub_release(void *dev)
+{
+	struct device *hdev = dev;
+
+	device_unregister(hdev);
+}
+
+/**
+ * devm_scrub_device_register - register hw scrubber device
+ * @dev: the parent device (mandatory)
+ * @name: hw scrubber name attribute (mandatory)
+ * @drvdata: driver data to attach to created device (mandatory)
+ * @ops: pointer to scrub_ops structure (mandatory)
+ * @nregions: number of scrub regions to create (optional)
+ *
+ * Returns the pointer to the new device. The new device is automatically
+ * unregistered with the parent device.
+ */
+struct device *
+devm_scrub_device_register(struct device *dev, const char *name,
+			   void *drvdata,
+			   const struct scrub_ops *ops,
+			   int nregions)
+{
+	struct device *hdev;
+
+	if (!dev || !name || !ops)
+		return ERR_PTR(-EINVAL);
+
+	hdev = scrub_device_register(dev, name, drvdata, ops, nregions);
+	if (IS_ERR(hdev))
+		return hdev;
+
+	if (devm_add_action_or_reset(dev, devm_scrub_release, hdev))
+		return NULL;
+
+	return hdev;
+}
+EXPORT_SYMBOL_GPL(devm_scrub_device_register);
+
+static int __init memory_scrub_control_init(void)
+{
+	int err;
+
+	err = class_register(&scrub_class);
+	if (err) {
+		pr_err("couldn't register memory scrub control sysfs class\n");
+		return err;
+	}
+
+	return 0;
+}
+subsys_initcall(memory_scrub_control_init);
diff --git a/include/memory/memory-scrub.h b/include/memory/memory-scrub.h
new file mode 100755
index 000000000000..8e999c9daaed
--- /dev/null
+++ b/include/memory/memory-scrub.h
@@ -0,0 +1,85 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Memory scrub controller driver support to configure
+ * the parameters of the memory scrubbers and enable.
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ */
+
+#ifndef __MEMORY_SCRUB_H
+#define __MEMORY_SCRUB_H
+
+#include <linux/types.h>
+
+enum scrub_types {
+	scrub_common,
+	scrub_max,
+};
+
+enum scrub_attributes {
+	scrub_addr_base,
+	scrub_addr_size,
+	scrub_enable,
+	scrub_speed,
+	scrub_speed_available,
+	max_attrs,
+};
+
+#define SCRUB_ENABLE		BIT(scrub_enable)
+#define SCRUB_ADDR_BASE		BIT(scrub_addr_base)
+#define SCRUB_ADDR_SIZE		BIT(scrub_addr_size)
+#define SCRUB_SPEED		BIT(scrub_speed)
+#define SCRUB_SPEED_AVAILABLE	BIT(scrub_speed_available)
+
+/**
+ * struct scrub_ops - scrub device operations
+ * @is_visible: Callback to return attribute visibility. Mandatory.
+ *		Parameters are:
+ *		@drvdata:
+ *			pointer to driver-private data structure passed
+ *			as argument to scrub_device_register().
+ *		@attr:	scrubber attribute
+ *		@region_id:
+ *			memory region id
+ *		The function returns the file permissions.
+ *		If the return value is 0, no attribute will be created.
+ * @read:	Read callback for data attributes. Mandatory if readable
+ *		data attributes are present.
+ *		Parameters are:
+ *		@dev:	pointer to hardware scrub device
+ *		@attr:	scrubber attribute
+ *		@region_id:
+ *			memory region id
+ *		@val:	pointer to returned value
+ *		The function returns 0 on success or a negative error number.
+ * @read_string: Read callback for string attributes. Mandatory if string
+ *		attributes are present.
+ *		Parameters are:
+ *		@dev:	pointer to hardware scrub device
+ *		@attr:	scrubber attribute
+ *		@region_id:
+ *			memory region id
+ *		@buf:	pointer to buffer to copy string
+ *		The function returns 0 on success or a negative error number.
+ * @write:	Write callback for data attributes. Mandatory if writeable
+ *		data attributes are present.
+ *		Parameters are:
+ *		@dev:	pointer to hardware scrub device
+ *		@attr:	scrubber attribute
+ *		@region_id:
+ *			memory region id
+ *		@val:	value to write
+ *		The function returns 0 on success or a negative error number.
+ */
+struct scrub_ops {
+	umode_t (*is_visible)(const void *drvdata, u32 attr, int region_id);
+	int (*read)(struct device *dev, u32 attr, int region_id, u64 *val);
+	int (*read_string)(struct device *dev, u32 attr, int region_id, char *buf);
+	int (*write)(struct device *dev, u32 attr, int region_id, u64 val);
+};
+
+struct device *
+devm_scrub_device_register(struct device *dev, const char *name,
+			   void *drvdata, const struct scrub_ops *ops,
+			   int nregions);
+#endif /* __MEMORY_SCRUB_H */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 1/9] memory: scrub: Add scrub driver supports configuring memory scrubbers in the system shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-22  0:07   ` Jiaqi Yan
  2023-09-15 17:28 ` [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver shiju.jose
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Add sysfs documentation entries for the set of attributes those are
exposed in /sys/class/scrub/ by the scrub driver. These attributes
support configuring parameters of a scrub device.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 .../ABI/testing/sysfs-class-scrub-configure   | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure

diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure b/Documentation/ABI/testing/sysfs-class-scrub-configure
new file mode 100644
index 000000000000..347e2167dc62
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
@@ -0,0 +1,82 @@
+What:		/sys/class/scrub/
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		The scrub/ class subdirectory belongs to the
+		scrubber subsystem.
+
+What:		/sys/class/scrub/scrubX/
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		The /sys/class/scrub/scrub{0,1,2,3,...} directories
+		correspond to each scrub device.
+
+What:		/sys/class/scrub/scrubX/name
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		(RO) name of the memory scrub device
+
+What:		/sys/class/scrub/scrubX/regionY/
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		The /sys/class/scrub/scrubX/region{0,1,2,3,...}
+		directories correspond to each scrub region under a scrub device.
+		Scrub region is a physical address range for which scrub may be
+		separately controlled. Regions may overlap in which case the
+		scrubbing rate of the overlapped memory will be at least that
+		expected due to each overlapping region.
+
+What:		/sys/class/scrub/scrubX/regionY/addr_base
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		(RW) The base of the address range of the memory region
+		to be patrol scrubbed.
+		On reading, returns the base of the memory region for
+		the actual address range(The platform calculates
+		the nearest patrol scrub boundary address from where
+		it can start scrubbing).
+
+What:		/sys/class/scrub/scrubX/regionY/addr_size
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		(RW) The size of the address range to be patrol scrubbed.
+		On reading, returns the size of the memory region for
+		the actual address range.
+
+What:		/sys/class/scrub/scrubX/regionY/enable
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		(WO) Start/Stop scrubbing the memory region.
+		1 - enable the memory scrubbing.
+		0 - disable the memory scrubbing..
+
+What:		/sys/class/scrub/scrubX/regionY/speed_available
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		(RO) Supported range for the partol speed(scrub rate)
+		by the scrubber for a memory region.
+		The unit of the scrub rate vary depends on the scrubber.
+
+What:		/sys/class/scrub/scrubX/regionY/speed
+Date:		September 2023
+KernelVersion:	6.7
+Contact:	linux-kernel@vger.kernel.org
+Description:
+		(RW) The partol speed(scrub rate) on the memory region specified and
+		it must be with in the supported range by the scrubber.
+		The unit of the scrub rate vary depends on the scrubber.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 1/9] memory: scrub: Add scrub driver supports configuring memory scrubbers in the system shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-18  7:23   ` David Hildenbrand
  2023-09-15 17:28 ` [RFC PATCH 4/9] ACPI:RASF: Add extract RASF table to register RASF platform devices shiju.jose
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Add documentation for scrub driver, supports configure scrub parameters,
in Documentation/scrub-configure.rst

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 Documentation/scrub-configure.rst | 55 +++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/scrub-configure.rst

diff --git a/Documentation/scrub-configure.rst b/Documentation/scrub-configure.rst
new file mode 100644
index 000000000000..9f8581b88788
--- /dev/null
+++ b/Documentation/scrub-configure.rst
@@ -0,0 +1,55 @@
+==========================
+Scrub subsystem driver
+==========================
+
+Copyright (c) 2023 HiSilicon Limited.
+
+:Author:   Shiju Jose <shiju.jose@huawei.com>
+:License:  The GNU Free Documentation License, Version 1.2
+          (dual licensed under the GPL v2)
+:Original Reviewers:
+
+- Written for: 6.7
+- Updated for:
+
+Introduction
+------------
+The scrub subsystem driver provides the interface for configure the
+parameters of memory scrubbers in the system. The scrub device drivers
+in the system register with the scrub configure subsystem.
+
+The scrub configure driver exposes the scrub controls to the user
+via sysfs.
+
+The File System
+---------------
+
+The configuration parameters of the registered scrubbers could be
+accessed via the /sys/class/scrub/scrubX/regionN/
+
+sysfs
+-----
+
+Sysfs files are documented in
+`Documentation/ABI/testing/sysfs-class-scrub-configure`.
+
+Example
+-------
+
+  The usage takes the form shown in this example::
+
+    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
+    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
+    # cat /sys/class/scrub/scrub0/region0/speed_available
+    # 1-60
+    # echo 25 > /sys/class/scrub/scrub0/region0/speed
+    # echo 1 > /sys/class/scrub/scrub0/region0/enable
+
+    # cat /sys/class/scrub/scrub0/region0/speed
+    # 0x19
+    # cat /sys/class/scrub/scrub0/region0/addr_base
+    # 0x100000
+    # cat /sys/class/scrub/scrub0/region0/addr_size
+    # 0x200000
+
+    # echo 0 > /sys/class/scrub/scrub0/region0/enable
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 4/9] ACPI:RASF: Add extract RASF table to register RASF platform devices
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (2 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 5/9] ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces shiju.jose
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: A Somasundaram <somasundaram.a@hpe.com>

The driver adds extraction of ACPI RASF table as per ACPI 5.1 & upwards
revision. ACPI specification section 5.2.20 for RASF table is the reference
for this implementation. Driver adds a platform device which binds to the
RASF memory driver.

Signed-off-by: A Somasundaram <somasundaram.a@hpe.com>
Co-developed-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/acpi/Kconfig     |  9 ++++
 drivers/acpi/Makefile    |  1 +
 drivers/acpi/rasf_acpi.c | 97 ++++++++++++++++++++++++++++++++++++++++
 include/acpi/rasf_acpi.h | 19 ++++++++
 4 files changed, 126 insertions(+)
 create mode 100755 drivers/acpi/rasf_acpi.c
 create mode 100755 include/acpi/rasf_acpi.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index cee82b473dc5..e5fd8edc5b35 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -279,6 +279,15 @@ config ACPI_CPPC_LIB
 	  If your platform does not support CPPC in firmware,
 	  leave this option disabled.
 
+config ACPI_RASF
+	bool "ACPI RASF driver"
+	depends on ACPI_PROCESSOR
+	select MAILBOX
+	help
+	  The driver adds support for extraction of RASF table from OS
+	  system table. Driver adds platform device which binds to the
+	  RASF memory driver.
+
 config ACPI_PROCESSOR
 	tristate "Processor"
 	depends on X86 || IA64 || ARM64 || LOONGARCH
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index eaa09bf52f17..f8a1263f6128 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
 obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
 obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
 obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
+obj-$(CONFIG_ACPI_RASF)		+= rasf_acpi.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
 obj-$(CONFIG_ACPI_PPTT) 	+= pptt.o
 obj-$(CONFIG_ACPI_PFRUT)	+= pfr_update.o pfr_telemetry.o
diff --git a/drivers/acpi/rasf_acpi.c b/drivers/acpi/rasf_acpi.c
new file mode 100755
index 000000000000..b30ba2a5e4ff
--- /dev/null
+++ b/drivers/acpi/rasf_acpi.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * rasf_acpi.c - ACPI RASF table processing functions
+ *
+ * (C) Copyright 2014, 2015 Hewlett-Packard Enterprises.
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ *
+ * Support for
+ * RASF - ACPI 6.5 Specification, section 5.2.20
+ *
+ * Code contains RASF init, which extracts the ACPI RASF table and
+ * RASF platform communication channel identifier for communicating
+ * with the ACPI compliant platform that contains RASF command
+ * support in the hardware. Driver adds platform device which
+ * binds to the RASF memory driver.
+ */
+
+#define pr_fmt(fmt)	"ACPI RASF: " fmt
+
+#include <linux/export.h>
+#include <linux/delay.h>
+#include <linux/ktime.h>
+#include <linux/platform_device.h>
+#include <acpi/rasf_acpi.h>
+#include <acpi/acpixf.h>
+
+static struct platform_device *rasf_add_platform_device(char *name, const void *data,
+							size_t size)
+{
+	int ret;
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc(name, PLATFORM_DEVID_AUTO);
+	if (!pdev)
+		return NULL;
+
+	ret = platform_device_add_data(pdev, data, size);
+	if (ret)
+		goto dev_put;
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto dev_put;
+
+	return pdev;
+
+dev_put:
+	platform_device_put(pdev);
+
+	return NULL;
+}
+
+int __init rasf_acpi_init(void)
+{
+	acpi_status status;
+	acpi_size rasf_size;
+	int pcc_subspace_idx;
+	struct platform_device *pdev;
+	struct acpi_table_rasf *pRasfTable;
+	struct acpi_table_header *pAcpiTable = NULL;
+
+	status = acpi_get_table("RASF", 0, &pAcpiTable);
+	if (ACPI_FAILURE(status) || !pAcpiTable) {
+		pr_err("ACPI RASF driver failed to initialize, get table failed\n");
+		return RASF_FAILURE;
+	}
+
+	rasf_size = pAcpiTable->length;
+	if (rasf_size < sizeof(struct acpi_table_rasf)) {
+		pr_err("ACPI RASF table present but broken (too short #1)\n");
+		goto free_rasf_table;
+	}
+
+	pRasfTable = (struct acpi_table_rasf *)pAcpiTable;
+
+	/* Extract the PCC subspace channel id from the table */
+	memcpy(&pcc_subspace_idx, &pRasfTable->channel_id, sizeof(pcc_subspace_idx));
+	pr_debug("ACPI RASF pcc subspace channel id =%d\n",
+		 pcc_subspace_idx);
+	if (pcc_subspace_idx < 0)
+		goto free_rasf_table;
+
+	/* Add the platform device and bind rasf memory driver */
+	pdev = rasf_add_platform_device("rasf", &pcc_subspace_idx,
+					sizeof(pcc_subspace_idx));
+	if (!pdev)
+		goto free_rasf_table;
+
+	acpi_put_table(pAcpiTable);
+	return RASF_SUCCESS;
+
+free_rasf_table:
+	acpi_put_table(pAcpiTable);
+	return RASF_FAILURE;
+}
+late_initcall(rasf_acpi_init)
diff --git a/include/acpi/rasf_acpi.h b/include/acpi/rasf_acpi.h
new file mode 100755
index 000000000000..1c4c3e94d472
--- /dev/null
+++ b/include/acpi/rasf_acpi.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/*
+ * RASF Diagnostic driver header file
+ *
+ * (C) Copyright 2014, 2015 Hewlett-Packard Enterprises
+ *
+ * Copyright (c) 2023 HiSilicon Limited
+ */
+
+#ifndef _RASF_ACPI_H
+#define _RASF_ACPI_H
+
+#include <linux/acpi.h>
+#include <linux/types.h>
+
+#define RASF_FAILURE 0
+#define RASF_SUCCESS 1
+
+#endif /* _RASF_ACPI_H */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 5/9] ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (3 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 4/9] ACPI:RASF: Add extract RASF table to register RASF platform devices shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 6/9] memory: RASF: Add memory RASF driver shiju.jose
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: A Somasundaram <somasundaram.a@hpe.com>

The code contains PCC interfaces for RASF and RAS2 table, functions to send
RASF commands as per ACPI 5.1 and RAS2 commands as per ACPI 6.5 & upwards
revision.

References for this implementation,
ACPI specification, section 5.2.20 for RASF table, section 5.2.21 for RAS2
table and chapter 14 for PCC (Platform Communication Channel).

Driver uses PCC interfaces to communicate to the ACPI HW.
This code implements PCC interfaces and the functions to send the RASF/RAS2
commands to be used by OSPM.

Signed-off-by: A Somasundaram <somasundaram.a@hpe.com>
Co-developed-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/acpi/Kconfig            |  12 +-
 drivers/acpi/Makefile           |   2 +-
 drivers/acpi/rasf_acpi.c        |  26 ---
 drivers/acpi/rasf_acpi_common.c | 272 ++++++++++++++++++++++++++++++++
 include/acpi/rasf_acpi.h        |  40 +++++
 5 files changed, 322 insertions(+), 30 deletions(-)
 create mode 100755 drivers/acpi/rasf_acpi_common.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index e5fd8edc5b35..057f7fbc9887 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -283,10 +283,16 @@ config ACPI_RASF
 	bool "ACPI RASF driver"
 	depends on ACPI_PROCESSOR
 	select MAILBOX
+	select PCC
 	help
-	  The driver adds support for extraction of RASF table from OS
-	  system table. Driver adds platform device which binds to the
-	  RASF memory driver.
+	  The driver adds support for PCC (platform communication
+	  channel) interfaces to communicate with the ACPI complaint
+	  hardware platform supports RASF(RAS Feature table) or
+	  and RAS2(RAS2 Feature table).
+	  The driver adds support for RASF(extraction of RASF tables
+	  from OS system table), PCC interfaces and OSPM interfaces to
+	  send RASF & RAS2 commands. Driver adds platform device which
+	  binds to the RASF/RAS2 memory driver.
 
 config ACPI_PROCESSOR
 	tristate "Processor"
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index f8a1263f6128..dd62d936cbe1 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -104,7 +104,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
 obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
 obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
 obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
-obj-$(CONFIG_ACPI_RASF)		+= rasf_acpi.o
+obj-$(CONFIG_ACPI_RASF)		+= rasf_acpi_common.o rasf_acpi.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
 obj-$(CONFIG_ACPI_PPTT) 	+= pptt.o
 obj-$(CONFIG_ACPI_PFRUT)	+= pfr_update.o pfr_telemetry.o
diff --git a/drivers/acpi/rasf_acpi.c b/drivers/acpi/rasf_acpi.c
index b30ba2a5e4ff..4c752fab9c4c 100755
--- a/drivers/acpi/rasf_acpi.c
+++ b/drivers/acpi/rasf_acpi.c
@@ -25,32 +25,6 @@
 #include <acpi/rasf_acpi.h>
 #include <acpi/acpixf.h>
 
-static struct platform_device *rasf_add_platform_device(char *name, const void *data,
-							size_t size)
-{
-	int ret;
-	struct platform_device *pdev;
-
-	pdev = platform_device_alloc(name, PLATFORM_DEVID_AUTO);
-	if (!pdev)
-		return NULL;
-
-	ret = platform_device_add_data(pdev, data, size);
-	if (ret)
-		goto dev_put;
-
-	ret = platform_device_add(pdev);
-	if (ret)
-		goto dev_put;
-
-	return pdev;
-
-dev_put:
-	platform_device_put(pdev);
-
-	return NULL;
-}
-
 int __init rasf_acpi_init(void)
 {
 	acpi_status status;
diff --git a/drivers/acpi/rasf_acpi_common.c b/drivers/acpi/rasf_acpi_common.c
new file mode 100755
index 000000000000..3ee34f5d12d3
--- /dev/null
+++ b/drivers/acpi/rasf_acpi_common.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * rasf_acpi_common.c - ACPI RASF table processing common functions
+ *
+ * (C) Copyright 2014, 2015 Hewlett-Packard Enterprises.
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ *
+ * Support for
+ * RASF - ACPI 6.5 Specification, section 5.2.20
+ * RAS2 - ACPI 6.5 Specification, section 5.2.21
+ * PCC(Platform Communications Channel) - ACPI 6.5 Specification,
+ * chapter 14.
+ *
+ * Code contains common functions for RASF.
+ * PCC(Platform communication channel) interfaces for the RASF & RAS2
+ * and the functions for sending RASF & RAS2 commands to the ACPI HW.
+ */
+
+#define pr_fmt(fmt)	"ACPI RASF COMMON: " fmt
+
+#include <linux/export.h>
+#include <linux/delay.h>
+#include <linux/ktime.h>
+#include <linux/platform_device.h>
+#include <acpi/rasf_acpi.h>
+#include <acpi/acpixf.h>
+
+static int rasf_check_pcc_chan(struct rasf_context *rasf_ctx)
+{
+	int ret = -EIO;
+	struct acpi_rasf_shared_memory  __iomem *generic_comm_base = rasf_ctx->pcc_comm_addr;
+	ktime_t next_deadline = ktime_add(ktime_get(), rasf_ctx->deadline);
+
+	while (!ktime_after(ktime_get(), next_deadline)) {
+		/*
+		 * As per ACPI spec, the PCC space wil be initialized by
+		 * platform and should have set the command completion bit when
+		 * PCC can be used by OSPM
+		 */
+		if (readw_relaxed(&generic_comm_base->status) & RASF_PCC_CMD_COMPLETE) {
+			ret = 0;
+			break;
+		}
+		/*
+		 * Reducing the bus traffic in case this loop takes longer than
+		 * a few retries.
+		 */
+		udelay(10);
+	}
+
+	return ret;
+}
+
+/**
+ * rasf_send_pcc_cmd() - Send RASF command via PCC channel
+ * @rasf_ctx:	pointer to the rasf context structure
+ * @cmd:	command to send
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+int rasf_send_pcc_cmd(struct rasf_context *rasf_ctx, u16 cmd)
+{
+	int ret = -EIO;
+	struct acpi_rasf_shared_memory  *generic_comm_base =
+		(struct acpi_rasf_shared_memory *)rasf_ctx->pcc_comm_addr;
+	static ktime_t last_cmd_cmpl_time, last_mpar_reset;
+	static int mpar_count;
+	unsigned int time_delta;
+
+	if (cmd == RASF_PCC_CMD_EXEC) {
+		ret = rasf_check_pcc_chan(rasf_ctx);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Handle the Minimum Request Turnaround Time(MRTT)
+	 * "The minimum amount of time that OSPM must wait after the completion
+	 * of a command before issuing the next command, in microseconds"
+	 */
+	if (rasf_ctx->pcc_mrtt) {
+		time_delta = ktime_us_delta(ktime_get(), last_cmd_cmpl_time);
+		if (rasf_ctx->pcc_mrtt > time_delta)
+			udelay(rasf_ctx->pcc_mrtt - time_delta);
+	}
+
+	/*
+	 * Handle the non-zero Maximum Periodic Access Rate(MPAR)
+	 * "The maximum number of periodic requests that the subspace channel can
+	 * support, reported in commands per minute. 0 indicates no limitation."
+	 *
+	 * This parameter should be ideally zero or large enough so that it can
+	 * handle maximum number of requests that all the cores in the system can
+	 * collectively generate. If it is not, we will follow the spec and just
+	 * not send the request to the platform after hitting the MPAR limit in
+	 * any 60s window
+	 */
+	if (rasf_ctx->pcc_mpar) {
+		if (mpar_count == 0) {
+			time_delta = ktime_ms_delta(ktime_get(), last_mpar_reset);
+			if (time_delta < 60 * MSEC_PER_SEC) {
+				pr_debug("PCC cmd not sent due to MPAR limit");
+				return -EIO;
+			}
+			last_mpar_reset = ktime_get();
+			mpar_count = rasf_ctx->pcc_mpar;
+		}
+		mpar_count--;
+	}
+
+	/* Write to the shared comm region. */
+	writew_relaxed(cmd, &generic_comm_base->command);
+
+	/* Flip CMD COMPLETE bit */
+	writew_relaxed(0, &generic_comm_base->status);
+
+	/* Ring doorbell */
+	ret = mbox_send_message(rasf_ctx->pcc_channel, &cmd);
+	if (ret < 0) {
+		pr_err("Err sending PCC mbox message. cmd:%d, ret:%d\n",
+				cmd, ret);
+		return ret;
+	}
+
+	/*
+	 * For READs we need to ensure the cmd completed to ensure
+	 * the ensuing read()s can proceed. For WRITEs we dont care
+	 * because the actual write()s are done before coming here
+	 * and the next READ or WRITE will check if the channel
+	 * is busy/free at the entry of this call.
+	 *
+	 * If Minimum Request Turnaround Time is non-zero, we need
+	 * to record the completion time of both READ and WRITE
+	 * command for proper handling of MRTT, so we need to check
+	 * for pcc_mrtt in addition to CMD_READ
+	 */
+	if (cmd == RASF_PCC_CMD_EXEC || rasf_ctx->pcc_mrtt) {
+		ret = rasf_check_pcc_chan(rasf_ctx);
+		if (rasf_ctx->pcc_mrtt)
+			last_cmd_cmpl_time = ktime_get();
+	}
+
+	if (rasf_ctx->pcc_channel->mbox->txdone_irq)
+		mbox_chan_txdone(rasf_ctx->pcc_channel, ret);
+	else
+		mbox_client_txdone(rasf_ctx->pcc_channel, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rasf_send_pcc_cmd);
+
+/**
+ * rasf_register_pcc_channel() - Register PCC channel
+ * @rasf_ctx:	pointer to the rasf context structure
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+int rasf_register_pcc_channel(struct rasf_context *rasf_ctx)
+{
+	u64 usecs_lat;
+	unsigned int len;
+	struct pcc_mbox_chan *pcc_chan;
+	struct mbox_client *rasf_mbox_cl;
+	struct acpi_pcct_hw_reduced *rasf_ss;
+
+	rasf_mbox_cl = &rasf_ctx->mbox_client;
+	if (!rasf_mbox_cl || rasf_ctx->pcc_subspace_idx < 0)
+		return -EINVAL;
+
+	pcc_chan = pcc_mbox_request_channel(rasf_mbox_cl,
+				rasf_ctx->pcc_subspace_idx);
+
+	if (IS_ERR(pcc_chan)) {
+		pr_err("Failed to find PCC channel for subspace %d\n",
+		       rasf_ctx->pcc_subspace_idx);
+		return -ENODEV;
+	}
+	rasf_ctx->pcc_chan = pcc_chan;
+	rasf_ctx->pcc_channel = pcc_chan->mchan;
+	/*
+	 * The PCC mailbox controller driver should
+	 * have parsed the PCCT (global table of all
+	 * PCC channels) and stored pointers to the
+	 * subspace communication region in con_priv.
+	 */
+	rasf_ss = rasf_ctx->pcc_channel->con_priv;
+
+	if (!rasf_ss) {
+		pr_err("No PCC subspace found for RASF\n");
+		pcc_mbox_free_channel(rasf_ctx->pcc_chan);
+		return -ENODEV;
+	}
+
+	/*
+	 * This is the shared communication region
+	 * for the OS and Platform to communicate over.
+	 */
+	rasf_ctx->comm_base_addr = rasf_ss->base_address;
+	len = rasf_ss->length;
+	pr_debug("PCC subspace for RASF=0x%llx len=%d\n",
+		  rasf_ctx->comm_base_addr, len);
+
+	/*
+	 * rasf_ss->latency is just a Nominal value. In reality
+	 * the remote processor could be much slower to reply.
+	 * So add an arbitrary amount of wait on top of Nominal.
+	 */
+	usecs_lat = RASF_NUM_RETRIES * rasf_ss->latency;
+	rasf_ctx->deadline = ns_to_ktime(usecs_lat * NSEC_PER_USEC);
+	rasf_ctx->pcc_mrtt = rasf_ss->min_turnaround_time;
+	rasf_ctx->pcc_mpar = rasf_ss->max_access_rate;
+	rasf_ctx->pcc_comm_addr = acpi_os_ioremap(rasf_ctx->comm_base_addr, len);
+	pr_debug("pcc_comm_addr=%p\n", rasf_ctx->pcc_comm_addr);
+
+	/* Set flag so that we dont come here for each CPU. */
+	rasf_ctx->pcc_channel_acquired = true;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rasf_register_pcc_channel);
+
+/**
+ * rasf_unregister_pcc_channel() - Unregister PCC channel
+ * @rasf_ctx:	pointer to the rasf context structure
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+int rasf_unregister_pcc_channel(struct rasf_context *rasf_ctx)
+{
+	if (!rasf_ctx->pcc_chan)
+		return -EINVAL;
+
+	pcc_mbox_free_channel(rasf_ctx->pcc_chan);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rasf_unregister_pcc_channel);
+
+/**
+ * rasf_add_platform_device() - Add a platform device for RASF
+ * @name:	name of the device we're adding
+ * @data:	platform specific data for this platform device
+ * @size:	size of platform specific data
+ *
+ * Returns: pointer to platform device on success, an error otherwise
+ */
+struct platform_device *rasf_add_platform_device(char *name, const void *data,
+						 size_t size)
+{
+	int ret;
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc(name, PLATFORM_DEVID_AUTO);
+	if (!pdev)
+		return NULL;
+
+	ret = platform_device_add_data(pdev, data, size);
+	if (ret)
+		goto dev_put;
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto dev_put;
+
+	return pdev;
+
+dev_put:
+	platform_device_put(pdev);
+
+	return NULL;
+}
diff --git a/include/acpi/rasf_acpi.h b/include/acpi/rasf_acpi.h
index 1c4c3e94d472..e91a451c5dc1 100755
--- a/include/acpi/rasf_acpi.h
+++ b/include/acpi/rasf_acpi.h
@@ -11,9 +11,49 @@
 #define _RASF_ACPI_H
 
 #include <linux/acpi.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox_client.h>
 #include <linux/types.h>
+#include <acpi/pcc.h>
+
+#define RASF_PCC_CMD_COMPLETE 1
+
+/* RASF specific PCC commands */
+#define RASF_PCC_CMD_EXEC 0x01
 
 #define RASF_FAILURE 0
 #define RASF_SUCCESS 1
 
+/*
+ * Arbitrary Retries for PCC commands.
+ */
+#define RASF_NUM_RETRIES 600
+
+/*
+ * Data structures for PCC communication and RASF table
+ */
+struct rasf_context {
+	struct device *dev;
+	int id;
+	struct mbox_client mbox_client;
+	struct mbox_chan *pcc_channel;
+	struct pcc_mbox_chan *pcc_chan;
+	void __iomem *pcc_comm_addr;
+	u64 comm_base_addr;
+	int pcc_subspace_idx;
+	bool pcc_channel_acquired;
+	ktime_t deadline;
+	unsigned int pcc_mpar;
+	unsigned int pcc_mrtt;
+	spinlock_t spinlock; /* Lock to provide mutually exclusive access to PCC channel */
+	struct device *scrub_dev;
+	u8 n_regions;
+	const struct rasf_hw_scrub_ops *ops;
+};
+
+struct platform_device *rasf_add_platform_device(char *name, const void *data,
+						 size_t size);
+int rasf_send_pcc_cmd(struct rasf_context *rasf_ctx, u16 cmd);
+int rasf_register_pcc_channel(struct rasf_context *rasf_ctx);
+int rasf_unregister_pcc_channel(struct rasf_context *rasf_ctx);
 #endif /* _RASF_ACPI_H */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 6/9] memory: RASF: Add memory RASF driver
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (4 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 5/9] ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 7/9] ACPICA: ACPI 6.5: Add support for RAS2 table shiju.jose
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Memory RASF driver binds to the platform device add by the ACPI RASF
driver.

Driver registers the PCC channel for communicating with the ACPI compliant
platform that contains RASF command support in the hardware.

Add interface functions to support configuring the parameters of HW patrol
scrubber in the system, which exposed to the kernel via the RASF and PCC,
using the RASF commands.

Add support for RASF scrub devices to register with scrub driver.
This enables user to configure the parameters of HW patrol scrubbers,
which exposed to the kernel via the RASF table, using the scrub sysfs
attributes.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/memory/Kconfig       |  14 ++
 drivers/memory/Makefile      |   2 +
 drivers/memory/rasf.c        | 335 +++++++++++++++++++++++++++++++++++
 drivers/memory/rasf_common.c | 251 ++++++++++++++++++++++++++
 include/memory/rasf.h        |  82 +++++++++
 5 files changed, 684 insertions(+)
 create mode 100644 drivers/memory/rasf.c
 create mode 100644 drivers/memory/rasf_common.c
 create mode 100755 include/memory/rasf.h

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index d2e015c09d83..b831e76fcdbf 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -225,6 +225,20 @@ config STM32_FMC2_EBI
 	  devices (like SRAM, ethernet adapters, FPGAs, LCD displays, ...) on
 	  SOCs containing the FMC2 External Bus Interface.
 
+config MEM_RASF
+	bool "Memory RASF driver"
+	depends on ACPI_RASF
+	depends on SCRUB
+	help
+	  The driver bound to the platform device added by the ACPI RASF
+	  driver. Driver registers the PCC channel for communicating with
+	  the ACPI compliant platform that contains RASF/RAS2 command support
+	  in the hardware.
+	  Registers with the scrub configure driver to provide sysfs interfaces
+	  for configuring the hw patrol scrubber in the system, which exposed
+	  via the ACPI RASF/RAS2 table and PCC. Provides the interface functions
+	  support configuring the HW patrol scrubbers in the system.
+
 source "drivers/memory/samsung/Kconfig"
 source "drivers/memory/tegra/Kconfig"
 source "drivers/memory/scrub/Kconfig"
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 4b37312cb342..49340cd100fc 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -7,6 +7,8 @@ obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
 ifeq ($(CONFIG_DDR),y)
 obj-$(CONFIG_OF)		+= of_memory.o
 endif
+obj-$(CONFIG_MEM_RASF)		+= rasf_common.o rasf.o
+
 obj-$(CONFIG_ARM_PL172_MPMC)	+= pl172.o
 obj-$(CONFIG_ATMEL_EBI)		+= atmel-ebi.o
 obj-$(CONFIG_BRCMSTB_DPFE)	+= brcmstb_dpfe.o
diff --git a/drivers/memory/rasf.c b/drivers/memory/rasf.c
new file mode 100644
index 000000000000..b33024c0ed15
--- /dev/null
+++ b/drivers/memory/rasf.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * rasf.c - Memory RASF Driver
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ *
+ * This driver:
+ *  - Registers the PCC channel for communicating with the
+ *    ACPI compliant platform that contains RASF command
+ *    support in the hardware.
+ *  - Provides functions to configure HW patrol scrubber
+ *    in the system.
+ *  - Registers with the scrub configure driver for the
+ *    hw patrol scrubber in the system, which exposed via
+ *    the ACPI RASF table and PCC.
+ */
+
+#define pr_fmt(fmt)     "MEMORY RASF: " fmt
+
+#include <linux/cleanup.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <acpi/rasf_acpi.h>
+#include <memory/rasf.h>
+
+/* RASF specific definitions. */
+#define RASF_SCRUB	"rasf_scrub"
+#define RASF_SUPPORT_HW_PARTOL_SCRUB		BIT(0)
+#define RASF_EXPOSE_HW_PARTOL_SCRUB_TO_SW	BIT(1)
+
+#define RASF_TYPE_PATROL_SCRUB	0x0000
+
+#define RASF_GET_PATROL_PARAMETERS	0x01
+#define	RASF_START_PATROL_SCRUBBER	0x02
+#define	RASF_STOP_PATROL_SCRUBBER	0x03
+
+#define RASF_PATROL_SCRUB_RATE_VALID	BIT(0)
+#define RASF_PATROL_SCRUB_SPEED_MASK	GENMASK(3, 1)
+#define RASF_PATROL_SCRUB_SLOW	0x0
+#define RASF_PATROL_SCRUB_MEDIUM	0x4
+#define	RASF_PATROL_SCRUB_FAST	0x7
+
+/*
+ * The number of regions may not be relavent for RASF
+ */
+#define RASF_NUM_REGIONS        1
+
+#define to_rasf_ctx(cl)		\
+	container_of(cl, struct rasf_context, mbox_client)
+
+static void rasf_tx_done(struct mbox_client *cl, void *msg, int ret)
+{
+	if (ret) {
+		dev_dbg(cl->dev, "TX did not complete: CMD sent:%x, ret:%d\n",
+			*(u16 *)msg, ret);
+	} else {
+		dev_dbg(cl->dev, "TX completed. CMD sent:%x, ret:%d\n",
+			*(u16 *)msg, ret);
+	}
+}
+
+/*
+ * The below functions are exposed to OSPM, to query, configure and
+ * initiate memory patrol scrubber.
+ */
+static int rasf_is_patrol_scrub_support(struct rasf_context *rasf_ctx)
+{
+	int ret;
+	struct acpi_rasf_shared_memory  __iomem *generic_comm_base;
+
+	if (!rasf_ctx || !rasf_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = rasf_ctx->pcc_comm_addr;
+	guard(spinlock_irqsave)(&rasf_ctx->spinlock);
+	generic_comm_base->set_capabilities[0] = 0;
+
+	/* send command for reading RASF capabilities */
+	ret = rasf_send_pcc_cmd(rasf_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: rasf_send_pcc_cmd failed\n", __func__);
+		return ret;
+	}
+
+	return FIELD_GET(RASF_EXPOSE_HW_PARTOL_SCRUB_TO_SW,
+			  generic_comm_base->capabilities[0]);
+}
+
+static int rasf_get_patrol_scrub_params(struct rasf_context *rasf_ctx,
+					struct rasf_scrub_params *params)
+{
+	int ret = 0;
+	struct acpi_rasf_shared_memory  __iomem *generic_comm_base;
+	struct acpi_rasf_patrol_scrub_parameter __iomem *patrol_scrub_params;
+
+	if (!rasf_ctx || !rasf_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = rasf_ctx->pcc_comm_addr;
+	patrol_scrub_params = rasf_ctx->pcc_comm_addr + sizeof(*generic_comm_base);
+
+	guard(spinlock_irqsave)(&rasf_ctx->spinlock);
+	generic_comm_base->set_capabilities[0] = RASF_EXPOSE_HW_PARTOL_SCRUB_TO_SW;
+
+	/* send command for reading RASF capabilities */
+	ret = rasf_send_pcc_cmd(rasf_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: rasf_send_pcc_cmd failed\n", __func__);
+		return ret;
+	}
+
+	if (!(generic_comm_base->capabilities[0] & RASF_EXPOSE_HW_PARTOL_SCRUB_TO_SW) ||
+	    !(generic_comm_base->num_parameter_blocks)) {
+		pr_err("%s: Platform does not support HW Patrol Scrubber\n", __func__);
+		return -ENOTSUPP;
+	}
+
+	if (!patrol_scrub_params->requested_address_range[1]) {
+		pr_err("%s: Invalid requested address range, \
+			requested_address base=0x%llx \
+			requested_address size=0x%llx\n",
+			__func__,
+			patrol_scrub_params->requested_address_range[0],
+			patrol_scrub_params->requested_address_range[1]);
+		return -ENOTSUPP;
+	}
+
+	generic_comm_base->set_capabilities[0] = RASF_EXPOSE_HW_PARTOL_SCRUB_TO_SW;
+	patrol_scrub_params->header.type = RASF_TYPE_PATROL_SCRUB;
+	patrol_scrub_params->patrol_scrub_command = RASF_GET_PATROL_PARAMETERS;
+
+	/* send command for reading the HW patrol scrub parameters */
+	ret = rasf_send_pcc_cmd(rasf_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: failed to read HW patrol scrub parameters\n", __func__);
+		return ret;
+	}
+
+	/* copy output scrubber parameters */
+	params->addr_base = patrol_scrub_params->actual_address_range[0];
+	params->addr_size = patrol_scrub_params->actual_address_range[1];
+	params->flags = patrol_scrub_params->flags;
+	if (patrol_scrub_params->flags & RASF_PATROL_SCRUB_RATE_VALID) {
+		params->speed = FIELD_GET(RASF_PATROL_SCRUB_SPEED_MASK,
+					  patrol_scrub_params->flags);
+		snprintf(params->speed_avail, RASF_MAX_SPEED_RANGE_LENGTH,
+			"%d,%d,%d", RASF_PATROL_SCRUB_SLOW,
+			RASF_PATROL_SCRUB_MEDIUM, RASF_PATROL_SCRUB_FAST);
+	} else {
+		params->speed = 0;
+		snprintf(params->speed_avail, RASF_MAX_SPEED_RANGE_LENGTH,
+			"%s", "Unavailable");
+	}
+
+	return 0;
+}
+
+static int rasf_enable_patrol_scrub(struct rasf_context *rasf_ctx, bool enable)
+{
+	int ret = 0;
+	struct rasf_scrub_params params;
+	struct acpi_rasf_shared_memory  __iomem *generic_comm_base;
+	struct acpi_rasf_patrol_scrub_parameter __iomem *patrol_scrub_params;
+
+	if (!rasf_ctx || !rasf_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = rasf_ctx->pcc_comm_addr;
+	patrol_scrub_params = rasf_ctx->pcc_comm_addr + sizeof(*generic_comm_base);
+
+	if (enable) {
+		ret = rasf_get_patrol_scrub_params(rasf_ctx, &params);
+		if (ret)
+			return ret;
+	}
+
+	guard(spinlock_irqsave)(&rasf_ctx->spinlock);
+	generic_comm_base->set_capabilities[0] = RASF_EXPOSE_HW_PARTOL_SCRUB_TO_SW;
+	patrol_scrub_params->header.type = RASF_TYPE_PATROL_SCRUB;
+
+	if (enable) {
+		patrol_scrub_params->patrol_scrub_command = RASF_START_PATROL_SCRUBBER;
+		patrol_scrub_params->requested_address_range[0] = params.addr_base;
+		patrol_scrub_params->requested_address_range[1] = params.addr_size;
+		/* requested speed already set in the rasf_set_patrol_scrub_params */
+	} else
+		patrol_scrub_params->patrol_scrub_command = RASF_STOP_PATROL_SCRUBBER;
+
+	/* send command for enable/disable the HW patrol scrub */
+	ret = rasf_send_pcc_cmd(rasf_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: failed to enable/disable the HW patrol scrub\n", __func__);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int rasf_set_patrol_scrub_params(struct rasf_context *rasf_ctx,
+					struct rasf_scrub_params *params, u8 param_type)
+{
+	struct acpi_rasf_shared_memory  __iomem *generic_comm_base;
+	struct acpi_rasf_patrol_scrub_parameter __iomem *patrol_scrub_params;
+
+	if (!rasf_ctx || !rasf_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = rasf_ctx->pcc_comm_addr;
+	patrol_scrub_params = rasf_ctx->pcc_comm_addr + sizeof(*generic_comm_base);
+
+	guard(spinlock_irqsave)(&rasf_ctx->spinlock);
+	patrol_scrub_params->header.type = RASF_TYPE_PATROL_SCRUB;
+	if (param_type == RASF_MEM_SCRUB_PARAM_ADDR_BASE && params->addr_base) {
+		patrol_scrub_params->requested_address_range[0] = params->addr_base;
+	} else if (param_type == RASF_MEM_SCRUB_PARAM_ADDR_SIZE && params->addr_size) {
+		patrol_scrub_params->requested_address_range[1] = params->addr_size;
+	} else if (param_type == RASF_MEM_SCRUB_PARAM_SPEED) {
+		if ((params->speed != RASF_PATROL_SCRUB_SLOW) &&
+		    (params->speed != RASF_PATROL_SCRUB_MEDIUM) &&
+		    params->speed != RASF_PATROL_SCRUB_FAST) {
+			pr_warn("rasf driver failed to set patrol scrub speed=%d\n",
+				params->speed);
+			pr_warn("Supported speeds: slow:%d medium:%d fast:%d\n",
+				RASF_PATROL_SCRUB_SLOW, RASF_PATROL_SCRUB_MEDIUM,
+				RASF_PATROL_SCRUB_FAST);
+			return -EINVAL;
+		}
+
+		/*
+		 * ACPI 6.5 Spec Table 5.78: Parameter Block Structure for PATROL_SCRUB
+		 * Requested Speed (INPUT)
+		 * Bit [0]: Will be set if patrol scrub is already running
+		 *	    for address range specified in “Actual Address Range”
+		 * Bits [2:0]: Requested Patrol Speeds
+		 */
+
+		/* Is the description about Bit[0] in the Spec an error? */
+
+		patrol_scrub_params->requested_speed &= ~RASF_PATROL_SCRUB_SPEED_MASK;
+		patrol_scrub_params->requested_speed |= FIELD_PREP(RASF_PATROL_SCRUB_SPEED_MASK, params->speed);
+	} else {
+		pr_err("Invalid patrol scrub parameter to set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct rasf_hw_scrub_ops rasf_hw_ops = {
+	.enable_scrub = rasf_enable_patrol_scrub,
+	.get_scrub_params = rasf_get_patrol_scrub_params,
+	.set_scrub_params = rasf_set_patrol_scrub_params,
+};
+
+static const struct scrub_ops rasf_scrub_ops = {
+	.is_visible = rasf_hw_scrub_is_visible,
+	.read = rasf_hw_scrub_read,
+	.write = rasf_hw_scrub_write,
+	.read_string = rasf_hw_scrub_read_strings,
+};
+
+static void devm_rasf_release(void *rasf_ctx)
+{
+	rasf_unregister_pcc_channel(rasf_ctx);
+}
+
+static int rasf_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct mbox_client *cl;
+	struct device *hw_scrub_dev;
+	struct rasf_context *rasf_ctx;
+	char scrub_name[RASF_MAX_NAME_LENGTH];
+
+	rasf_ctx = devm_kzalloc(&pdev->dev, sizeof(*rasf_ctx), GFP_KERNEL);
+	if (!rasf_ctx)
+		return -ENOMEM;
+
+	rasf_ctx->dev = &pdev->dev;
+	rasf_ctx->ops = &rasf_hw_ops;
+	spin_lock_init(&rasf_ctx->spinlock);
+
+	platform_set_drvdata(pdev, rasf_ctx);
+	cl = &rasf_ctx->mbox_client;
+
+	/* Request mailbox channel */
+	cl->dev = &pdev->dev;
+	cl->tx_done = rasf_tx_done;
+	cl->knows_txdone = true;
+
+	rasf_ctx->pcc_subspace_idx = *((int *)pdev->dev.platform_data);
+	dev_dbg(&pdev->dev, "pcc-subspace-id=%d\n", rasf_ctx->pcc_subspace_idx);
+
+	ret = rasf_register_pcc_channel(rasf_ctx);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, devm_rasf_release, rasf_ctx);
+	if (ret < 0)
+		return ret;
+
+	if (rasf_is_patrol_scrub_support(rasf_ctx)) {
+		rasf_ctx->n_regions = RASF_NUM_REGIONS;
+		snprintf(scrub_name, sizeof(scrub_name), "%s", RASF_SCRUB);
+		hw_scrub_dev = devm_scrub_device_register(&pdev->dev, scrub_name, rasf_ctx,
+							  &rasf_scrub_ops,
+							  rasf_ctx->n_regions);
+		if (PTR_ERR_OR_ZERO(hw_scrub_dev))
+			return PTR_ERR_OR_ZERO(hw_scrub_dev);
+	}
+	rasf_ctx->scrub_dev = hw_scrub_dev;
+
+	return 0;
+}
+
+static const struct platform_device_id rasf_id_table[] = {
+	{ .name = "rasf", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, rasf_id_table);
+
+static struct platform_driver rasf_driver = {
+	.probe = rasf_probe,
+	.driver = {
+		.name = "rasf",
+		.suppress_bind_attrs = true,
+	},
+	.id_table = rasf_id_table,
+};
+module_driver(rasf_driver, platform_driver_register, platform_driver_unregister);
+
+MODULE_DESCRIPTION("rasf driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/memory/rasf_common.c b/drivers/memory/rasf_common.c
new file mode 100644
index 000000000000..aa0deb0d6d1e
--- /dev/null
+++ b/drivers/memory/rasf_common.c
@@ -0,0 +1,251 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * rasf_common.c - Common functions for memory RASF driver
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ *
+ * This driver implements call back functions for the scrub
+ * configure driver to configure the parameters of the hw patrol
+ * scrubbers in the system, which exposed via the ACPI RASF/RAS2
+ * table and PCC.
+ */
+
+#define pr_fmt(fmt)     "MEMORY RASF COMMON: " fmt
+
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <acpi/rasf_acpi.h>
+#include <memory/rasf.h>
+
+static int enable_write(struct rasf_context *rasf_ctx, long val)
+{
+	int ret;
+	bool scrub_enable = val;
+
+	ret = rasf_ctx->ops->enable_scrub(rasf_ctx, scrub_enable);
+	if (ret) {
+		pr_err("enable patrol scrub for enable fail, enable=%d ret=%d\n",
+		       scrub_enable, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int addr_base_read(struct rasf_context *rasf_ctx, u64 *val)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	ret = rasf_ctx->ops->get_scrub_params(rasf_ctx, &params);
+	if (ret) {
+		pr_err("get patrol scrub params fail ret=%d\n", ret);
+		return ret;
+	}
+	*val = params.addr_base;
+
+	return 0;
+}
+
+static int addr_base_write(struct rasf_context *rasf_ctx, u64 val)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	params.addr_base = val;
+	ret = rasf_ctx->ops->set_scrub_params(rasf_ctx, &params, RASF_MEM_SCRUB_PARAM_ADDR_BASE);
+	if (ret) {
+		pr_err("set patrol scrub params for addr_base fail ret=%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int addr_size_read(struct rasf_context *rasf_ctx, u64 *val)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	ret = rasf_ctx->ops->get_scrub_params(rasf_ctx, &params);
+	if (ret) {
+		pr_err("get patrol scrub params fail ret=%d\n", ret);
+		return ret;
+	}
+	*val = params.addr_size;
+
+	return 0;
+}
+
+static int addr_size_write(struct rasf_context *rasf_ctx, u64 val)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	params.addr_size = val;
+	ret = rasf_ctx->ops->set_scrub_params(rasf_ctx, &params, RASF_MEM_SCRUB_PARAM_ADDR_SIZE);
+	if (ret) {
+		pr_err("set patrol scrub params for addr_size fail ret=%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int speed_read(struct  rasf_context *rasf_ctx, u64 *val)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	ret = rasf_ctx->ops->get_scrub_params(rasf_ctx, &params);
+	if (ret) {
+		pr_err("get patrol scrub params fail ret=%d\n", ret);
+		return ret;
+	}
+	*val = params.speed;
+
+	return 0;
+}
+
+static int speed_write(struct rasf_context *rasf_ctx, long val)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	params.speed = val;
+	ret = rasf_ctx->ops->set_scrub_params(rasf_ctx, &params, RASF_MEM_SCRUB_PARAM_SPEED);
+	if (ret) {
+		pr_err("set patrol scrub params for speed fail ret=%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int speed_available_read(struct rasf_context *rasf_ctx, char *buf)
+{
+	int ret;
+	struct rasf_scrub_params params;
+
+	ret = rasf_ctx->ops->get_scrub_params(rasf_ctx, &params);
+	if (ret) {
+		pr_err("get patrol scrub params fail ret=%d\n", ret);
+		return ret;
+	}
+
+	sprintf(buf, "%s\n", params.speed_avail);
+
+	return 0;
+}
+
+/**
+ * rasf_hw_scrub_is_visible() - Callback to return attribute visibility
+ * @drv_data: Pointer to driver-private data structure passed
+ *	      as argument to devm_scrub_device_register().
+ * @attr: Scrub attribute
+ * @region_id: ID of the memory region
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+umode_t rasf_hw_scrub_is_visible(const void *drv_data, u32 attr, int region_id)
+{
+	switch (attr) {
+	case scrub_speed_available:
+		return 0444;
+	case scrub_enable:
+		return 0200;
+	case scrub_addr_base:
+	case scrub_addr_size:
+	case scrub_speed:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+/**
+ * rasf_hw_scrub_read() - Read callback for data attributes
+ * @device: Pointer to scrub device
+ * @attr: Scrub attribute
+ * @region_id: ID of the memory region
+ * @val: Pointer to the returned data
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+int rasf_hw_scrub_read(struct device *device, u32 attr, int region_id, u64 *val)
+{
+	struct rasf_context *rasf_ctx;
+
+	rasf_ctx = dev_get_drvdata(device);
+
+	switch (attr) {
+	case scrub_addr_base:
+		return addr_base_read(rasf_ctx, val);
+	case scrub_addr_size:
+		return addr_size_read(rasf_ctx, val);
+	case scrub_speed:
+		return speed_read(rasf_ctx, val);
+	default:
+		return -ENOTSUPP;
+	}
+}
+
+/**
+ * rasf_hw_scrub_write() - Write callback for data attributes
+ * @device: Pointer to scrub device
+ * @attr: Scrub attribute
+ * @region_id: ID of the memory region
+ * @val: Value to write
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+int rasf_hw_scrub_write(struct device *device, u32 attr, int region_id, u64 val)
+{
+	struct rasf_context *rasf_ctx;
+
+	rasf_ctx = dev_get_drvdata(device);
+
+	switch (attr) {
+	case scrub_addr_base:
+		return addr_base_write(rasf_ctx, val);
+	case scrub_addr_size:
+		return addr_size_write(rasf_ctx, val);
+	case scrub_enable:
+		return enable_write(rasf_ctx, val);
+	case scrub_speed:
+		return speed_write(rasf_ctx, val);
+	default:
+		return -ENOTSUPP;
+	}
+}
+
+/**
+ * rasf_hw_scrub_read_strings() - Read callback for string attributes
+ * @device: Pointer to scrub device
+ * @attr: Scrub attribute
+ * @region_id: ID of the memory region
+ * @buf: Pointer to the buffer for copying returned string
+ *
+ * Returns: 0 on success, an error otherwise
+ */
+int rasf_hw_scrub_read_strings(struct device *device, u32 attr, int region_id,
+			       char *buf)
+{
+	struct rasf_context *rasf_ctx;
+
+	rasf_ctx = dev_get_drvdata(device);
+
+	switch (attr) {
+	case scrub_speed_available:
+		return speed_available_read(rasf_ctx, buf);
+	default:
+		return -ENOTSUPP;
+	}
+}
diff --git a/include/memory/rasf.h b/include/memory/rasf.h
new file mode 100755
index 000000000000..3ec291ddff97
--- /dev/null
+++ b/include/memory/rasf.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/*
+ * Memory RASF driver header file
+ *
+ * Copyright (c) 2023 HiSilicon Limited
+ */
+
+#ifndef _RASF_H
+#define _RASF_H
+
+#include <memory/memory-scrub.h>
+
+#define RASF_MAX_NAME_LENGTH      64
+#define RASF_MAX_SPEED_RANGE_LENGTH      64
+
+/*
+ * Data structures RASF
+ */
+
+/**
+ * struct rasf_scrub_params- RASF scrub parameter data structure.
+ * @addr_base:	[IN] Base address of the address range to be patrol scrubbed.
+ *		[OUT] Base address of the actual address range.
+ * @addr_size:	[IN] Size of the address range to be patrol scrubbed.
+ *		[OUT] Size of the actual address range.
+ * @flags:	[OUT] The platform returns this value in response to
+ *		GET_PATROL_PARAMETERS.
+ *		For RASF and RAS2:
+ *		Bit [0]: Will be set if memory scrubber is already
+ *		running for address range specified in “Actual Address Range”.
+ *		For RASF:
+ *		Bits [3:1]: Current Patrol Speeds, if Bit [0] is set.
+ * @speed:	[IN] Requested patrol Speed.
+ *		[OUT] Current patrol scrub Speed.
+ * @speed_avail:[OUT] Supported patrol speeds.
+ */
+struct rasf_scrub_params {
+	u64 addr_base;
+	u64 addr_size;
+	u16 flags;
+	u32 speed;
+	char speed_avail[RASF_MAX_SPEED_RANGE_LENGTH];
+};
+
+enum {
+	RASF_MEM_SCRUB_PARAM_ADDR_BASE = 0,
+	RASF_MEM_SCRUB_PARAM_ADDR_SIZE,
+	RASF_MEM_SCRUB_PARAM_SPEED,
+};
+
+/**
+ * struct rasf_hw_scrub_ops - rasf hw scrub device operations
+ * @enable_scrub: Function to enable/disable RASF/RAS2 scrubber. Mandatory.
+ *		Parameters are:
+ *		@rasf_ctx: Pointer to RASF/RAS2 context structure.
+ *		@enable: enable/disable RASF scrubber.
+ *		The function returns 0 on success or a negative error number.
+ * @get_scrub_params:	Read scrubber parameters. Mandatory
+ *		Parameters are:
+ *		@rasf_ctx: Pointer to RASF/RAS2 context structure.
+ *		@params: Pointer to scrub params data structure.
+ *		The function returns 0 on success or a negative error number.
+ * @set_scrub_params: Set scrubber parameters. Mandatory.
+ *		Parameters are:
+ *		@rasf_ctx: Pointer to RASF/RAS2 context structure.
+ *		@params: Pointer to scrub params data structure.
+ *		@param_type: Scrub parameter type to set.
+ *		The function returns 0 on success or a negative error number.
+ */
+struct rasf_hw_scrub_ops {
+	int (*enable_scrub)(struct rasf_context *rasf_ctx, bool enable);
+	int (*get_scrub_params)(struct rasf_context *rasf_ctx,
+				struct rasf_scrub_params *params);
+	int (*set_scrub_params)(struct rasf_context *rasf_ctx,
+				struct rasf_scrub_params *params, u8 param_type);
+};
+
+umode_t rasf_hw_scrub_is_visible(const void *drv_data, u32 attr, int region_id);
+int rasf_hw_scrub_read(struct device *dev, u32 attr, int region_id, u64 *val);
+int rasf_hw_scrub_write(struct device *dev, u32 attr, int region_id, u64 val);
+int rasf_hw_scrub_read_strings(struct device *dev, u32 attr, int region_id, char *buf);
+#endif /* _RASF_H */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 7/9] ACPICA: ACPI 6.5: Add support for RAS2 table
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (5 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 6/9] memory: RASF: Add memory RASF driver shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 8/9] ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2) shiju.jose
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Add support for ACPI RAS2 feature table(RAS2) defined in the ACPI 6.5
Specification & upwards revision, section 5.2.21.

The RAS2 table provides interfaces for platform RAS features. RAS2 offers
the same services as RASF, but is more scalable than the latter.
RAS2 supports independent RAS controls and capabilities for a given RAS
feature for multiple instances of the same component in a given system.
The platform can support either RAS2 or RASF but not both.

Link: https://github.com/acpica/acpica/pull/892

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 include/acpi/actbl2.h | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 3751ae69432f..41e77cacc5c6 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -47,6 +47,7 @@
 #define ACPI_SIG_PPTT           "PPTT"	/* Processor Properties Topology Table */
 #define ACPI_SIG_PRMT           "PRMT"	/* Platform Runtime Mechanism Table */
 #define ACPI_SIG_RASF           "RASF"	/* RAS Feature table */
+#define ACPI_SIG_RAS2           "RAS2"	/* RAS2 Feature table */
 #define ACPI_SIG_RGRT           "RGRT"	/* Regulatory Graphics Resource Table */
 #define ACPI_SIG_RHCT           "RHCT"	/* RISC-V Hart Capabilities Table */
 #define ACPI_SIG_SBST           "SBST"	/* Smart Battery Specification Table */
@@ -2743,6 +2744,60 @@ enum acpi_rasf_status {
 #define ACPI_RASF_ERROR                 (1<<2)
 #define ACPI_RASF_STATUS                (0x1F<<3)
 
+/*******************************************************************************
+ *
+ * RAS2 - RAS2 Feature Table (ACPI 6.5)
+ *        Version 2
+ *
+ *
+ ******************************************************************************/
+
+struct acpi_table_ras2 {
+	struct acpi_table_header header;        /* Common ACPI table header */
+	u16 reserved;
+	u16 num_pcc_descs;
+};
+
+/*
+ * RAS2 Platform Communication Channel Descriptor
+ */
+
+struct acpi_ras2_pcc_desc {
+	u8 channel_id;
+	u16 reserved;
+	u8 feature_type;
+	u32 instance;
+};
+
+/*
+ * RAS2 Platform Communication Channel Shared Memory Region
+ */
+
+struct acpi_ras2_shared_memory {
+	u32 signature;
+	u16 command;
+	u16 status;
+	u16 version;
+	u8 features[16];
+	u8 set_capabilities[16];
+	u16 num_parameter_blocks;
+	u32 set_capabilities_status;
+};
+
+/*
+ * RAS2 Parameter Block Structure for PATROL_SCRUB
+ */
+
+struct acpi_ras2_patrol_scrub_parameter {
+	struct acpi_rasf_parameter_block header;
+	u16 patrol_scrub_command;
+	u64 requested_address_range[2];
+	u64 actual_address_range[2];
+	u32 flags;
+	u32 scrub_params_out;
+	u32 scrub_params_in;
+};
+
 /*******************************************************************************
  *
  * RGRT - Regulatory Graphics Resource Table
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 8/9] ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2)
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (6 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 7/9] ACPICA: ACPI 6.5: Add support for RAS2 table shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-15 17:28 ` [RFC PATCH 9/9] memory: RAS2: Add memory RAS2 driver shiju.jose
  2023-09-17 21:14 ` [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers Jiaqi Yan
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Add support for ACPI RAS2 feature table (RAS2) defined in the ACPI 6.5
Specification, section 5.2.21.
This driver contains RAS2 Init, which extracts the RAS2 table.
Driver adds platform device, for each memory feature, which binds
to the RAS2 memory driver.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/acpi/Makefile    |  2 +-
 drivers/acpi/ras2_acpi.c | 97 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100755 drivers/acpi/ras2_acpi.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index dd62d936cbe1..4fbe7ea0be27 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -104,7 +104,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
 obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
 obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
 obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
-obj-$(CONFIG_ACPI_RASF)		+= rasf_acpi_common.o rasf_acpi.o
+obj-$(CONFIG_ACPI_RASF)		+= rasf_acpi_common.o rasf_acpi.o ras2_acpi.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
 obj-$(CONFIG_ACPI_PPTT) 	+= pptt.o
 obj-$(CONFIG_ACPI_PFRUT)	+= pfr_update.o pfr_telemetry.o
diff --git a/drivers/acpi/ras2_acpi.c b/drivers/acpi/ras2_acpi.c
new file mode 100755
index 000000000000..b8a7740355a8
--- /dev/null
+++ b/drivers/acpi/ras2_acpi.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ras2_acpi.c - Implementation of ACPI RAS2 feature table processing
+ * functions.
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ *
+ * Support for
+ * RAS2 - ACPI 6.5 Specification, section 5.2.21
+ *
+ * Driver contains RAS2 init, which extracts the RAS2 table and
+ * registers the PCC channel for communicating with the ACPI compliant
+ * platform that contains RAS2 command support in hardware.Driver adds
+ * platform device which binds to the RAS2 memory driver.
+ */
+
+#define pr_fmt(fmt)	"ACPI RAS2: " fmt
+
+#include <linux/export.h>
+#include <linux/delay.h>
+#include <linux/ktime.h>
+#include <linux/platform_device.h>
+#include <acpi/rasf_acpi.h>
+#include <acpi/acpixf.h>
+
+#define RAS2_FEATURE_TYPE_MEMORY        0x00
+
+int __init ras2_acpi_init(void)
+{
+	u8 count;
+	acpi_status status;
+	acpi_size ras2_size;
+	int pcc_subspace_idx;
+	struct platform_device *pdev;
+	struct acpi_table_ras2 *pRas2Table;
+	struct acpi_ras2_pcc_desc *pcc_desc_list;
+	struct platform_device **pdev_list = NULL;
+	struct acpi_table_header *pAcpiTable = NULL;
+
+	status = acpi_get_table("RAS2", 0, &pAcpiTable);
+	if (ACPI_FAILURE(status) || !pAcpiTable) {
+		pr_err("ACPI RAS2 driver failed to initialize, get table failed\n");
+		return RASF_FAILURE;
+	}
+
+	ras2_size = pAcpiTable->length;
+	if (ras2_size < sizeof(struct acpi_table_ras2)) {
+		pr_err("ACPI RAS2 table present but broken (too short #1)\n");
+		goto free_ras2_table;
+	}
+
+	pRas2Table = (struct acpi_table_ras2 *)pAcpiTable;
+
+	if (pRas2Table->num_pcc_descs <= 0) {
+		pr_err("ACPI RAS2 table does not contain PCC descriptors\n");
+		goto free_ras2_table;
+	}
+
+	pdev_list = kzalloc((pRas2Table->num_pcc_descs * sizeof(struct platform_device *)),
+			     GFP_KERNEL);
+	if (!pdev_list)
+		goto free_ras2_table;
+
+	pcc_desc_list = (struct acpi_ras2_pcc_desc *)
+				((void *)pRas2Table + sizeof(struct acpi_table_ras2));
+	count = 0;
+	while (count < pRas2Table->num_pcc_descs) {
+		if (pcc_desc_list->feature_type == RAS2_FEATURE_TYPE_MEMORY) {
+			pcc_subspace_idx = pcc_desc_list->channel_id;
+			/* Add the platform device and bind ras2 memory driver */
+			pdev = rasf_add_platform_device("ras2", &pcc_subspace_idx,
+							sizeof(pcc_subspace_idx));
+			if (!pdev)
+				goto free_ras2_pdev;
+			pdev_list[count] = pdev;
+		}
+		count++;
+		pcc_desc_list = pcc_desc_list + sizeof(struct acpi_ras2_pcc_desc);
+	}
+
+	acpi_put_table(pAcpiTable);
+	return RASF_SUCCESS;
+
+free_ras2_pdev:
+	count = 0;
+	while (count < pRas2Table->num_pcc_descs) {
+		if (pcc_desc_list->feature_type ==
+				RAS2_FEATURE_TYPE_MEMORY)
+			platform_device_put(pdev_list[count++]);
+	}
+	kfree(pdev_list);
+
+free_ras2_table:
+	acpi_put_table(pAcpiTable);
+	return RASF_FAILURE;
+}
+late_initcall(ras2_acpi_init)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 9/9] memory: RAS2: Add memory RAS2 driver
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (7 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 8/9] ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2) shiju.jose
@ 2023-09-15 17:28 ` shiju.jose
  2023-09-17 21:14 ` [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers Jiaqi Yan
  9 siblings, 0 replies; 32+ messages in thread
From: shiju.jose @ 2023-09-15 17:28 UTC (permalink / raw)
  To: linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, david, jiaqiyan, jthoughton, somasundaram.a,
	erdemaktas, pgonda, rientjes, duenwen, Vilas.Sridharan,
	mike.malvestuto, gthelen, linuxarm, jonathan.cameron, tanxiaofei,
	prime.zeng, shiju.jose

From: Shiju Jose <shiju.jose@huawei.com>

Memory RAS2 driver binds to the platform device add by the ACPI RAS2
driver.
Driver registers the PCC channel for communicating with the ACPI compliant
platform that contains RAS2 command support in the hardware.

Add interface functions to support configuring the parameters of HW patrol
scrubber in the system, which exposed to the kernel via the RAS2 and PCC,
using the RAS2 commands.

Add support for RAS2 platform devices to register with scrub config driver.
This enables user to configure the parameters of HW patrol scrubbers,
which exposed to the kernel via the RAS2 table, through the scrub sysfs
attributes.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/memory/Makefile |   2 +-
 drivers/memory/ras2.c   | 334 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 335 insertions(+), 1 deletion(-)
 create mode 100644 drivers/memory/ras2.c

diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 49340cd100fc..b4b8ff7c6926 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_DDR)		+= jedec_ddr_data.o
 ifeq ($(CONFIG_DDR),y)
 obj-$(CONFIG_OF)		+= of_memory.o
 endif
-obj-$(CONFIG_MEM_RASF)		+= rasf_common.o rasf.o
+obj-$(CONFIG_MEM_RASF)		+= rasf_common.o rasf.o ras2.o
 
 obj-$(CONFIG_ARM_PL172_MPMC)	+= pl172.o
 obj-$(CONFIG_ATMEL_EBI)		+= atmel-ebi.o
diff --git a/drivers/memory/ras2.c b/drivers/memory/ras2.c
new file mode 100644
index 000000000000..80da996bf12e
--- /dev/null
+++ b/drivers/memory/ras2.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * ras2.c - ACPI RAS2 memory driver
+ *
+ * Copyright (c) 2023 HiSilicon Limited.
+ *
+ *  - Registers the PCC channel for communicating with the
+ *    ACPI compliant platform that contains RAS2 command
+ *    support in the hardware.
+ *  - Provides functions to configure HW patrol scrubber
+ *    in the system.
+ *  - Registers with the scrub configure driver for the
+ *    hw patrol scrubber in the system, which exposed via
+ *    the ACPI RAS2 table and PCC.
+ */
+
+#define pr_fmt(fmt)	"MEMORY RAS2: " fmt
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/cleanup.h>
+
+#include <acpi/rasf_acpi.h>
+#include <memory/rasf.h>
+
+/* RAS2 specific definitions. */
+#define RAS2_SCRUB	"ras2_scrub"
+#define RAS2_ID_FORMAT RAS2_SCRUB "%d"
+#define RAS2_SUPPORT_HW_PARTOL_SCRUB	BIT(0)
+#define RAS2_TYPE_PATROL_SCRUB	0x0000
+
+#define RAS2_GET_PATROL_PARAMETERS	0x01
+#define	RAS2_START_PATROL_SCRUBBER	0x02
+#define	RAS2_STOP_PATROL_SCRUBBER	0x03
+
+#define RAS2_PATROL_SCRUB_RATE_VALID	BIT(0)
+#define RAS2_PATROL_SCRUB_RATE_IN_MASK	GENMASK(15, 8)
+#define RAS2_PATROL_SCRUB_EN_BACKGROUND	BIT(0)
+#define RAS2_PATROL_SCRUB_RATE_OUT_MASK	GENMASK(7, 0)
+#define RAS2_PATROL_SCRUB_MIN_RATE_OUT_MASK	GENMASK(15, 8)
+#define RAS2_PATROL_SCRUB_MAX_RATE_OUT_MASK	GENMASK(23, 16)
+
+/* The default number of regions for RAS2 */
+#define RAS2_NUM_REGIONS	1
+
+static void ras2_tx_done(struct mbox_client *cl, void *msg, int ret)
+{
+	if (ret) {
+		dev_dbg(cl->dev, "TX did not complete: CMD sent:%x, ret:%d\n",
+			*(u16 *)msg, ret);
+	} else {
+		dev_dbg(cl->dev, "TX completed. CMD sent:%x, ret:%d\n",
+			*(u16 *)msg, ret);
+	}
+}
+
+/*
+ * The below functions are exposed to OSPM, to query, configure and
+ * initiate memory patrol scrubber.
+ */
+static int ras2_is_patrol_scrub_support(struct rasf_context *ras2_ctx)
+{
+	int ret;
+	struct acpi_ras2_shared_memory  __iomem *generic_comm_base;
+
+	if (!ras2_ctx || !ras2_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = ras2_ctx->pcc_comm_addr;
+	guard(spinlock_irqsave)(&ras2_ctx->spinlock);
+	generic_comm_base->set_capabilities[0] = 0;
+
+	/* send command for reading RAS2 capabilities */
+	ret = rasf_send_pcc_cmd(ras2_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: rasf_send_pcc_cmd failed\n", __func__);
+		return ret;
+	}
+
+	return generic_comm_base->features[0] & RAS2_SUPPORT_HW_PARTOL_SCRUB;
+}
+
+static int ras2_get_patrol_scrub_params(struct rasf_context *ras2_ctx,
+					struct rasf_scrub_params *params)
+{
+	int ret = 0;
+	u8  min_supp_scrub_rate, max_supp_scrub_rate;
+	struct acpi_ras2_shared_memory  __iomem *generic_comm_base;
+	struct acpi_ras2_patrol_scrub_parameter __iomem *patrol_scrub_params;
+
+	if (!ras2_ctx || !ras2_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = ras2_ctx->pcc_comm_addr;
+	patrol_scrub_params = ras2_ctx->pcc_comm_addr + sizeof(*generic_comm_base);
+
+	guard(spinlock_irqsave)(&ras2_ctx->spinlock);
+	generic_comm_base->set_capabilities[0] = RAS2_SUPPORT_HW_PARTOL_SCRUB;
+	/* send command for reading RASF capabilities */
+	ret = rasf_send_pcc_cmd(ras2_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: rasf_send_pcc_cmd failed\n", __func__);
+		return ret;
+	}
+
+	if (!(generic_comm_base->features[0] & RAS2_SUPPORT_HW_PARTOL_SCRUB) ||
+	    !(generic_comm_base->num_parameter_blocks)) {
+		pr_err("%s: Platform does not support HW Patrol Scrubber\n", __func__);
+		return -ENOTSUPP;
+	}
+
+	if (!patrol_scrub_params->requested_address_range[1]) {
+		pr_err("%s: Invalid requested address range, \
+			requested_address_range[0]=0x%llx \
+			requested_address_range[1]=0x%llx\n",
+			__func__,
+			patrol_scrub_params->requested_address_range[0],
+			patrol_scrub_params->requested_address_range[1]);
+		return -ENOTSUPP;
+	}
+
+	generic_comm_base->set_capabilities[0] = RAS2_SUPPORT_HW_PARTOL_SCRUB;
+	patrol_scrub_params->header.type = RAS2_TYPE_PATROL_SCRUB;
+	patrol_scrub_params->patrol_scrub_command = RAS2_GET_PATROL_PARAMETERS;
+
+	/* send command for reading the HW patrol scrub parameters */
+	ret = rasf_send_pcc_cmd(ras2_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: failed to read HW patrol scrub parameters\n", __func__);
+		return ret;
+	}
+
+	/* copy output scrub parameters */
+	params->addr_base = patrol_scrub_params->actual_address_range[0];
+	params->addr_size = patrol_scrub_params->actual_address_range[1];
+	params->flags = patrol_scrub_params->flags;
+	if (patrol_scrub_params->flags & RAS2_PATROL_SCRUB_RATE_VALID) {
+		params->speed = FIELD_GET(RAS2_PATROL_SCRUB_RATE_OUT_MASK,
+					  patrol_scrub_params->scrub_params_out);
+		min_supp_scrub_rate = FIELD_GET(RAS2_PATROL_SCRUB_MIN_RATE_OUT_MASK,
+						patrol_scrub_params->scrub_params_out);
+		max_supp_scrub_rate = FIELD_GET(RAS2_PATROL_SCRUB_MAX_RATE_OUT_MASK,
+						patrol_scrub_params->scrub_params_out);
+		snprintf(params->speed_avail, RASF_MAX_SPEED_RANGE_LENGTH,
+			"%d-%d", min_supp_scrub_rate, max_supp_scrub_rate);
+	} else {
+		params->speed = 0;
+		snprintf(params->speed_avail, RASF_MAX_SPEED_RANGE_LENGTH,
+			"%s", "Unavailable");
+	}
+
+	return 0;
+}
+
+static int ras2_enable_patrol_scrub(struct rasf_context *ras2_ctx, bool enable)
+{
+	int ret = 0;
+	struct rasf_scrub_params params;
+	struct acpi_ras2_shared_memory  __iomem *generic_comm_base;
+	u8 scrub_rate_to_set, min_supp_scrub_rate, max_supp_scrub_rate;
+	struct acpi_ras2_patrol_scrub_parameter __iomem *patrol_scrub_params;
+
+	if (!ras2_ctx || !ras2_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = ras2_ctx->pcc_comm_addr;
+	patrol_scrub_params = ras2_ctx->pcc_comm_addr + sizeof(*generic_comm_base);
+
+	if (enable) {
+		ret = ras2_get_patrol_scrub_params(ras2_ctx, &params);
+		if (ret)
+			return ret;
+	}
+
+	guard(spinlock_irqsave)(&ras2_ctx->spinlock);
+	generic_comm_base->set_capabilities[0] = RAS2_SUPPORT_HW_PARTOL_SCRUB;
+	patrol_scrub_params->header.type = RAS2_TYPE_PATROL_SCRUB;
+
+	if (enable) {
+		patrol_scrub_params->patrol_scrub_command = RAS2_START_PATROL_SCRUBBER;
+		patrol_scrub_params->requested_address_range[0] = params.addr_base;
+		patrol_scrub_params->requested_address_range[1] = params.addr_size;
+
+		if (patrol_scrub_params->flags & RAS2_PATROL_SCRUB_RATE_VALID) {
+			scrub_rate_to_set = FIELD_GET(RAS2_PATROL_SCRUB_RATE_IN_MASK,
+						      patrol_scrub_params->scrub_params_in);
+			min_supp_scrub_rate = FIELD_GET(RAS2_PATROL_SCRUB_MIN_RATE_OUT_MASK,
+							patrol_scrub_params->scrub_params_out);
+			max_supp_scrub_rate = FIELD_GET(RAS2_PATROL_SCRUB_MAX_RATE_OUT_MASK,
+							patrol_scrub_params->scrub_params_out);
+			if (scrub_rate_to_set < min_supp_scrub_rate ||
+			    scrub_rate_to_set > max_supp_scrub_rate) {
+				pr_warn("patrol scrub rate to set is out of the supported range\n");
+				pr_warn("min_supp_scrub_rate=%d max_supp_scrub_rate=%d\n",
+					min_supp_scrub_rate, max_supp_scrub_rate);
+				return -EINVAL;
+			}
+		}
+	} else
+		patrol_scrub_params->patrol_scrub_command = RAS2_STOP_PATROL_SCRUBBER;
+
+	/* send command for enable/disable HW patrol scrub */
+	ret = rasf_send_pcc_cmd(ras2_ctx, RASF_PCC_CMD_EXEC);
+	if (ret) {
+		pr_err("%s: failed to enable/disable the HW patrol scrub\n", __func__);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ras2_set_patrol_scrub_params(struct rasf_context *ras2_ctx,
+					struct rasf_scrub_params *params, u8 param_type)
+{
+	struct acpi_ras2_shared_memory  __iomem *generic_comm_base;
+	struct acpi_ras2_patrol_scrub_parameter __iomem *patrol_scrub_params;
+
+	if (!ras2_ctx || !ras2_ctx->pcc_comm_addr)
+		return -EFAULT;
+
+	generic_comm_base = ras2_ctx->pcc_comm_addr;
+	patrol_scrub_params = ras2_ctx->pcc_comm_addr + sizeof(*generic_comm_base);
+
+	guard(spinlock_irqsave)(&ras2_ctx->spinlock);
+	patrol_scrub_params->header.type = RAS2_TYPE_PATROL_SCRUB;
+	if (param_type == RASF_MEM_SCRUB_PARAM_ADDR_BASE && params->addr_base) {
+		patrol_scrub_params->requested_address_range[0] = params->addr_base;
+	} else if (param_type == RASF_MEM_SCRUB_PARAM_ADDR_SIZE && params->addr_size) {
+		patrol_scrub_params->requested_address_range[1] = params->addr_size;
+	} else if (param_type == RASF_MEM_SCRUB_PARAM_SPEED) {
+		patrol_scrub_params->scrub_params_in &= ~RAS2_PATROL_SCRUB_RATE_IN_MASK;
+		patrol_scrub_params->scrub_params_in |= FIELD_PREP(RAS2_PATROL_SCRUB_RATE_IN_MASK,
+								   params->speed);
+	} else {
+		pr_err("Invalid patrol scrub parameter to set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct rasf_hw_scrub_ops ras2_hw_ops = {
+	.enable_scrub = ras2_enable_patrol_scrub,
+	.get_scrub_params = ras2_get_patrol_scrub_params,
+	.set_scrub_params = ras2_set_patrol_scrub_params,
+};
+
+static const struct scrub_ops ras2_scrub_ops = {
+	.is_visible = rasf_hw_scrub_is_visible,
+	.read = rasf_hw_scrub_read,
+	.write = rasf_hw_scrub_write,
+	.read_string = rasf_hw_scrub_read_strings,
+};
+
+static DEFINE_IDA(ras2_ida);
+
+static void devm_ras2_release(void *ctx)
+{
+	struct rasf_context *ras2_ctx = ctx;
+
+	ida_free(&ras2_ida, ras2_ctx->id);
+	rasf_unregister_pcc_channel(ras2_ctx);
+}
+
+static int ras2_probe(struct platform_device *pdev)
+{
+	int ret, id;
+	struct mbox_client *cl;
+	struct device *hw_scrub_dev;
+	struct rasf_context *ras2_ctx;
+	char scrub_name[RASF_MAX_NAME_LENGTH];
+
+	ras2_ctx = devm_kzalloc(&pdev->dev, sizeof(*ras2_ctx), GFP_KERNEL);
+	if (!ras2_ctx)
+		return -ENOMEM;
+
+	ras2_ctx->dev = &pdev->dev;
+	ras2_ctx->ops = &ras2_hw_ops;
+	spin_lock_init(&ras2_ctx->spinlock);
+	platform_set_drvdata(pdev, ras2_ctx);
+
+	cl = &ras2_ctx->mbox_client;
+	/* Request mailbox channel */
+	cl->dev = &pdev->dev;
+	cl->tx_done = ras2_tx_done;
+	cl->knows_txdone = true;
+	ras2_ctx->pcc_subspace_idx = *((int *)pdev->dev.platform_data);
+	dev_dbg(&pdev->dev, "pcc-subspace-id=%d\n", ras2_ctx->pcc_subspace_idx);
+	ret = rasf_register_pcc_channel(ras2_ctx);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, devm_ras2_release, ras2_ctx);
+	if (ret < 0)
+		return ret;
+
+	if (ras2_is_patrol_scrub_support(ras2_ctx)) {
+		id = ida_alloc(&ras2_ida, GFP_KERNEL);
+		if (id < 0)
+			return id;
+		ras2_ctx->id = id;
+		ras2_ctx->n_regions = RAS2_NUM_REGIONS;
+		snprintf(scrub_name, sizeof(scrub_name), "%s%d", RAS2_SCRUB, id);
+		dev_set_name(&pdev->dev, RAS2_ID_FORMAT, id);
+		hw_scrub_dev = devm_scrub_device_register(&pdev->dev, scrub_name,
+							  ras2_ctx, &ras2_scrub_ops,
+							  ras2_ctx->n_regions);
+		if (PTR_ERR_OR_ZERO(hw_scrub_dev))
+			return PTR_ERR_OR_ZERO(hw_scrub_dev);
+	}
+	ras2_ctx->scrub_dev = hw_scrub_dev;
+
+	return 0;
+}
+
+static const struct platform_device_id ras2_id_table[] = {
+	{ .name = "ras2", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, ras2_id_table);
+
+static struct platform_driver ras2_driver = {
+	.probe = ras2_probe,
+	.driver = {
+		.name = "ras2",
+		.suppress_bind_attrs = true,
+	},
+	.id_table = ras2_id_table,
+};
+module_driver(ras2_driver, platform_driver_register, platform_driver_unregister);
+
+MODULE_DESCRIPTION("ras2 memory driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers
  2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
                   ` (8 preceding siblings ...)
  2023-09-15 17:28 ` [RFC PATCH 9/9] memory: RAS2: Add memory RAS2 driver shiju.jose
@ 2023-09-17 21:14 ` Jiaqi Yan
  2023-09-18 10:19   ` Shiju Jose
  9 siblings, 1 reply; 32+ messages in thread
From: Jiaqi Yan @ 2023-09-17 21:14 UTC (permalink / raw)
  To: shiju.jose
  Cc: linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, david,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen, linuxarm,
	jonathan.cameron, tanxiaofei, prime.zeng

On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
>
> From: Shiju Jose <shiju.jose@huawei.com>
>
> This series add,
> 1. support for ACPI RASF(RAS feature table) PCC interfaces
> to communicate with the HW patrol scrubber in the platform,
> as per ACPI 5.1 & upwards revision. Section 5.2.20.
>
> 2. support for ACPI RAS2(RAS2 feature table), as per
> ACPI 6.5 & upwards revision. Section 5.2.21.
>
> 3. scrub driver supports configuring parameters of the memory
> scrubbers in the system. This driver has been implemented
> based on the hwmon subsystem.
>
> The features have tested with RASF and RAS2 emulation in the QEMU.

I am very curious how the test is done. Does the hw patrol scrubber on
host actually been driven by the driver to scrub memory DIMMs (doesn't
seem so to me, but do correct me)? Or it is like to a VM scrubbing is
simulated and no real op to DIMMs?

>
> Previous references to the memory scub and RASF topics.
> https://lore.kernel.org/all/20221103155029.2451105-1-jiaqiyan@google.com/
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/CS1PR84MB0038718F49DBC0FF03919E1184390@CS1PR84MB0038.NAMPRD84.PROD.OUTLOOK.COM/
>
> A Somasundaram (2):
>   ACPI:RASF: Add extract RASF table to register RASF platform devices
>   ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces
>
> Shiju Jose (7):
>   memory: scrub: Add scrub driver supports configuring memory scrubbers
>     in the system
>   memory: scrub: sysfs: Add Documentation entries for set of scrub
>     attributes
>   Documentation/scrub-configure.rst: Add documentation for scrub driver
>   memory: RASF: Add memory RASF driver
>   ACPICA: ACPI 6.5: Add support for RAS2 table
>   ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2)
>   memory: RAS2: Add memory RAS2 driver
>
>  .../ABI/testing/sysfs-class-scrub-configure   |  82 ++++
>  Documentation/scrub-configure.rst             |  55 +++
>  drivers/acpi/Kconfig                          |  15 +
>  drivers/acpi/Makefile                         |   1 +
>  drivers/acpi/ras2_acpi.c                      |  97 ++++
>  drivers/acpi/rasf_acpi.c                      |  71 +++
>  drivers/acpi/rasf_acpi_common.c               | 272 +++++++++++
>  drivers/memory/Kconfig                        |  15 +
>  drivers/memory/Makefile                       |   3 +
>  drivers/memory/ras2.c                         | 334 +++++++++++++
>  drivers/memory/rasf.c                         | 335 +++++++++++++
>  drivers/memory/rasf_common.c                  | 251 ++++++++++
>  drivers/memory/scrub/Kconfig                  |  11 +
>  drivers/memory/scrub/Makefile                 |   6 +
>  drivers/memory/scrub/memory-scrub.c           | 452 ++++++++++++++++++
>  include/acpi/actbl2.h                         |  55 +++
>  include/acpi/rasf_acpi.h                      |  59 +++
>  include/memory/memory-scrub.h                 |  85 ++++
>  include/memory/rasf.h                         |  82 ++++
>  19 files changed, 2281 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure
>  create mode 100644 Documentation/scrub-configure.rst
>  create mode 100755 drivers/acpi/ras2_acpi.c
>  create mode 100755 drivers/acpi/rasf_acpi.c
>  create mode 100755 drivers/acpi/rasf_acpi_common.c
>  create mode 100644 drivers/memory/ras2.c
>  create mode 100644 drivers/memory/rasf.c
>  create mode 100644 drivers/memory/rasf_common.c
>  create mode 100644 drivers/memory/scrub/Kconfig
>  create mode 100644 drivers/memory/scrub/Makefile
>  create mode 100755 drivers/memory/scrub/memory-scrub.c
>  create mode 100755 include/acpi/rasf_acpi.h
>  create mode 100755 include/memory/memory-scrub.h
>  create mode 100755 include/memory/rasf.h
>
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-15 17:28 ` [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver shiju.jose
@ 2023-09-18  7:23   ` David Hildenbrand
  2023-09-18 10:25     ` Shiju Jose
  0 siblings, 1 reply; 32+ messages in thread
From: David Hildenbrand @ 2023-09-18  7:23 UTC (permalink / raw)
  To: shiju.jose, linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, jiaqiyan, jthoughton, somasundaram.a, erdemaktas,
	pgonda, rientjes, duenwen, Vilas.Sridharan, mike.malvestuto,
	gthelen, linuxarm, jonathan.cameron, tanxiaofei, prime.zeng

On 15.09.23 19:28, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> Add documentation for scrub driver, supports configure scrub parameters,
> in Documentation/scrub-configure.rst
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
>   Documentation/scrub-configure.rst | 55 +++++++++++++++++++++++++++++++
>   1 file changed, 55 insertions(+)
>   create mode 100644 Documentation/scrub-configure.rst
> 
> diff --git a/Documentation/scrub-configure.rst b/Documentation/scrub-configure.rst
> new file mode 100644
> index 000000000000..9f8581b88788
> --- /dev/null
> +++ b/Documentation/scrub-configure.rst
> @@ -0,0 +1,55 @@
> +==========================
> +Scrub subsystem driver
> +==========================
> +
> +Copyright (c) 2023 HiSilicon Limited.
> +
> +:Author:   Shiju Jose <shiju.jose@huawei.com>
> +:License:  The GNU Free Documentation License, Version 1.2
> +          (dual licensed under the GPL v2)
> +:Original Reviewers:
> +
> +- Written for: 6.7
> +- Updated for:
> +
> +Introduction
> +------------
> +The scrub subsystem driver provides the interface for configure the

"... interface for configuring memory scrubbers in the system."

are we only configuring firmware/hw-based memory scrubbing? I assume so.

> +parameters of memory scrubbers in the system. The scrub device drivers
> +in the system register with the scrub configure subsystem.

Maybe say a few words what memory scrubbing is, and what it is used for.

> +
> +The scrub configure driver exposes the scrub controls to the user
> +via sysfs.
> +
> +The File System
> +---------------
> +
> +The configuration parameters of the registered scrubbers could be
> +accessed via the /sys/class/scrub/scrubX/regionN/
> +
> +sysfs
> +-----
> +
> +Sysfs files are documented in
> +`Documentation/ABI/testing/sysfs-class-scrub-configure`.
> +
> +Example
> +-------
> +
> +  The usage takes the form shown in this example::
> +
> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
> +    # cat /sys/class/scrub/scrub0/region0/speed_available
> +    # 1-60
> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
> +
> +    # cat /sys/class/scrub/scrub0/region0/speed
> +    # 0x19

Is it reasonable to return the speed as hex? You set it as dec.

> +    # cat /sys/class/scrub/scrub0/region0/addr_base
> +    # 0x100000

But didn't we set it to 0x300000 ...

> +    # cat /sys/class/scrub/scrub0/region0/addr_size
> +    # 0x200000

... and didn't we set it to 0x100000 ?

Or what's the magic happening here?

> +
> +    # echo 0 > /sys/class/scrub/scrub0/region0/enable

-- 
Cheers,

David / dhildenb


^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers
  2023-09-17 21:14 ` [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers Jiaqi Yan
@ 2023-09-18 10:19   ` Shiju Jose
  2023-09-18 17:47     ` Jiaqi Yan
  0 siblings, 1 reply; 32+ messages in thread
From: Shiju Jose @ 2023-09-18 10:19 UTC (permalink / raw)
  To: Jiaqi Yan
  Cc: linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, david,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen, Linuxarm,
	Jonathan Cameron, tanxiaofei, Zengtao (B),
	bp, mchehab, rric, linux-edac

[+cc linux-edac@vger.kernel.org]
 
Hello, 

>-----Original Message-----
>From: Jiaqi Yan <jiaqiyan@google.com>
>Sent: 17 September 2023 22:14
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-
>kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
>naoya.horiguchi@nec.com; tony.luck@intel.com; james.morse@arm.com;
>dave.hansen@linux.intel.com; david@redhat.com; jthoughton@google.com;
>somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com;
>rientjes@google.com; duenwen@google.com; Vilas.Sridharan@amd.com;
>mike.malvestuto@intel.com; gthelen@google.com; Linuxarm
><linuxarm@huawei.com>; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2
>and configure scrubbers
>
>On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
>>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> This series add,
>> 1. support for ACPI RASF(RAS feature table) PCC interfaces to
>> communicate with the HW patrol scrubber in the platform, as per ACPI
>> 5.1 & upwards revision. Section 5.2.20.
>>
>> 2. support for ACPI RAS2(RAS2 feature table), as per ACPI 6.5 &
>> upwards revision. Section 5.2.21.
>>
>> 3. scrub driver supports configuring parameters of the memory
>> scrubbers in the system. This driver has been implemented based on the
>> hwmon subsystem.
>>
>> The features have tested with RASF and RAS2 emulation in the QEMU.
>
>I am very curious how the test is done. Does the hw patrol scrubber on host
>actually been driven by the driver to scrub memory DIMMs (doesn't seem so to
>me, but do correct me)? Or it is like to a VM scrubbing is simulated and no real
>op to DIMMs?
Intent here is hardware scrubber on host as far as we are concerned. 
Could be used for VM too perhaps. We did it with QEMU emulation for now
 to get the flexibility of configuration. However there will be other scrub controls
over time, such as DDR5 ECS.
https://media-www.micron.com/-/media/client/global/documents/products/white-paper/ddr5_new_features_white_paper.pdf?rev=b98f4977d9334b4aa5d0d211a92bf14a

Also found there is very simple support for scrub control in edac, and an alternative path
would be to look at extending that to sufficient complexity to support region based scanning. 
https://elixir.bootlin.com/linux/latest/source/include/linux/edac.h#L512

>
>>
>> Previous references to the memory scub and RASF topics.
>> https://lore.kernel.org/all/20221103155029.2451105-1-jiaqiyan@google.c
>> om/
>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/CS1PR84MB0
>>
>038718F49DBC0FF03919E1184390@CS1PR84MB0038.NAMPRD84.PROD.OUTLO
>OK.COM/
>>
>> A Somasundaram (2):
>>   ACPI:RASF: Add extract RASF table to register RASF platform devices
>>   ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces
>>
>> Shiju Jose (7):
>>   memory: scrub: Add scrub driver supports configuring memory scrubbers
>>     in the system
>>   memory: scrub: sysfs: Add Documentation entries for set of scrub
>>     attributes
>>   Documentation/scrub-configure.rst: Add documentation for scrub driver
>>   memory: RASF: Add memory RASF driver
>>   ACPICA: ACPI 6.5: Add support for RAS2 table
>>   ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2)
>>   memory: RAS2: Add memory RAS2 driver
>>
>>  .../ABI/testing/sysfs-class-scrub-configure   |  82 ++++
>>  Documentation/scrub-configure.rst             |  55 +++
>>  drivers/acpi/Kconfig                          |  15 +
>>  drivers/acpi/Makefile                         |   1 +
>>  drivers/acpi/ras2_acpi.c                      |  97 ++++
>>  drivers/acpi/rasf_acpi.c                      |  71 +++
>>  drivers/acpi/rasf_acpi_common.c               | 272 +++++++++++
>>  drivers/memory/Kconfig                        |  15 +
>>  drivers/memory/Makefile                       |   3 +
>>  drivers/memory/ras2.c                         | 334 +++++++++++++
>>  drivers/memory/rasf.c                         | 335 +++++++++++++
>>  drivers/memory/rasf_common.c                  | 251 ++++++++++
>>  drivers/memory/scrub/Kconfig                  |  11 +
>>  drivers/memory/scrub/Makefile                 |   6 +
>>  drivers/memory/scrub/memory-scrub.c           | 452 ++++++++++++++++++
>>  include/acpi/actbl2.h                         |  55 +++
>>  include/acpi/rasf_acpi.h                      |  59 +++
>>  include/memory/memory-scrub.h                 |  85 ++++
>>  include/memory/rasf.h                         |  82 ++++
>>  19 files changed, 2281 insertions(+)
>>  create mode 100644
>> Documentation/ABI/testing/sysfs-class-scrub-configure
>>  create mode 100644 Documentation/scrub-configure.rst  create mode
>> 100755 drivers/acpi/ras2_acpi.c  create mode 100755
>> drivers/acpi/rasf_acpi.c  create mode 100755
>> drivers/acpi/rasf_acpi_common.c  create mode 100644
>> drivers/memory/ras2.c  create mode 100644 drivers/memory/rasf.c
>> create mode 100644 drivers/memory/rasf_common.c  create mode 100644
>> drivers/memory/scrub/Kconfig  create mode 100644
>> drivers/memory/scrub/Makefile  create mode 100755
>> drivers/memory/scrub/memory-scrub.c
>>  create mode 100755 include/acpi/rasf_acpi.h  create mode 100755
>> include/memory/memory-scrub.h  create mode 100755
>> include/memory/rasf.h
>>
>> --
>> 2.34.1
>>

Thanks,
Shiju

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-18  7:23   ` David Hildenbrand
@ 2023-09-18 10:25     ` Shiju Jose
  2023-09-18 12:15       ` David Hildenbrand
  0 siblings, 1 reply; 32+ messages in thread
From: Shiju Jose @ 2023-09-18 10:25 UTC (permalink / raw)
  To: David Hildenbrand, linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, jiaqiyan, jthoughton, somasundaram.a, erdemaktas,
	pgonda, rientjes, duenwen, Vilas.Sridharan, mike.malvestuto,
	gthelen, Linuxarm, Jonathan Cameron, tanxiaofei, Zengtao (B)

Hi David,

Thanks for looking into this.

>-----Original Message-----
>From: David Hildenbrand <david@redhat.com>
>Sent: 18 September 2023 08:24
>To: Shiju Jose <shiju.jose@huawei.com>; linux-acpi@vger.kernel.org; linux-
>mm@kvack.org; linux-kernel@vger.kernel.org
>Cc: rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com;
>tony.luck@intel.com; james.morse@arm.com; dave.hansen@linux.intel.com;
>jiaqiyan@google.com; jthoughton@google.com; somasundaram.a@hpe.com;
>erdemaktas@google.com; pgonda@google.com; rientjes@google.com;
>duenwen@google.com; Vilas.Sridharan@amd.com; mike.malvestuto@intel.com;
>gthelen@google.com; Linuxarm <linuxarm@huawei.com>; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>documentation for scrub driver
>
>On 15.09.23 19:28, shiju.jose@huawei.com wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Add documentation for scrub driver, supports configure scrub
>> parameters, in Documentation/scrub-configure.rst
>>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>> ---
>>   Documentation/scrub-configure.rst | 55
>+++++++++++++++++++++++++++++++
>>   1 file changed, 55 insertions(+)
>>   create mode 100644 Documentation/scrub-configure.rst
>>
>> diff --git a/Documentation/scrub-configure.rst
>> b/Documentation/scrub-configure.rst
>> new file mode 100644
>> index 000000000000..9f8581b88788
>> --- /dev/null
>> +++ b/Documentation/scrub-configure.rst
>> @@ -0,0 +1,55 @@
>> +==========================
>> +Scrub subsystem driver
>> +==========================
>> +
>> +Copyright (c) 2023 HiSilicon Limited.
>> +
>> +:Author:   Shiju Jose <shiju.jose@huawei.com>
>> +:License:  The GNU Free Documentation License, Version 1.2
>> +          (dual licensed under the GPL v2) :Original Reviewers:
>> +
>> +- Written for: 6.7
>> +- Updated for:
>> +
>> +Introduction
>> +------------
>> +The scrub subsystem driver provides the interface for configure the
>
>"... interface for configuring memory scrubbers in the system."
>
>are we only configuring firmware/hw-based memory scrubbing? I assume so.
The scrub control could be used for the SW  based memory scrubbing too.

>
>> +parameters of memory scrubbers in the system. The scrub device
>> +drivers in the system register with the scrub configure subsystem.
>
>Maybe say a few words what memory scrubbing is, and what it is used for.
Sure.

>
>> +
>> +The scrub configure driver exposes the scrub controls to the user via
>> +sysfs.
>> +
>> +The File System
>> +---------------
>> +
>> +The configuration parameters of the registered scrubbers could be
>> +accessed via the /sys/class/scrub/scrubX/regionN/
>> +
>> +sysfs
>> +-----
>> +
>> +Sysfs files are documented in
>> +`Documentation/ABI/testing/sysfs-class-scrub-configure`.
>> +
>> +Example
>> +-------
>> +
>> +  The usage takes the form shown in this example::
>> +
>> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
>> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
>> +    # cat /sys/class/scrub/scrub0/region0/speed_available
>> +    # 1-60
>> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
>> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
>> +
>> +    # cat /sys/class/scrub/scrub0/region0/speed
>> +    # 0x19
>
>Is it reasonable to return the speed as hex? You set it as dec.
Presently return speed  as hex to reduce the number of callback function needed
for reading the hex/dec data because the values for the address range
need to be in hex.

>
>> +    # cat /sys/class/scrub/scrub0/region0/addr_base
>> +    # 0x100000
>
>But didn't we set it to 0x300000 ...
This is an emulated example for testing the RASF/RAS2 definition.
According to the RASF & RAS2 definition, the actual address range in the
platform could vary from the requested address range for the patrol scrubbing.
"The platform calculates the nearest patrol scrub boundary address
from where it can start". The platform returns the actual address range
in response to GET_PATROL_PARAMETERS command to the firmware.
Please see section 5.2.21.2.1 Hardware-based Memory Scrubbing ,
Table 5.87: Parameter Block Structure for PATROL_SCRUB in the 
ACPI 6.5 specification.

>
>> +    # cat /sys/class/scrub/scrub0/region0/addr_size
>> +    # 0x200000
>
>... and didn't we set it to 0x100000 ?
>
>Or what's the magic happening here?
Same as above.

>
>> +
>> +    # echo 0 > /sys/class/scrub/scrub0/region0/enable
>
>--
>Cheers,
>
>David / dhildenb
>

Thanks,
Shiju

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-18 10:25     ` Shiju Jose
@ 2023-09-18 12:15       ` David Hildenbrand
  2023-09-18 12:28         ` Jonathan Cameron
  0 siblings, 1 reply; 32+ messages in thread
From: David Hildenbrand @ 2023-09-18 12:15 UTC (permalink / raw)
  To: Shiju Jose, linux-acpi, linux-mm, linux-kernel
  Cc: rafael, lenb, naoya.horiguchi, tony.luck, james.morse,
	dave.hansen, jiaqiyan, jthoughton, somasundaram.a, erdemaktas,
	pgonda, rientjes, duenwen, Vilas.Sridharan, mike.malvestuto,
	gthelen, Linuxarm, Jonathan Cameron, tanxiaofei, Zengtao (B)

On 18.09.23 12:25, Shiju Jose wrote:
> Hi David,
> 
> Thanks for looking into this.
> 
>> -----Original Message-----
>> From: David Hildenbrand <david@redhat.com>
>> Sent: 18 September 2023 08:24
>> To: Shiju Jose <shiju.jose@huawei.com>; linux-acpi@vger.kernel.org; linux-
>> mm@kvack.org; linux-kernel@vger.kernel.org
>> Cc: rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com;
>> tony.luck@intel.com; james.morse@arm.com; dave.hansen@linux.intel.com;
>> jiaqiyan@google.com; jthoughton@google.com; somasundaram.a@hpe.com;
>> erdemaktas@google.com; pgonda@google.com; rientjes@google.com;
>> duenwen@google.com; Vilas.Sridharan@amd.com; mike.malvestuto@intel.com;
>> gthelen@google.com; Linuxarm <linuxarm@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>> Zengtao (B) <prime.zeng@hisilicon.com>
>> Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>> documentation for scrub driver
>>
>> On 15.09.23 19:28, shiju.jose@huawei.com wrote:
>>> From: Shiju Jose <shiju.jose@huawei.com>
>>>
>>> Add documentation for scrub driver, supports configure scrub
>>> parameters, in Documentation/scrub-configure.rst
>>>
>>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>>> ---
>>>    Documentation/scrub-configure.rst | 55
>> +++++++++++++++++++++++++++++++
>>>    1 file changed, 55 insertions(+)
>>>    create mode 100644 Documentation/scrub-configure.rst
>>>
>>> diff --git a/Documentation/scrub-configure.rst
>>> b/Documentation/scrub-configure.rst
>>> new file mode 100644
>>> index 000000000000..9f8581b88788
>>> --- /dev/null
>>> +++ b/Documentation/scrub-configure.rst
>>> @@ -0,0 +1,55 @@
>>> +==========================
>>> +Scrub subsystem driver
>>> +==========================
>>> +
>>> +Copyright (c) 2023 HiSilicon Limited.
>>> +
>>> +:Author:   Shiju Jose <shiju.jose@huawei.com>
>>> +:License:  The GNU Free Documentation License, Version 1.2
>>> +          (dual licensed under the GPL v2) :Original Reviewers:
>>> +
>>> +- Written for: 6.7
>>> +- Updated for:
>>> +
>>> +Introduction
>>> +------------
>>> +The scrub subsystem driver provides the interface for configure the
>>
>> "... interface for configuring memory scrubbers in the system."
>>
>> are we only configuring firmware/hw-based memory scrubbing? I assume so.
> The scrub control could be used for the SW  based memory scrubbing too.

Okay, looks like there is not too much hw/firmware specific in there 
(besides these weird range changes).
[...]

>>> +-------
>>> +
>>> +  The usage takes the form shown in this example::
>>> +
>>> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
>>> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
>>> +    # cat /sys/class/scrub/scrub0/region0/speed_available
>>> +    # 1-60
>>> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
>>> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
>>> +
>>> +    # cat /sys/class/scrub/scrub0/region0/speed
>>> +    # 0x19
>>
>> Is it reasonable to return the speed as hex? You set it as dec.
> Presently return speed  as hex to reduce the number of callback function needed
> for reading the hex/dec data because the values for the address range
> need to be in hex.

If speed_available returns dec, speed better also return dec IMHO.

> 
>>
>>> +    # cat /sys/class/scrub/scrub0/region0/addr_base
>>> +    # 0x100000
>>
>> But didn't we set it to 0x300000 ...
> This is an emulated example for testing the RASF/RAS2 definition.
> According to the RASF & RAS2 definition, the actual address range in the
> platform could vary from the requested address range for the patrol scrubbing.
> "The platform calculates the nearest patrol scrub boundary address
> from where it can start". The platform returns the actual address range
> in response to GET_PATROL_PARAMETERS command to the firmware.
> Please see section 5.2.21.2.1 Hardware-based Memory Scrubbing ,
> Table 5.87: Parameter Block Structure for PATROL_SCRUB in the
> ACPI 6.5 specification.
> 

So you configure [0x300000 - 0x400000] and you get [0x100000 - 0x300000]

How does that make any sense? :)

Shouldn't we rather return an error when setting a range that is 
impossible, instead of the hardware deciding to scrub something 
completely different (as can be seen in the example)?

-- 
Cheers,

David / dhildenb


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-18 12:15       ` David Hildenbrand
@ 2023-09-18 12:28         ` Jonathan Cameron
  2023-09-18 12:34           ` David Hildenbrand
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2023-09-18 12:28 UTC (permalink / raw)
  To: David Hildenbrand, linuxarm
  Cc: Shiju Jose, linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, jiaqiyan,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen,
	Jonathan Cameron, tanxiaofei, Zengtao (B)

On Mon, 18 Sep 2023 14:15:33 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 18.09.23 12:25, Shiju Jose wrote:
> > Hi David,
> > 
> > Thanks for looking into this.
> >   
> >> -----Original Message-----
> >> From: David Hildenbrand <david@redhat.com>
> >> Sent: 18 September 2023 08:24
> >> To: Shiju Jose <shiju.jose@huawei.com>; linux-acpi@vger.kernel.org; linux-
> >> mm@kvack.org; linux-kernel@vger.kernel.org
> >> Cc: rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com;
> >> tony.luck@intel.com; james.morse@arm.com; dave.hansen@linux.intel.com;
> >> jiaqiyan@google.com; jthoughton@google.com; somasundaram.a@hpe.com;
> >> erdemaktas@google.com; pgonda@google.com; rientjes@google.com;
> >> duenwen@google.com; Vilas.Sridharan@amd.com; mike.malvestuto@intel.com;
> >> gthelen@google.com; Linuxarm <linuxarm@huawei.com>; Jonathan Cameron
> >> <jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
> >> Zengtao (B) <prime.zeng@hisilicon.com>
> >> Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
> >> documentation for scrub driver
> >>
> >> On 15.09.23 19:28, shiju.jose@huawei.com wrote:  
> >>> From: Shiju Jose <shiju.jose@huawei.com>
> >>>
> >>> Add documentation for scrub driver, supports configure scrub
> >>> parameters, in Documentation/scrub-configure.rst
> >>>
> >>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> >>> ---
> >>>    Documentation/scrub-configure.rst | 55  
> >> +++++++++++++++++++++++++++++++  
> >>>    1 file changed, 55 insertions(+)
> >>>    create mode 100644 Documentation/scrub-configure.rst
> >>>
> >>> diff --git a/Documentation/scrub-configure.rst
> >>> b/Documentation/scrub-configure.rst
> >>> new file mode 100644
> >>> index 000000000000..9f8581b88788
> >>> --- /dev/null
> >>> +++ b/Documentation/scrub-configure.rst
> >>> @@ -0,0 +1,55 @@
> >>> +==========================
> >>> +Scrub subsystem driver
> >>> +==========================
> >>> +
> >>> +Copyright (c) 2023 HiSilicon Limited.
> >>> +
> >>> +:Author:   Shiju Jose <shiju.jose@huawei.com>
> >>> +:License:  The GNU Free Documentation License, Version 1.2
> >>> +          (dual licensed under the GPL v2) :Original Reviewers:
> >>> +
> >>> +- Written for: 6.7
> >>> +- Updated for:
> >>> +
> >>> +Introduction
> >>> +------------
> >>> +The scrub subsystem driver provides the interface for configure the  
> >>
> >> "... interface for configuring memory scrubbers in the system."
> >>
> >> are we only configuring firmware/hw-based memory scrubbing? I assume so.  
> > The scrub control could be used for the SW  based memory scrubbing too.  
> 
> Okay, looks like there is not too much hw/firmware specific in there 
> (besides these weird range changes).
> [...]
> 
> >>> +-------
> >>> +
> >>> +  The usage takes the form shown in this example::
> >>> +
> >>> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
> >>> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
> >>> +    # cat /sys/class/scrub/scrub0/region0/speed_available
> >>> +    # 1-60
> >>> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
> >>> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
> >>> +
> >>> +    # cat /sys/class/scrub/scrub0/region0/speed
> >>> +    # 0x19  
> >>
> >> Is it reasonable to return the speed as hex? You set it as dec.  
> > Presently return speed  as hex to reduce the number of callback function needed
> > for reading the hex/dec data because the values for the address range
> > need to be in hex.  
> 
> If speed_available returns dec, speed better also return dec IMHO.
> 
> >   
> >>  
> >>> +    # cat /sys/class/scrub/scrub0/region0/addr_base
> >>> +    # 0x100000  
> >>
> >> But didn't we set it to 0x300000 ...  
> > This is an emulated example for testing the RASF/RAS2 definition.
> > According to the RASF & RAS2 definition, the actual address range in the
> > platform could vary from the requested address range for the patrol scrubbing.
> > "The platform calculates the nearest patrol scrub boundary address
> > from where it can start". The platform returns the actual address range
> > in response to GET_PATROL_PARAMETERS command to the firmware.
> > Please see section 5.2.21.2.1 Hardware-based Memory Scrubbing ,
> > Table 5.87: Parameter Block Structure for PATROL_SCRUB in the
> > ACPI 6.5 specification.
> >   
> 
> So you configure [0x300000 - 0x400000] and you get [0x100000 - 0x300000]
> 
> How does that make any sense? :)
> 
> Shouldn't we rather return an error when setting a range that is 
> impossible, instead of the hardware deciding to scrub something 
> completely different (as can be seen in the example)?
> 

A broader scrub is probably reasonable, but agreed that scrubbing narrower
is 'interesting' as not scrubbing the memory requeseted.
It's really annoying that neither ACPI table provides any proper
discoverability.  Whilst we can fix that long term, we are stuck with
a clunky poke it and see interface in the meantime.

Jonathan



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-18 12:28         ` Jonathan Cameron
@ 2023-09-18 12:34           ` David Hildenbrand
  2023-09-18 15:03             ` Shiju Jose
  0 siblings, 1 reply; 32+ messages in thread
From: David Hildenbrand @ 2023-09-18 12:34 UTC (permalink / raw)
  To: Jonathan Cameron, linuxarm
  Cc: Shiju Jose, linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, jiaqiyan,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen, tanxiaofei,
	Zengtao (B)

On 18.09.23 14:28, Jonathan Cameron wrote:
> On Mon, 18 Sep 2023 14:15:33 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 18.09.23 12:25, Shiju Jose wrote:
>>> Hi David,
>>>
>>> Thanks for looking into this.
>>>    
>>>> -----Original Message-----
>>>> From: David Hildenbrand <david@redhat.com>
>>>> Sent: 18 September 2023 08:24
>>>> To: Shiju Jose <shiju.jose@huawei.com>; linux-acpi@vger.kernel.org; linux-
>>>> mm@kvack.org; linux-kernel@vger.kernel.org
>>>> Cc: rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com;
>>>> tony.luck@intel.com; james.morse@arm.com; dave.hansen@linux.intel.com;
>>>> jiaqiyan@google.com; jthoughton@google.com; somasundaram.a@hpe.com;
>>>> erdemaktas@google.com; pgonda@google.com; rientjes@google.com;
>>>> duenwen@google.com; Vilas.Sridharan@amd.com; mike.malvestuto@intel.com;
>>>> gthelen@google.com; Linuxarm <linuxarm@huawei.com>; Jonathan Cameron
>>>> <jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>>>> Zengtao (B) <prime.zeng@hisilicon.com>
>>>> Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>>>> documentation for scrub driver
>>>>
>>>> On 15.09.23 19:28, shiju.jose@huawei.com wrote:
>>>>> From: Shiju Jose <shiju.jose@huawei.com>
>>>>>
>>>>> Add documentation for scrub driver, supports configure scrub
>>>>> parameters, in Documentation/scrub-configure.rst
>>>>>
>>>>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>>>>> ---
>>>>>     Documentation/scrub-configure.rst | 55
>>>> +++++++++++++++++++++++++++++++
>>>>>     1 file changed, 55 insertions(+)
>>>>>     create mode 100644 Documentation/scrub-configure.rst
>>>>>
>>>>> diff --git a/Documentation/scrub-configure.rst
>>>>> b/Documentation/scrub-configure.rst
>>>>> new file mode 100644
>>>>> index 000000000000..9f8581b88788
>>>>> --- /dev/null
>>>>> +++ b/Documentation/scrub-configure.rst
>>>>> @@ -0,0 +1,55 @@
>>>>> +==========================
>>>>> +Scrub subsystem driver
>>>>> +==========================
>>>>> +
>>>>> +Copyright (c) 2023 HiSilicon Limited.
>>>>> +
>>>>> +:Author:   Shiju Jose <shiju.jose@huawei.com>
>>>>> +:License:  The GNU Free Documentation License, Version 1.2
>>>>> +          (dual licensed under the GPL v2) :Original Reviewers:
>>>>> +
>>>>> +- Written for: 6.7
>>>>> +- Updated for:
>>>>> +
>>>>> +Introduction
>>>>> +------------
>>>>> +The scrub subsystem driver provides the interface for configure the
>>>>
>>>> "... interface for configuring memory scrubbers in the system."
>>>>
>>>> are we only configuring firmware/hw-based memory scrubbing? I assume so.
>>> The scrub control could be used for the SW  based memory scrubbing too.
>>
>> Okay, looks like there is not too much hw/firmware specific in there
>> (besides these weird range changes).
>> [...]
>>
>>>>> +-------
>>>>> +
>>>>> +  The usage takes the form shown in this example::
>>>>> +
>>>>> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
>>>>> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
>>>>> +    # cat /sys/class/scrub/scrub0/region0/speed_available
>>>>> +    # 1-60
>>>>> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
>>>>> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
>>>>> +
>>>>> +    # cat /sys/class/scrub/scrub0/region0/speed
>>>>> +    # 0x19
>>>>
>>>> Is it reasonable to return the speed as hex? You set it as dec.
>>> Presently return speed  as hex to reduce the number of callback function needed
>>> for reading the hex/dec data because the values for the address range
>>> need to be in hex.
>>
>> If speed_available returns dec, speed better also return dec IMHO.
>>
>>>    
>>>>   
>>>>> +    # cat /sys/class/scrub/scrub0/region0/addr_base
>>>>> +    # 0x100000
>>>>
>>>> But didn't we set it to 0x300000 ...
>>> This is an emulated example for testing the RASF/RAS2 definition.
>>> According to the RASF & RAS2 definition, the actual address range in the
>>> platform could vary from the requested address range for the patrol scrubbing.
>>> "The platform calculates the nearest patrol scrub boundary address
>>> from where it can start". The platform returns the actual address range
>>> in response to GET_PATROL_PARAMETERS command to the firmware.
>>> Please see section 5.2.21.2.1 Hardware-based Memory Scrubbing ,
>>> Table 5.87: Parameter Block Structure for PATROL_SCRUB in the
>>> ACPI 6.5 specification.
>>>    
>>
>> So you configure [0x300000 - 0x400000] and you get [0x100000 - 0x300000]
>>
>> How does that make any sense? :)
>>
>> Shouldn't we rather return an error when setting a range that is
>> impossible, instead of the hardware deciding to scrub something
>> completely different (as can be seen in the example)?
>>
> 
> A broader scrub is probably reasonable, but agreed that scrubbing narrower
> is 'interesting' as not scrubbing the memory requeseted.

It's not even narrower. Both ranges don't even intersect! (sorry to say, 
but this configuration interface doesn't make any sense if hardware just 
does *something* else).

If you can't configure it properly, fail with an error.

> It's really annoying that neither ACPI table provides any proper
> discoverability.  Whilst we can fix that long term, we are stuck with
> a clunky poke it and see interface in the meantime.

Can't you set it, briefly enable it, and read the values back? Then, you 
can complain to the user that the configured range is impossible.

-- 
Cheers,

David / dhildenb


^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver
  2023-09-18 12:34           ` David Hildenbrand
@ 2023-09-18 15:03             ` Shiju Jose
  0 siblings, 0 replies; 32+ messages in thread
From: Shiju Jose @ 2023-09-18 15:03 UTC (permalink / raw)
  To: David Hildenbrand, Jonathan Cameron, Linuxarm
  Cc: linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, jiaqiyan,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen, tanxiaofei,
	Zengtao (B)



>-----Original Message-----
>From: David Hildenbrand <david@redhat.com>
>Sent: 18 September 2023 13:35
>To: Jonathan Cameron <jonathan.cameron@huawei.com>; Linuxarm
><linuxarm@huawei.com>
>Cc: Shiju Jose <shiju.jose@huawei.com>; linux-acpi@vger.kernel.org; linux-
>mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org;
>lenb@kernel.org; naoya.horiguchi@nec.com; tony.luck@intel.com;
>james.morse@arm.com; dave.hansen@linux.intel.com; jiaqiyan@google.com;
>jthoughton@google.com; somasundaram.a@hpe.com;
>erdemaktas@google.com; pgonda@google.com; rientjes@google.com;
>duenwen@google.com; Vilas.Sridharan@amd.com; mike.malvestuto@intel.com;
>gthelen@google.com; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>documentation for scrub driver
>
>On 18.09.23 14:28, Jonathan Cameron wrote:
>> On Mon, 18 Sep 2023 14:15:33 +0200
>> David Hildenbrand <david@redhat.com> wrote:
>>
>>> On 18.09.23 12:25, Shiju Jose wrote:
>>>> Hi David,
>>>>
>>>> Thanks for looking into this.
>>>>
>>>>> -----Original Message-----
>>>>> From: David Hildenbrand <david@redhat.com>
>>>>> Sent: 18 September 2023 08:24
>>>>> To: Shiju Jose <shiju.jose@huawei.com>; linux-acpi@vger.kernel.org;
>>>>> linux- mm@kvack.org; linux-kernel@vger.kernel.org
>>>>> Cc: rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com;
>>>>> tony.luck@intel.com; james.morse@arm.com;
>>>>> dave.hansen@linux.intel.com; jiaqiyan@google.com;
>>>>> jthoughton@google.com; somasundaram.a@hpe.com;
>>>>> erdemaktas@google.com; pgonda@google.com; rientjes@google.com;
>>>>> duenwen@google.com; Vilas.Sridharan@amd.com;
>>>>> mike.malvestuto@intel.com; gthelen@google.com; Linuxarm
>>>>> <linuxarm@huawei.com>; Jonathan Cameron
>>>>> <jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>>>>> Zengtao (B) <prime.zeng@hisilicon.com>
>>>>> Subject: Re: [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add
>>>>> documentation for scrub driver
>>>>>
>>>>> On 15.09.23 19:28, shiju.jose@huawei.com wrote:
>>>>>> From: Shiju Jose <shiju.jose@huawei.com>
>>>>>>
>>>>>> Add documentation for scrub driver, supports configure scrub
>>>>>> parameters, in Documentation/scrub-configure.rst
>>>>>>
>>>>>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>>>>>> ---
>>>>>>     Documentation/scrub-configure.rst | 55
>>>>> +++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 55 insertions(+)
>>>>>>     create mode 100644 Documentation/scrub-configure.rst
>>>>>>
>>>>>> diff --git a/Documentation/scrub-configure.rst
>>>>>> b/Documentation/scrub-configure.rst
>>>>>> new file mode 100644
>>>>>> index 000000000000..9f8581b88788
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/scrub-configure.rst
>>>>>> @@ -0,0 +1,55 @@
>>>>>> +==========================
>>>>>> +Scrub subsystem driver
>>>>>> +==========================
>>>>>> +
>>>>>> +Copyright (c) 2023 HiSilicon Limited.
>>>>>> +
>>>>>> +:Author:   Shiju Jose <shiju.jose@huawei.com>
>>>>>> +:License:  The GNU Free Documentation License, Version 1.2
>>>>>> +          (dual licensed under the GPL v2) :Original Reviewers:
>>>>>> +
>>>>>> +- Written for: 6.7
>>>>>> +- Updated for:
>>>>>> +
>>>>>> +Introduction
>>>>>> +------------
>>>>>> +The scrub subsystem driver provides the interface for configure
>>>>>> +the
>>>>>
>>>>> "... interface for configuring memory scrubbers in the system."
>>>>>
>>>>> are we only configuring firmware/hw-based memory scrubbing? I assume
>so.
>>>> The scrub control could be used for the SW  based memory scrubbing too.
>>>
>>> Okay, looks like there is not too much hw/firmware specific in there
>>> (besides these weird range changes).
>>> [...]
>>>
>>>>>> +-------
>>>>>> +
>>>>>> +  The usage takes the form shown in this example::
>>>>>> +
>>>>>> +    # echo 0x300000 > /sys/class/scrub/scrub0/region0/addr_base
>>>>>> +    # echo 0x100000 > /sys/class/scrub/scrub0/region0/addr_size
>>>>>> +    # cat /sys/class/scrub/scrub0/region0/speed_available
>>>>>> +    # 1-60
>>>>>> +    # echo 25 > /sys/class/scrub/scrub0/region0/speed
>>>>>> +    # echo 1 > /sys/class/scrub/scrub0/region0/enable
>>>>>> +
>>>>>> +    # cat /sys/class/scrub/scrub0/region0/speed
>>>>>> +    # 0x19
>>>>>
>>>>> Is it reasonable to return the speed as hex? You set it as dec.
>>>> Presently return speed  as hex to reduce the number of callback
>>>> function needed for reading the hex/dec data because the values for
>>>> the address range need to be in hex.
>>>
>>> If speed_available returns dec, speed better also return dec IMHO.
>>>
>>>>
>>>>>
>>>>>> +    # cat /sys/class/scrub/scrub0/region0/addr_base
>>>>>> +    # 0x100000
>>>>>
>>>>> But didn't we set it to 0x300000 ...
>>>> This is an emulated example for testing the RASF/RAS2 definition.
>>>> According to the RASF & RAS2 definition, the actual address range in
>>>> the platform could vary from the requested address range for the patrol
>scrubbing.
>>>> "The platform calculates the nearest patrol scrub boundary address
>>>> from where it can start". The platform returns the actual address
>>>> range in response to GET_PATROL_PARAMETERS command to the
>firmware.
>>>> Please see section 5.2.21.2.1 Hardware-based Memory Scrubbing ,
>>>> Table 5.87: Parameter Block Structure for PATROL_SCRUB in the ACPI
>>>> 6.5 specification.
>>>>
>>>
>>> So you configure [0x300000 - 0x400000] and you get [0x100000 -
>>> 0x300000]
>>>
>>> How does that make any sense? :)
>>>
>>> Shouldn't we rather return an error when setting a range that is
>>> impossible, instead of the hardware deciding to scrub something
>>> completely different (as can be seen in the example)?
>>>
>>
>> A broader scrub is probably reasonable, but agreed that scrubbing
>> narrower is 'interesting' as not scrubbing the memory requeseted.
>
>It's not even narrower. Both ranges don't even intersect! (sorry to say, but this
>configuration interface doesn't make any sense if hardware just does
>*something* else).
>
>If you can't configure it properly, fail with an error.
>
>> It's really annoying that neither ACPI table provides any proper
>> discoverability.  Whilst we can fix that long term, we are stuck with
>> a clunky poke it and see interface in the meantime.
>
>Can't you set it, briefly enable it, and read the values back? Then, you can
>complain to the user that the configured range is impossible.

Will try to add report to the user that the configured address range is not possible.

>
>--
>Cheers,
>
>David / dhildenb

Thanks,
Shiju

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers
  2023-09-18 10:19   ` Shiju Jose
@ 2023-09-18 17:47     ` Jiaqi Yan
  2023-09-19  8:28       ` Shiju Jose
  0 siblings, 1 reply; 32+ messages in thread
From: Jiaqi Yan @ 2023-09-18 17:47 UTC (permalink / raw)
  To: Shiju Jose
  Cc: linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, david,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen, Linuxarm,
	Jonathan Cameron, tanxiaofei, Zengtao (B),
	bp, mchehab, rric, linux-edac

On Mon, Sep 18, 2023 at 3:20 AM Shiju Jose <shiju.jose@huawei.com> wrote:
>
> [+cc linux-edac@vger.kernel.org]
>
> Hello,
>
> >-----Original Message-----
> >From: Jiaqi Yan <jiaqiyan@google.com>
> >Sent: 17 September 2023 22:14
> >To: Shiju Jose <shiju.jose@huawei.com>
> >Cc: linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-
> >kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> >naoya.horiguchi@nec.com; tony.luck@intel.com; james.morse@arm.com;
> >dave.hansen@linux.intel.com; david@redhat.com; jthoughton@google.com;
> >somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com;
> >rientjes@google.com; duenwen@google.com; Vilas.Sridharan@amd.com;
> >mike.malvestuto@intel.com; gthelen@google.com; Linuxarm
> ><linuxarm@huawei.com>; Jonathan Cameron
> ><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
> >Zengtao (B) <prime.zeng@hisilicon.com>
> >Subject: Re: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2
> >and configure scrubbers
> >
> >On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
> >>
> >> From: Shiju Jose <shiju.jose@huawei.com>
> >>
> >> This series add,
> >> 1. support for ACPI RASF(RAS feature table) PCC interfaces to
> >> communicate with the HW patrol scrubber in the platform, as per ACPI
> >> 5.1 & upwards revision. Section 5.2.20.
> >>
> >> 2. support for ACPI RAS2(RAS2 feature table), as per ACPI 6.5 &
> >> upwards revision. Section 5.2.21.
> >>
> >> 3. scrub driver supports configuring parameters of the memory
> >> scrubbers in the system. This driver has been implemented based on the
> >> hwmon subsystem.
> >>
> >> The features have tested with RASF and RAS2 emulation in the QEMU.
> >
> >I am very curious how the test is done. Does the hw patrol scrubber on host
> >actually been driven by the driver to scrub memory DIMMs (doesn't seem so to
> >me, but do correct me)? Or it is like to a VM scrubbing is simulated and no real
> >op to DIMMs?
> Intent here is hardware scrubber on host as far as we are concerned.

Sorry maybe my question was not clear, so let me try again.

Does the driver being tested on a machine and directly or indirectly
control (start/stop) hardware patrol scrubber to scrub physical
memory? what are the CPU chip, memory controller, and DIMM chips?

> Could be used for VM too perhaps. We did it with QEMU emulation for now
>  to get the flexibility of configuration. However there will be other scrub controls
> over time, such as DDR5 ECS.
> https://media-www.micron.com/-/media/client/global/documents/products/white-paper/ddr5_new_features_white_paper.pdf?rev=b98f4977d9334b4aa5d0d211a92bf14a
>
> Also found there is very simple support for scrub control in edac, and an alternative path
> would be to look at extending that to sufficient complexity to support region based scanning.
> https://elixir.bootlin.com/linux/latest/source/include/linux/edac.h#L512
>
> >
> >>
> >> Previous references to the memory scub and RASF topics.
> >> https://lore.kernel.org/all/20221103155029.2451105-1-jiaqiyan@google.c
> >> om/
> >> https://patchwork.kernel.org/project/linux-arm-kernel/patch/CS1PR84MB0
> >>
> >038718F49DBC0FF03919E1184390@CS1PR84MB0038.NAMPRD84.PROD.OUTLO
> >OK.COM/
> >>
> >> A Somasundaram (2):
> >>   ACPI:RASF: Add extract RASF table to register RASF platform devices
> >>   ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces
> >>
> >> Shiju Jose (7):
> >>   memory: scrub: Add scrub driver supports configuring memory scrubbers
> >>     in the system
> >>   memory: scrub: sysfs: Add Documentation entries for set of scrub
> >>     attributes
> >>   Documentation/scrub-configure.rst: Add documentation for scrub driver
> >>   memory: RASF: Add memory RASF driver
> >>   ACPICA: ACPI 6.5: Add support for RAS2 table
> >>   ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2)
> >>   memory: RAS2: Add memory RAS2 driver
> >>
> >>  .../ABI/testing/sysfs-class-scrub-configure   |  82 ++++
> >>  Documentation/scrub-configure.rst             |  55 +++
> >>  drivers/acpi/Kconfig                          |  15 +
> >>  drivers/acpi/Makefile                         |   1 +
> >>  drivers/acpi/ras2_acpi.c                      |  97 ++++
> >>  drivers/acpi/rasf_acpi.c                      |  71 +++
> >>  drivers/acpi/rasf_acpi_common.c               | 272 +++++++++++
> >>  drivers/memory/Kconfig                        |  15 +
> >>  drivers/memory/Makefile                       |   3 +
> >>  drivers/memory/ras2.c                         | 334 +++++++++++++
> >>  drivers/memory/rasf.c                         | 335 +++++++++++++
> >>  drivers/memory/rasf_common.c                  | 251 ++++++++++
> >>  drivers/memory/scrub/Kconfig                  |  11 +
> >>  drivers/memory/scrub/Makefile                 |   6 +
> >>  drivers/memory/scrub/memory-scrub.c           | 452 ++++++++++++++++++
> >>  include/acpi/actbl2.h                         |  55 +++
> >>  include/acpi/rasf_acpi.h                      |  59 +++
> >>  include/memory/memory-scrub.h                 |  85 ++++
> >>  include/memory/rasf.h                         |  82 ++++
> >>  19 files changed, 2281 insertions(+)
> >>  create mode 100644
> >> Documentation/ABI/testing/sysfs-class-scrub-configure
> >>  create mode 100644 Documentation/scrub-configure.rst  create mode
> >> 100755 drivers/acpi/ras2_acpi.c  create mode 100755
> >> drivers/acpi/rasf_acpi.c  create mode 100755
> >> drivers/acpi/rasf_acpi_common.c  create mode 100644
> >> drivers/memory/ras2.c  create mode 100644 drivers/memory/rasf.c
> >> create mode 100644 drivers/memory/rasf_common.c  create mode 100644
> >> drivers/memory/scrub/Kconfig  create mode 100644
> >> drivers/memory/scrub/Makefile  create mode 100755
> >> drivers/memory/scrub/memory-scrub.c
> >>  create mode 100755 include/acpi/rasf_acpi.h  create mode 100755
> >> include/memory/memory-scrub.h  create mode 100755
> >> include/memory/rasf.h
> >>
> >> --
> >> 2.34.1
> >>
>
> Thanks,
> Shiju

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers
  2023-09-18 17:47     ` Jiaqi Yan
@ 2023-09-19  8:28       ` Shiju Jose
  0 siblings, 0 replies; 32+ messages in thread
From: Shiju Jose @ 2023-09-19  8:28 UTC (permalink / raw)
  To: Jiaqi Yan
  Cc: linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, tony.luck, james.morse, dave.hansen, david,
	jthoughton, somasundaram.a, erdemaktas, pgonda, rientjes,
	duenwen, Vilas.Sridharan, mike.malvestuto, gthelen, Linuxarm,
	Jonathan Cameron, tanxiaofei, Zengtao (B),
	bp, mchehab, rric, linux-edac

>-----Original Message-----
>From: Jiaqi Yan <jiaqiyan@google.com>
>Sent: 18 September 2023 18:47
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-
>kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
>naoya.horiguchi@nec.com; tony.luck@intel.com; james.morse@arm.com;
>dave.hansen@linux.intel.com; david@redhat.com; jthoughton@google.com;
>somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com;
>rientjes@google.com; duenwen@google.com; Vilas.Sridharan@amd.com;
>mike.malvestuto@intel.com; gthelen@google.com; Linuxarm
><linuxarm@huawei.com>; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>; bp@alien8.de; mchehab@kernel.org;
>rric@kernel.org; linux-edac@vger.kernel.org
>Subject: Re: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2
>and configure scrubbers
>
>On Mon, Sep 18, 2023 at 3:20 AM Shiju Jose <shiju.jose@huawei.com> wrote:
>>
>> [+cc linux-edac@vger.kernel.org]
>>
>> Hello,
>>
>> >-----Original Message-----
>> >From: Jiaqi Yan <jiaqiyan@google.com>
>> >Sent: 17 September 2023 22:14
>> >To: Shiju Jose <shiju.jose@huawei.com>
>> >Cc: linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-
>> >kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
>> >naoya.horiguchi@nec.com; tony.luck@intel.com; james.morse@arm.com;
>> >dave.hansen@linux.intel.com; david@redhat.com; jthoughton@google.com;
>> >somasundaram.a@hpe.com; erdemaktas@google.com;
>pgonda@google.com;
>> >rientjes@google.com; duenwen@google.com; Vilas.Sridharan@amd.com;
>> >mike.malvestuto@intel.com; gthelen@google.com; Linuxarm
>> ><linuxarm@huawei.com>; Jonathan Cameron
>> ><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>> >Zengtao (B) <prime.zeng@hisilicon.com>
>> >Subject: Re: [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF,
>> >ACPI RAS2 and configure scrubbers
>> >
>> >On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
>> >>
>> >> From: Shiju Jose <shiju.jose@huawei.com>
>> >>
>> >> This series add,
>> >> 1. support for ACPI RASF(RAS feature table) PCC interfaces to
>> >> communicate with the HW patrol scrubber in the platform, as per
>> >> ACPI
>> >> 5.1 & upwards revision. Section 5.2.20.
>> >>
>> >> 2. support for ACPI RAS2(RAS2 feature table), as per ACPI 6.5 &
>> >> upwards revision. Section 5.2.21.
>> >>
>> >> 3. scrub driver supports configuring parameters of the memory
>> >> scrubbers in the system. This driver has been implemented based on
>> >> the hwmon subsystem.
>> >>
>> >> The features have tested with RASF and RAS2 emulation in the QEMU.
>> >
>> >I am very curious how the test is done. Does the hw patrol scrubber
>> >on host actually been driven by the driver to scrub memory DIMMs
>> >(doesn't seem so to me, but do correct me)? Or it is like to a VM
>> >scrubbing is simulated and no real op to DIMMs?
>> Intent here is hardware scrubber on host as far as we are concerned.
>
>Sorry maybe my question was not clear, so let me try again.
>
>Does the driver being tested on a machine and directly or indirectly control
>(start/stop) hardware patrol scrubber to scrub physical memory? what are the
>CPU chip, memory controller, and DIMM chips?
The driver has been tested with qemu_system_aarch64 with state machine to emulate
the RASF and RAS2 tables support for configuring parameters of a patrol scrubber.
Thus no real HW patrol scrubber and DIMM involved in the testing.

>
>> Could be used for VM too perhaps. We did it with QEMU emulation for
>> now  to get the flexibility of configuration. However there will be
>> other scrub controls over time, such as DDR5 ECS.
>> https://media-www.micron.com/-/media/client/global/documents/products/
>> white-
>paper/ddr5_new_features_white_paper.pdf?rev=b98f4977d9334b4aa5d0
>> d211a92bf14a
>>
>> Also found there is very simple support for scrub control in edac, and

Thanks,
Shiju

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-09-15 17:28 ` [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes shiju.jose
@ 2023-09-22  0:07   ` Jiaqi Yan
  2023-09-22 10:20     ` Jonathan Cameron
  0 siblings, 1 reply; 32+ messages in thread
From: Jiaqi Yan @ 2023-09-22  0:07 UTC (permalink / raw)
  To: shiju.jose, Luck, Tony, dave.hansen, jon.grimm, vilas.sridharan,
	David Rientjes
  Cc: linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, james.morse, david, jthoughton, somasundaram.a,
	erdemaktas, pgonda, duenwen, mike.malvestuto, gthelen, linuxarm,
	jonathan.cameron, tanxiaofei, prime.zeng

On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
>
> From: Shiju Jose <shiju.jose@huawei.com>
>
> Add sysfs documentation entries for the set of attributes those are
> exposed in /sys/class/scrub/ by the scrub driver. These attributes
> support configuring parameters of a scrub device.
>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
>  .../ABI/testing/sysfs-class-scrub-configure   | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure
>
> diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure b/Documentation/ABI/testing/sysfs-class-scrub-configure
> new file mode 100644
> index 000000000000..347e2167dc62
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
> @@ -0,0 +1,82 @@
> +What:          /sys/class/scrub/
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               The scrub/ class subdirectory belongs to the
> +               scrubber subsystem.
> +
> +What:          /sys/class/scrub/scrubX/
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               The /sys/class/scrub/scrub{0,1,2,3,...} directories

This API (sysfs interface) is very specific to the ACPI interface
defined for hardware patrol scrubber. I wonder can we have some
interface that is more generic, for a couple of reasons:

1. I am not aware of any chip/platform hardware that implemented the
hw ps part defined in ACPI RASF/RAS2 spec. So I am curious what the
RAS experts from different hardware vendors think about this. For
example, Tony and Dave from Intel, Jon and Vilas from AMD. Is there
any hardware platform (if allowed to disclose) that implemented ACPI
RASF/RAS2? If so, will vendors continue to support the control of
patrol scrubber using the ACPI spec? If not (as Tony said in [1], will
the vendor consider starting some future platform?

If we are unlikely to get the vendor support, creating this ACPI
specific sysfs API (and the driver implementations) in Linux seems to
have limited meaning.

> +               correspond to each scrub device.
> +
> +What:          /sys/class/scrub/scrubX/name
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               (RO) name of the memory scrub device
> +
> +What:          /sys/class/scrub/scrubX/regionY/

2. I believe the concept of "region" here is probably from
PATROL_SCRUB defined in “APCI section 5.2.20.5. Parameter Block". It
is indeed powerful: if a process's physical memory spans over multiple
memory controllers, OS can in theory scrub chunks of the memory
belonging to the process. However, from a previous discussion [1],
"From a h/w perspective it might always be complex". IIUC, the address
translation from physical address to channel address is hard to
achieve, and probably that's one of the tech reasons the patrol scrub
ACPI spec is not in practice implemented?

So my take is, control at the granularity of the memory controller is
probably a nice compromise. Both OS and userspace can get a pretty
decent amount of flexibility, start/stop/adjust speed of the scrubbing
on a memory controller; meanwhile it doesn't impose too much pain to
hardware vendors when they provide these features in hardware. In
terms of how these controls/features will be implemented, I imagine it
could be implemented:
* via hardware registers that directly or indirectly control on memory
controllers
* via ACPI if the situation changes in 10 years (and the RASF/RAS2/PCC
drivers implemented in this patchset can be directly plugged into)
* a kernel-thread that uses cpu read to detect memory errors, if
hardware support is unavailable or not good enough

Given these possible backends of scrubbing, I think a more generic
sysfs API that covers and abstracts these backends will be more
valuable right now. I haven’t thought thoroughly, but how about
defining the top-level interface as something like
“/sys/devices/system/memory_controller_scrubX/”, or
“/sys/class/memory_controllerX/scrub”?

[1] https://lore.kernel.org/linux-mm/SJ1PR11MB6083BF93E9A88E659CED5EC4FC3F9@SJ1PR11MB6083.namprd11.prod.outlook.com/T/#m13516ee35caa05b506080ae805bee14f9f958d43

> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               The /sys/class/scrub/scrubX/region{0,1,2,3,...}
> +               directories correspond to each scrub region under a scrub device.
> +               Scrub region is a physical address range for which scrub may be
> +               separately controlled. Regions may overlap in which case the
> +               scrubbing rate of the overlapped memory will be at least that
> +               expected due to each overlapping region.
> +
> +What:          /sys/class/scrub/scrubX/regionY/addr_base
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               (RW) The base of the address range of the memory region
> +               to be patrol scrubbed.
> +               On reading, returns the base of the memory region for
> +               the actual address range(The platform calculates
> +               the nearest patrol scrub boundary address from where
> +               it can start scrubbing).
> +
> +What:          /sys/class/scrub/scrubX/regionY/addr_size
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               (RW) The size of the address range to be patrol scrubbed.
> +               On reading, returns the size of the memory region for
> +               the actual address range.
> +
> +What:          /sys/class/scrub/scrubX/regionY/enable
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               (WO) Start/Stop scrubbing the memory region.
> +               1 - enable the memory scrubbing.
> +               0 - disable the memory scrubbing..
> +
> +What:          /sys/class/scrub/scrubX/regionY/speed_available
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               (RO) Supported range for the partol speed(scrub rate)
> +               by the scrubber for a memory region.
> +               The unit of the scrub rate vary depends on the scrubber.
> +
> +What:          /sys/class/scrub/scrubX/regionY/speed
> +Date:          September 2023
> +KernelVersion: 6.7
> +Contact:       linux-kernel@vger.kernel.org
> +Description:
> +               (RW) The partol speed(scrub rate) on the memory region specified and
> +               it must be with in the supported range by the scrubber.
> +               The unit of the scrub rate vary depends on the scrubber.
> --
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-09-22  0:07   ` Jiaqi Yan
@ 2023-09-22 10:20     ` Jonathan Cameron
  2023-09-28  5:25       ` Jiaqi Yan
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2023-09-22 10:20 UTC (permalink / raw)
  To: Jiaqi Yan, linuxarm
  Cc: shiju.jose, Luck, Tony, dave.hansen, jon.grimm, vilas.sridharan,
	David Rientjes, linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, james.morse, david, jthoughton, somasundaram.a,
	erdemaktas, pgonda, duenwen, mike.malvestuto, gthelen,
	jonathan.cameron, tanxiaofei, prime.zeng

On Thu, 21 Sep 2023 17:07:04 -0700
Jiaqi Yan <jiaqiyan@google.com> wrote:

> On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
> >
> > From: Shiju Jose <shiju.jose@huawei.com>
> >
> > Add sysfs documentation entries for the set of attributes those are
> > exposed in /sys/class/scrub/ by the scrub driver. These attributes
> > support configuring parameters of a scrub device.
> >
> > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> > ---
> >  .../ABI/testing/sysfs-class-scrub-configure   | 82 +++++++++++++++++++
> >  1 file changed, 82 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure b/Documentation/ABI/testing/sysfs-class-scrub-configure
> > new file mode 100644
> > index 000000000000..347e2167dc62
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
> > @@ -0,0 +1,82 @@
> > +What:          /sys/class/scrub/
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               The scrub/ class subdirectory belongs to the
> > +               scrubber subsystem.
> > +
> > +What:          /sys/class/scrub/scrubX/
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               The /sys/class/scrub/scrub{0,1,2,3,...} directories  
> 
> This API (sysfs interface) is very specific to the ACPI interface
> defined for hardware patrol scrubber. I wonder can we have some
> interface that is more generic, for a couple of reasons:

Agreed that it makes sense to define a broad interface.  We have
some hardware focused drivers we can't share yet (IP rules until
a release date in the near future) where this is a reasonable fit
- but indeed there are others such as mapping this to DDR ECS
where it isn't a great mapping.

I'd love to come up with an interface that has the right blend
of generality and flexibility.  That is easiest done before we have
any implementation merged.

> 
> 1. I am not aware of any chip/platform hardware that implemented the
> hw ps part defined in ACPI RASF/RAS2 spec. So I am curious what the
> RAS experts from different hardware vendors think about this. For
> example, Tony and Dave from Intel, Jon and Vilas from AMD. Is there
> any hardware platform (if allowed to disclose) that implemented ACPI
> RASF/RAS2? If so, will vendors continue to support the control of
> patrol scrubber using the ACPI spec? If not (as Tony said in [1], will
> the vendor consider starting some future platform?
> 
> If we are unlikely to get the vendor support, creating this ACPI
> specific sysfs API (and the driver implementations) in Linux seems to
> have limited meaning.

There is a bit of a chicken and egg problem here. Until there is 
reasonable support in kernel (or it looks like there will be),
BIOS teams push back on a requirement to add the tables.
I'd encourage no one to bother with RASF - RAS2 is much less
ambiguous.

> 
> > +               correspond to each scrub device.
> > +
> > +What:          /sys/class/scrub/scrubX/name
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               (RO) name of the memory scrub device
> > +
> > +What:          /sys/class/scrub/scrubX/regionY/  
> 
> 2. I believe the concept of "region" here is probably from
> PATROL_SCRUB defined in “APCI section 5.2.20.5. Parameter Block". It
> is indeed powerful: if a process's physical memory spans over multiple
> memory controllers, OS can in theory scrub chunks of the memory
> belonging to the process. However, from a previous discussion [1],
> "From a h/w perspective it might always be complex". IIUC, the address
> translation from physical address to channel address is hard to
> achieve, and probably that's one of the tech reasons the patrol scrub
> ACPI spec is not in practice implemented?

Next bit is kind of an aside as I mostly agree with your conclusions ;)

This obviously depends on your memory interleave. You want to present
physical address ranges as single controllable regions - probably
most interesting being stuff that maps to NUMA nodes.  The short
answer is that any firmware control will end up writing to all the
memory controllers involved in a given PA range - firmware can easily
establish which those are.

A memory controller can support multiple scrub regions
which map from a PA range to a particular set of RAM addresses
- that's implementation specific. The memory controller is getting
the host PA and can carry out appropriate hashing if it wants to.
Many scrub solutions won't do this - in which case it's max one
region per memory controller (mapped by firmware to one region per
interleave set - assuming interleave sets take in whole memory
controllers - which they normally do).

I would expect existing systems (not design with this differentiated
scrub in mind) to only support scrub control by PA range at the
granularity of interleave sets.

Note that this general question of PA based controls also
maps to things like resource control (resctl) where it's only interesting
to partition memory bandwidth such that the partition applies to the
whole interleave set - that's done for ARM systems anyway by having
the userspace interface pretend there is only one control, but
write the settings to all actual controllers involved. Not sure what
x86 does.

Taking a few examples that I know of.  All 4 socket server - with
control of these as bios options ;).
Assuming individual memory controllers don't allow scrub to be
configured by PA range.

1. Full system memory interleave (people do this form of crazy)
   In that case, there is only one set of firmware controls
   that write to the interfaces of every memory controller.  Depending
   on the interleave design that may still allow multiple regions.
  
2. Socket wide memory interleave.  In that case, firmware controls
   need to effect all memory controllers in that socket if the
   requested 'region' maps to them.  So 4 PA regions. 

3. Die wide memory interleave.  Finer granularity of control
   so perhaps 8 PA rgiones.

4. Finer granularity (there are reasons to do this for above mentioned
   bandwidth resource control which you can only do if not interleaving
   across multiple controllers).



> 
> So my take is, control at the granularity of the memory controller is
> probably a nice compromise. 
> Both OS and userspace can get a pretty
> decent amount of flexibility, start/stop/adjust speed of the scrubbing
> on a memory controller; meanwhile it doesn't impose too much pain to
> hardware vendors when they provide these features in hardware. In
> terms of how these controls/features will be implemented, I imagine it
> could be implemented:
> * via hardware registers that directly or indirectly control on memory
> controllers
> * via ACPI if the situation changes in 10 years (and the RASF/RAS2/PCC
> drivers implemented in this patchset can be directly plugged into)
> * a kernel-thread that uses cpu read to detect memory errors, if
> hardware support is unavailable or not good enough
> 

I more or less agree, but would tweak a little.

1) Allow for multiple regions per memory controller - that's needed
   for RASF etc anyway - because as far as they are concerned there
   is only one interface presented.
2) Tie the interface to interleave sets, not memory controllers.
   NUMA nodes often being a good stand in for those.
   Controlling memory controllers separately for parts of an interleave
   isn't something I'd think was very useful.  This will probably get
   messy in the future though and the complexity could be pushed to
   a userspace tool - as long as enough info is available elsewhere
   in sysfs.  So need those memory controller directories you propose
   to include info on the PA ranges that they are involved in backing
   (which to keep things horrible, can change at runtime via memory
    hotplug and remapping of host physical address ranges on CXL etc)

> Given these possible backends of scrubbing, I think a more generic
> sysfs API that covers and abstracts these backends will be more
> valuable right now. I haven’t thought thoroughly, but how about
> defining the top-level interface as something like
> “/sys/devices/system/memory_controller_scrubX/”, or
> “/sys/class/memory_controllerX/scrub”?

No particular harm in the rename of the directory I guess.
Though some of those 'memory_controllers' would be virtual as they
wouldn't correspond to actual memory controllers but rather to
sets of them.

Jonathan

> 
> [1] https://lore.kernel.org/linux-mm/SJ1PR11MB6083BF93E9A88E659CED5EC4FC3F9@SJ1PR11MB6083.namprd11.prod.outlook.com/T/#m13516ee35caa05b506080ae805bee14f9f958d43

> 
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               The /sys/class/scrub/scrubX/region{0,1,2,3,...}
> > +               directories correspond to each scrub region under a scrub device.
> > +               Scrub region is a physical address range for which scrub may be
> > +               separately controlled. Regions may overlap in which case the
> > +               scrubbing rate of the overlapped memory will be at least that
> > +               expected due to each overlapping region.
> > +
> > +What:          /sys/class/scrub/scrubX/regionY/addr_base
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               (RW) The base of the address range of the memory region
> > +               to be patrol scrubbed.
> > +               On reading, returns the base of the memory region for
> > +               the actual address range(The platform calculates
> > +               the nearest patrol scrub boundary address from where
> > +               it can start scrubbing).
> > +
> > +What:          /sys/class/scrub/scrubX/regionY/addr_size
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               (RW) The size of the address range to be patrol scrubbed.
> > +               On reading, returns the size of the memory region for
> > +               the actual address range.
> > +
> > +What:          /sys/class/scrub/scrubX/regionY/enable
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               (WO) Start/Stop scrubbing the memory region.
> > +               1 - enable the memory scrubbing.
> > +               0 - disable the memory scrubbing..
> > +
> > +What:          /sys/class/scrub/scrubX/regionY/speed_available
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               (RO) Supported range for the partol speed(scrub rate)
> > +               by the scrubber for a memory region.
> > +               The unit of the scrub rate vary depends on the scrubber.
> > +
> > +What:          /sys/class/scrub/scrubX/regionY/speed
> > +Date:          September 2023
> > +KernelVersion: 6.7
> > +Contact:       linux-kernel@vger.kernel.org
> > +Description:
> > +               (RW) The partol speed(scrub rate) on the memory region specified and
> > +               it must be with in the supported range by the scrubber.
> > +               The unit of the scrub rate vary depends on the scrubber.
> > --
> > 2.34.1
> >
> >  
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-09-22 10:20     ` Jonathan Cameron
@ 2023-09-28  5:25       ` Jiaqi Yan
  2023-09-28 13:14         ` Jonathan Cameron
  2023-10-05  3:18         ` David Rientjes
  0 siblings, 2 replies; 32+ messages in thread
From: Jiaqi Yan @ 2023-09-28  5:25 UTC (permalink / raw)
  To: Jonathan Cameron, Luck, Tony, dave.hansen, jon.grimm, vilas.sridharan
  Cc: linuxarm, shiju.jose, David Rientjes, linux-acpi, linux-mm,
	linux-kernel, rafael, lenb, naoya.horiguchi, james.morse, david,
	jthoughton, somasundaram.a, erdemaktas, pgonda, duenwen,
	mike.malvestuto, gthelen, tanxiaofei, prime.zeng

On Fri, Sep 22, 2023 at 3:20 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Thu, 21 Sep 2023 17:07:04 -0700
> Jiaqi Yan <jiaqiyan@google.com> wrote:
>
> > On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:
> > >
> > > From: Shiju Jose <shiju.jose@huawei.com>
> > >
> > > Add sysfs documentation entries for the set of attributes those are
> > > exposed in /sys/class/scrub/ by the scrub driver. These attributes
> > > support configuring parameters of a scrub device.
> > >
> > > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> > > ---
> > >  .../ABI/testing/sysfs-class-scrub-configure   | 82 +++++++++++++++++++
> > >  1 file changed, 82 insertions(+)
> > >  create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure b/Documentation/ABI/testing/sysfs-class-scrub-configure
> > > new file mode 100644
> > > index 000000000000..347e2167dc62
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
> > > @@ -0,0 +1,82 @@
> > > +What:          /sys/class/scrub/
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               The scrub/ class subdirectory belongs to the
> > > +               scrubber subsystem.
> > > +
> > > +What:          /sys/class/scrub/scrubX/
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               The /sys/class/scrub/scrub{0,1,2,3,...} directories
> >
> > This API (sysfs interface) is very specific to the ACPI interface
> > defined for hardware patrol scrubber. I wonder can we have some
> > interface that is more generic, for a couple of reasons:
>
> Agreed that it makes sense to define a broad interface.  We have
> some hardware focused drivers we can't share yet (IP rules until
> a release date in the near future) where this is a reasonable fit
> - but indeed there are others such as mapping this to DDR ECS
> where it isn't a great mapping.
>
> I'd love to come up with an interface that has the right blend
> of generality and flexibility.  That is easiest done before we have
> any implementation merged.
>
> >
> > 1. I am not aware of any chip/platform hardware that implemented the
> > hw ps part defined in ACPI RASF/RAS2 spec. So I am curious what the
> > RAS experts from different hardware vendors think about this. For
> > example, Tony and Dave from Intel, Jon and Vilas from AMD. Is there
> > any hardware platform (if allowed to disclose) that implemented ACPI
> > RASF/RAS2? If so, will vendors continue to support the control of
> > patrol scrubber using the ACPI spec? If not (as Tony said in [1], will
> > the vendor consider starting some future platform?
> >
> > If we are unlikely to get the vendor support, creating this ACPI
> > specific sysfs API (and the driver implementations) in Linux seems to
> > have limited meaning.
>
> There is a bit of a chicken and egg problem here. Until there is
> reasonable support in kernel (or it looks like there will be),
> BIOS teams push back on a requirement to add the tables.
> I'd encourage no one to bother with RASF - RAS2 is much less
> ambiguous.

Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD (Jon
and Vilas) for your opinion on RAS2.

>
> >
> > > +               correspond to each scrub device.
> > > +
> > > +What:          /sys/class/scrub/scrubX/name
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               (RO) name of the memory scrub device
> > > +
> > > +What:          /sys/class/scrub/scrubX/regionY/
> >
> > 2. I believe the concept of "region" here is probably from
> > PATROL_SCRUB defined in “APCI section 5.2.20.5. Parameter Block". It
> > is indeed powerful: if a process's physical memory spans over multiple
> > memory controllers, OS can in theory scrub chunks of the memory
> > belonging to the process. However, from a previous discussion [1],
> > "From a h/w perspective it might always be complex". IIUC, the address
> > translation from physical address to channel address is hard to
> > achieve, and probably that's one of the tech reasons the patrol scrub
> > ACPI spec is not in practice implemented?
>
> Next bit is kind of an aside as I mostly agree with your conclusions ;)
>
> This obviously depends on your memory interleave. You want to present
> physical address ranges as single controllable regions - probably
> most interesting being stuff that maps to NUMA nodes.  The short
> answer is that any firmware control will end up writing to all the
> memory controllers involved in a given PA range - firmware can easily
> establish which those are.
>
> A memory controller can support multiple scrub regions
> which map from a PA range to a particular set of RAM addresses
> - that's implementation specific. The memory controller is getting
> the host PA and can carry out appropriate hashing if it wants to.
> Many scrub solutions won't do this - in which case it's max one
> region per memory controller (mapped by firmware to one region per
> interleave set - assuming interleave sets take in whole memory
> controllers - which they normally do).
>
> I would expect existing systems (not design with this differentiated
> scrub in mind) to only support scrub control by PA range at the
> granularity of interleave sets.
>
> Note that this general question of PA based controls also
> maps to things like resource control (resctl) where it's only interesting
> to partition memory bandwidth such that the partition applies to the
> whole interleave set - that's done for ARM systems anyway by having
> the userspace interface pretend there is only one control, but
> write the settings to all actual controllers involved. Not sure what
> x86 does.
>
> Taking a few examples that I know of.  All 4 socket server - with
> control of these as bios options ;).
> Assuming individual memory controllers don't allow scrub to be
> configured by PA range.
>
> 1. Full system memory interleave (people do this form of crazy)
>    In that case, there is only one set of firmware controls
>    that write to the interfaces of every memory controller.  Depending
>    on the interleave design that may still allow multiple regions.
>
> 2. Socket wide memory interleave.  In that case, firmware controls
>    need to effect all memory controllers in that socket if the
>    requested 'region' maps to them.  So 4 PA regions.
>
> 3. Die wide memory interleave.  Finer granularity of control
>    so perhaps 8 PA rgiones.
>
> 4. Finer granularity (there are reasons to do this for above mentioned
>    bandwidth resource control which you can only do if not interleaving
>    across multiple controllers).
>
>
>
> >
> > So my take is, control at the granularity of the memory controller is
> > probably a nice compromise.
> > Both OS and userspace can get a pretty
> > decent amount of flexibility, start/stop/adjust speed of the scrubbing
> > on a memory controller; meanwhile it doesn't impose too much pain to
> > hardware vendors when they provide these features in hardware. In
> > terms of how these controls/features will be implemented, I imagine it
> > could be implemented:
> > * via hardware registers that directly or indirectly control on memory
> > controllers
> > * via ACPI if the situation changes in 10 years (and the RASF/RAS2/PCC
> > drivers implemented in this patchset can be directly plugged into)
> > * a kernel-thread that uses cpu read to detect memory errors, if
> > hardware support is unavailable or not good enough
> >
>
> I more or less agree, but would tweak a little.
>
> 1) Allow for multiple regions per memory controller - that's needed
>    for RASF etc anyway - because as far as they are concerned there
>    is only one interface presented.
> 2) Tie the interface to interleave sets, not memory controllers.
>    NUMA nodes often being a good stand in for those.

Does you mean /sys/devices/system/node/nodeX/scrub, where scrub is a
virtual concept of scrubbing device that mapps to 1 or several
physical scrubber backends.

For example, starting/stopping the virtual device means issuing
START/STOP cmd to all backends. And...

>    Controlling memory controllers separately for parts of an interleave
>    isn't something I'd think was very useful.  This will probably get
>    messy in the future though and the complexity could be pushed to
>    a userspace tool - as long as enough info is available elsewhere
>    in sysfs.  So need those memory controller directories you propose
>    to include info on the PA ranges that they are involved in backing

is it acceptable if we don't provide PA range or region in the
interface *for now* if it complicates things a lot? I could be wrong,
but the user of scrubber seems would be ok with not being able to
scrub an arbitrary physical address range. In contrast, not knowing
the scrub results seems to be more annoying to users. So simply giving
some progress indicator like how many bytes a scrubber has scrubbed.

When we really need to support PA range or region, under the
/sys/devices/system/node/nodeX/scrub interface, it basically uses NUMA
node X's PA range. Then to scrub node memory in range [PA1, PA2), some
driver that understand all backends (or can talk to all backends'
drivers) needs to translate the PA into the address in backend's
address space, for example, [PA1, PA2) is mapped to 2 device ranges
[DA11, DA12) on backend_1 and [DA21, DA22) on backend_2.

>    (which to keep things horrible, can change at runtime via memory
>     hotplug and remapping of host physical address ranges on CXL etc)

CXL memory locally attached to the host is probably more or less the
same as normal physical memory. I wonder what it would be like for CXL
memory remotely attached through a memory pool. Does it make sense
that the controller/owner of the memory pool takes the responsibility
of controlling the CXL memory controller to control scrubbing? Does
the owner need to provide/mediate scrubbing support for other clients
using the memory pool?

Thanks,
Jiaqi

>
> > Given these possible backends of scrubbing, I think a more generic
> > sysfs API that covers and abstracts these backends will be more
> > valuable right now. I haven’t thought thoroughly, but how about
> > defining the top-level interface as something like
> > “/sys/devices/system/memory_controller_scrubX/”, or
> > “/sys/class/memory_controllerX/scrub”?
>
> No particular harm in the rename of the directory I guess.
> Though some of those 'memory_controllers' would be virtual as they
> wouldn't correspond to actual memory controllers but rather to
> sets of them.
>
> Jonathan
>
> >
> > [1] https://lore.kernel.org/linux-mm/SJ1PR11MB6083BF93E9A88E659CED5EC4FC3F9@SJ1PR11MB6083.namprd11.prod.outlook.com/T/#m13516ee35caa05b506080ae805bee14f9f958d43
>
> >
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               The /sys/class/scrub/scrubX/region{0,1,2,3,...}
> > > +               directories correspond to each scrub region under a scrub device.
> > > +               Scrub region is a physical address range for which scrub may be
> > > +               separately controlled. Regions may overlap in which case the
> > > +               scrubbing rate of the overlapped memory will be at least that
> > > +               expected due to each overlapping region.
> > > +
> > > +What:          /sys/class/scrub/scrubX/regionY/addr_base
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               (RW) The base of the address range of the memory region
> > > +               to be patrol scrubbed.
> > > +               On reading, returns the base of the memory region for
> > > +               the actual address range(The platform calculates
> > > +               the nearest patrol scrub boundary address from where
> > > +               it can start scrubbing).
> > > +
> > > +What:          /sys/class/scrub/scrubX/regionY/addr_size
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               (RW) The size of the address range to be patrol scrubbed.
> > > +               On reading, returns the size of the memory region for
> > > +               the actual address range.
> > > +
> > > +What:          /sys/class/scrub/scrubX/regionY/enable
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               (WO) Start/Stop scrubbing the memory region.
> > > +               1 - enable the memory scrubbing.
> > > +               0 - disable the memory scrubbing..
> > > +
> > > +What:          /sys/class/scrub/scrubX/regionY/speed_available
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               (RO) Supported range for the partol speed(scrub rate)
> > > +               by the scrubber for a memory region.
> > > +               The unit of the scrub rate vary depends on the scrubber.
> > > +
> > > +What:          /sys/class/scrub/scrubX/regionY/speed
> > > +Date:          September 2023
> > > +KernelVersion: 6.7
> > > +Contact:       linux-kernel@vger.kernel.org
> > > +Description:
> > > +               (RW) The partol speed(scrub rate) on the memory region specified and
> > > +               it must be with in the supported range by the scrubber.
> > > +               The unit of the scrub rate vary depends on the scrubber.
> > > --
> > > 2.34.1
> > >
> > >
> >
>
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-09-28  5:25       ` Jiaqi Yan
@ 2023-09-28 13:14         ` Jonathan Cameron
  2023-10-05  3:18         ` David Rientjes
  1 sibling, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2023-09-28 13:14 UTC (permalink / raw)
  To: Jiaqi Yan
  Cc: Luck, Tony, dave.hansen, jon.grimm, vilas.sridharan, linuxarm,
	shiju.jose, David Rientjes, linux-acpi, linux-mm, linux-kernel,
	rafael, lenb, naoya.horiguchi, james.morse, david, jthoughton,
	somasundaram.a, erdemaktas, pgonda, duenwen, mike.malvestuto,
	gthelen, tanxiaofei, prime.zeng, linux-cxl

On Wed, 27 Sep 2023 22:25:52 -0700
Jiaqi Yan <jiaqiyan@google.com> wrote:

> On Fri, Sep 22, 2023 at 3:20 AM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Thu, 21 Sep 2023 17:07:04 -0700
> > Jiaqi Yan <jiaqiyan@google.com> wrote:
> >  
> > > On Fri, Sep 15, 2023 at 10:29 AM <shiju.jose@huawei.com> wrote:  
> > > >
> > > > From: Shiju Jose <shiju.jose@huawei.com>
> > > >
> > > > Add sysfs documentation entries for the set of attributes those are
> > > > exposed in /sys/class/scrub/ by the scrub driver. These attributes
> > > > support configuring parameters of a scrub device.
> > > >
> > > > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> > > > ---
> > > >  .../ABI/testing/sysfs-class-scrub-configure   | 82 +++++++++++++++++++
> > > >  1 file changed, 82 insertions(+)
> > > >  create mode 100644 Documentation/ABI/testing/sysfs-class-scrub-configure
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-class-scrub-configure b/Documentation/ABI/testing/sysfs-class-scrub-configure
> > > > new file mode 100644
> > > > index 000000000000..347e2167dc62
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-class-scrub-configure
> > > > @@ -0,0 +1,82 @@
> > > > +What:          /sys/class/scrub/
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               The scrub/ class subdirectory belongs to the
> > > > +               scrubber subsystem.
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               The /sys/class/scrub/scrub{0,1,2,3,...} directories  
> > >
> > > This API (sysfs interface) is very specific to the ACPI interface
> > > defined for hardware patrol scrubber. I wonder can we have some
> > > interface that is more generic, for a couple of reasons:  
> >
> > Agreed that it makes sense to define a broad interface.  We have
> > some hardware focused drivers we can't share yet (IP rules until
> > a release date in the near future) where this is a reasonable fit
> > - but indeed there are others such as mapping this to DDR ECS
> > where it isn't a great mapping.
> >
> > I'd love to come up with an interface that has the right blend
> > of generality and flexibility.  That is easiest done before we have
> > any implementation merged.
> >  
> > >
> > > 1. I am not aware of any chip/platform hardware that implemented the
> > > hw ps part defined in ACPI RASF/RAS2 spec. So I am curious what the
> > > RAS experts from different hardware vendors think about this. For
> > > example, Tony and Dave from Intel, Jon and Vilas from AMD. Is there
> > > any hardware platform (if allowed to disclose) that implemented ACPI
> > > RASF/RAS2? If so, will vendors continue to support the control of
> > > patrol scrubber using the ACPI spec? If not (as Tony said in [1], will
> > > the vendor consider starting some future platform?
> > >
> > > If we are unlikely to get the vendor support, creating this ACPI
> > > specific sysfs API (and the driver implementations) in Linux seems to
> > > have limited meaning.  
> >
> > There is a bit of a chicken and egg problem here. Until there is
> > reasonable support in kernel (or it looks like there will be),
> > BIOS teams push back on a requirement to add the tables.
> > I'd encourage no one to bother with RASF - RAS2 is much less
> > ambiguous.  
> 
> Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD (Jon
> and Vilas) for your opinion on RAS2.
> 
> >  
> > >  
> > > > +               correspond to each scrub device.
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/name
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               (RO) name of the memory scrub device
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/regionY/  
> > >
> > > 2. I believe the concept of "region" here is probably from
> > > PATROL_SCRUB defined in “APCI section 5.2.20.5. Parameter Block". It
> > > is indeed powerful: if a process's physical memory spans over multiple
> > > memory controllers, OS can in theory scrub chunks of the memory
> > > belonging to the process. However, from a previous discussion [1],
> > > "From a h/w perspective it might always be complex". IIUC, the address
> > > translation from physical address to channel address is hard to
> > > achieve, and probably that's one of the tech reasons the patrol scrub
> > > ACPI spec is not in practice implemented?  
> >
> > Next bit is kind of an aside as I mostly agree with your conclusions ;)
> >
> > This obviously depends on your memory interleave. You want to present
> > physical address ranges as single controllable regions - probably
> > most interesting being stuff that maps to NUMA nodes.  The short
> > answer is that any firmware control will end up writing to all the
> > memory controllers involved in a given PA range - firmware can easily
> > establish which those are.
> >
> > A memory controller can support multiple scrub regions
> > which map from a PA range to a particular set of RAM addresses
> > - that's implementation specific. The memory controller is getting
> > the host PA and can carry out appropriate hashing if it wants to.
> > Many scrub solutions won't do this - in which case it's max one
> > region per memory controller (mapped by firmware to one region per
> > interleave set - assuming interleave sets take in whole memory
> > controllers - which they normally do).
> >
> > I would expect existing systems (not design with this differentiated
> > scrub in mind) to only support scrub control by PA range at the
> > granularity of interleave sets.
> >
> > Note that this general question of PA based controls also
> > maps to things like resource control (resctl) where it's only interesting
> > to partition memory bandwidth such that the partition applies to the
> > whole interleave set - that's done for ARM systems anyway by having
> > the userspace interface pretend there is only one control, but
> > write the settings to all actual controllers involved. Not sure what
> > x86 does.
> >
> > Taking a few examples that I know of.  All 4 socket server - with
> > control of these as bios options ;).
> > Assuming individual memory controllers don't allow scrub to be
> > configured by PA range.
> >
> > 1. Full system memory interleave (people do this form of crazy)
> >    In that case, there is only one set of firmware controls
> >    that write to the interfaces of every memory controller.  Depending
> >    on the interleave design that may still allow multiple regions.
> >
> > 2. Socket wide memory interleave.  In that case, firmware controls
> >    need to effect all memory controllers in that socket if the
> >    requested 'region' maps to them.  So 4 PA regions.
> >
> > 3. Die wide memory interleave.  Finer granularity of control
> >    so perhaps 8 PA rgiones.
> >
> > 4. Finer granularity (there are reasons to do this for above mentioned
> >    bandwidth resource control which you can only do if not interleaving
> >    across multiple controllers).
> >
> >
> >  
> > >
> > > So my take is, control at the granularity of the memory controller is
> > > probably a nice compromise.
> > > Both OS and userspace can get a pretty
> > > decent amount of flexibility, start/stop/adjust speed of the scrubbing
> > > on a memory controller; meanwhile it doesn't impose too much pain to
> > > hardware vendors when they provide these features in hardware. In
> > > terms of how these controls/features will be implemented, I imagine it
> > > could be implemented:
> > > * via hardware registers that directly or indirectly control on memory
> > > controllers
> > > * via ACPI if the situation changes in 10 years (and the RASF/RAS2/PCC
> > > drivers implemented in this patchset can be directly plugged into)
> > > * a kernel-thread that uses cpu read to detect memory errors, if
> > > hardware support is unavailable or not good enough
> > >  
> >
> > I more or less agree, but would tweak a little.
> >
> > 1) Allow for multiple regions per memory controller - that's needed
> >    for RASF etc anyway - because as far as they are concerned there
> >    is only one interface presented.
> > 2) Tie the interface to interleave sets, not memory controllers.
> >    NUMA nodes often being a good stand in for those.  
> 
> Does you mean /sys/devices/system/node/nodeX/scrub, where scrub is a
> virtual concept of scrubbing device that mapps to 1 or several
> physical scrubber backends.
> 
> For example, starting/stopping the virtual device means issuing
> START/STOP cmd to all backends. And...

That file location would work I think, though in some cases it
may get complex with single memory controllers covering parts of
different interleave sets. I guess mapping that mess is all a
software problem.


> 
> >    Controlling memory controllers separately for parts of an interleave
> >    isn't something I'd think was very useful.  This will probably get
> >    messy in the future though and the complexity could be pushed to
> >    a userspace tool - as long as enough info is available elsewhere
> >    in sysfs.  So need those memory controller directories you propose
> >    to include info on the PA ranges that they are involved in backing  
> 
> is it acceptable if we don't provide PA range or region in the
> interface *for now* if it complicates things a lot? I could be wrong,
> but the user of scrubber seems would be ok with not being able to
> scrub an arbitrary physical address range. In contrast, not knowing
> the scrub results seems to be more annoying to users. So simply giving
> some progress indicator like how many bytes a scrubber has scrubbed.

Progress is usually a non starter given it tends to be continuous in
the hardware controllers I've seen. So you control that it does it every
X hours, but doesn't provide a progress indicator.

I'd stil like PA range being there, but maybe RO fixed values.
Should be easy enough to establish. Also, won't matter if multiple
controllers identify same PA range, assumption would be control them all from
userspace, or use the NUMA type mapping above.

> 
> When we really need to support PA range or region, under the
> /sys/devices/system/node/nodeX/scrub interface, it basically uses NUMA
> node X's PA range. Then to scrub node memory in range [PA1, PA2), some
> driver that understand all backends (or can talk to all backends'
> drivers) needs to translate the PA into the address in backend's
> address space, for example, [PA1, PA2) is mapped to 2 device ranges
> [DA11, DA12) on backend_1 and [DA21, DA22) on backend_2.

Agreed.

> 
> >    (which to keep things horrible, can change at runtime via memory
> >     hotplug and remapping of host physical address ranges on CXL etc)  
> 
> CXL memory locally attached to the host is probably more or less the
> same as normal physical memory. I wonder what it would be like for CXL
> memory remotely attached through a memory pool. Does it make sense
> that the controller/owner of the memory pool takes the responsibility
> of controlling the CXL memory controller to control scrubbing? Does
> the owner need to provide/mediate scrubbing support for other clients
> using the memory pool?

Good and complex questions but lets leave those for a while ;)
beyond comprehending the fact that we can't always map interleave to
particular memory controllers as they may use only a subset of the
memory on that device. 

+CC'd linux-cxl for info, not because I want that discussion to go deep
at the moment.

Jonathan

> 
> Thanks,
> Jiaqi
> 
> >  
> > > Given these possible backends of scrubbing, I think a more generic
> > > sysfs API that covers and abstracts these backends will be more
> > > valuable right now. I haven’t thought thoroughly, but how about
> > > defining the top-level interface as something like
> > > “/sys/devices/system/memory_controller_scrubX/”, or
> > > “/sys/class/memory_controllerX/scrub”?  
> >
> > No particular harm in the rename of the directory I guess.
> > Though some of those 'memory_controllers' would be virtual as they
> > wouldn't correspond to actual memory controllers but rather to
> > sets of them.
> >
> > Jonathan
> >  
> > >
> > > [1] https://lore.kernel.org/linux-mm/SJ1PR11MB6083BF93E9A88E659CED5EC4FC3F9@SJ1PR11MB6083.namprd11.prod.outlook.com/T/#m13516ee35caa05b506080ae805bee14f9f958d43  
> >  
> > >  
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               The /sys/class/scrub/scrubX/region{0,1,2,3,...}
> > > > +               directories correspond to each scrub region under a scrub device.
> > > > +               Scrub region is a physical address range for which scrub may be
> > > > +               separately controlled. Regions may overlap in which case the
> > > > +               scrubbing rate of the overlapped memory will be at least that
> > > > +               expected due to each overlapping region.
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/regionY/addr_base
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               (RW) The base of the address range of the memory region
> > > > +               to be patrol scrubbed.
> > > > +               On reading, returns the base of the memory region for
> > > > +               the actual address range(The platform calculates
> > > > +               the nearest patrol scrub boundary address from where
> > > > +               it can start scrubbing).
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/regionY/addr_size
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               (RW) The size of the address range to be patrol scrubbed.
> > > > +               On reading, returns the size of the memory region for
> > > > +               the actual address range.
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/regionY/enable
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               (WO) Start/Stop scrubbing the memory region.
> > > > +               1 - enable the memory scrubbing.
> > > > +               0 - disable the memory scrubbing..
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/regionY/speed_available
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               (RO) Supported range for the partol speed(scrub rate)
> > > > +               by the scrubber for a memory region.
> > > > +               The unit of the scrub rate vary depends on the scrubber.
> > > > +
> > > > +What:          /sys/class/scrub/scrubX/regionY/speed
> > > > +Date:          September 2023
> > > > +KernelVersion: 6.7
> > > > +Contact:       linux-kernel@vger.kernel.org
> > > > +Description:
> > > > +               (RW) The partol speed(scrub rate) on the memory region specified and
> > > > +               it must be with in the supported range by the scrubber.
> > > > +               The unit of the scrub rate vary depends on the scrubber.
> > > > --
> > > > 2.34.1
> > > >
> > > >  
> > >  
> >
> >  
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-09-28  5:25       ` Jiaqi Yan
  2023-09-28 13:14         ` Jonathan Cameron
@ 2023-10-05  3:18         ` David Rientjes
  2023-10-06 13:02           ` Jonathan Cameron
  1 sibling, 1 reply; 32+ messages in thread
From: David Rientjes @ 2023-10-05  3:18 UTC (permalink / raw)
  To: Jiaqi Yan, Luck, Tony, Grimm, Jon, dave.hansen, vilas.sridharan
  Cc: Jonathan Cameron, linuxarm, shiju.jose, linux-acpi, linux-mm,
	linux-kernel, rafael, lenb, naoya.horiguchi, james.morse, david,
	jthoughton, somasundaram.a, erdemaktas, pgonda, duenwen,
	mike.malvestuto, gthelen, tanxiaofei, prime.zeng

On Wed, 27 Sep 2023, Jiaqi Yan wrote:

> > > 1. I am not aware of any chip/platform hardware that implemented the
> > > hw ps part defined in ACPI RASF/RAS2 spec. So I am curious what the
> > > RAS experts from different hardware vendors think about this. For
> > > example, Tony and Dave from Intel, Jon and Vilas from AMD. Is there
> > > any hardware platform (if allowed to disclose) that implemented ACPI
> > > RASF/RAS2? If so, will vendors continue to support the control of
> > > patrol scrubber using the ACPI spec? If not (as Tony said in [1], will
> > > the vendor consider starting some future platform?
> > >
> > > If we are unlikely to get the vendor support, creating this ACPI
> > > specific sysfs API (and the driver implementations) in Linux seems to
> > > have limited meaning.
> >
> > There is a bit of a chicken and egg problem here. Until there is
> > reasonable support in kernel (or it looks like there will be),
> > BIOS teams push back on a requirement to add the tables.
> > I'd encourage no one to bother with RASF - RAS2 is much less
> > ambiguous.
> 
> Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD (Jon
> and Vilas) for your opinion on RAS2.
> 

We'll need to know from vendors, ideally at minimum from both Intel and 
AMD, whether RAS2 is the long-term vision here.  Nothing is set in stone, 
of course, but deciding whether RAS2 is the standard that we should be 
rallying around will help to guide future development including in the 
kernel.

If RAS2 is insufficient for future use cases or we would need to support 
multiple implementations in the kernel for configuring the patrol scrubber 
depending on vendor, that's great feedback to have.

I'd much rather focus on implementing something in the kernel that we have 
some clarity about the vendors supporting, especially when it comes with 
user visible interfaces, as opposed to something that may not be used long 
term.  I think that's a fair ask and that vendor feedback is required 
here?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-05  3:18         ` David Rientjes
@ 2023-10-06 13:02           ` Jonathan Cameron
  2023-10-06 13:06             ` Sridharan, Vilas
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2023-10-06 13:02 UTC (permalink / raw)
  To: David Rientjes
  Cc: Jiaqi Yan, Luck, Tony, Grimm, Jon, dave.hansen, vilas.sridharan,
	linuxarm, shiju.jose, linux-acpi, linux-mm, linux-kernel, rafael,
	lenb, naoya.horiguchi, james.morse, david, jthoughton,
	somasundaram.a, erdemaktas, pgonda, duenwen, mike.malvestuto,
	gthelen, tanxiaofei, prime.zeng

On Wed, 4 Oct 2023 20:18:12 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Wed, 27 Sep 2023, Jiaqi Yan wrote:
> 
> > > > 1. I am not aware of any chip/platform hardware that implemented the
> > > > hw ps part defined in ACPI RASF/RAS2 spec. So I am curious what the
> > > > RAS experts from different hardware vendors think about this. For
> > > > example, Tony and Dave from Intel, Jon and Vilas from AMD. Is there
> > > > any hardware platform (if allowed to disclose) that implemented ACPI
> > > > RASF/RAS2? If so, will vendors continue to support the control of
> > > > patrol scrubber using the ACPI spec? If not (as Tony said in [1], will
> > > > the vendor consider starting some future platform?
> > > >
> > > > If we are unlikely to get the vendor support, creating this ACPI
> > > > specific sysfs API (and the driver implementations) in Linux seems to
> > > > have limited meaning.  
> > >
> > > There is a bit of a chicken and egg problem here. Until there is
> > > reasonable support in kernel (or it looks like there will be),
> > > BIOS teams push back on a requirement to add the tables.
> > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > ambiguous.  
> > 
> > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD (Jon
> > and Vilas) for your opinion on RAS2.
> >   
> 
> We'll need to know from vendors, ideally at minimum from both Intel and 
> AMD, whether RAS2 is the long-term vision here.  Nothing is set in stone, 
> of course, but deciding whether RAS2 is the standard that we should be 
> rallying around will help to guide future development including in the 
> kernel.
> 
> If RAS2 is insufficient for future use cases or we would need to support 
> multiple implementations in the kernel for configuring the patrol scrubber 
> depending on vendor, that's great feedback to have.
> 
> I'd much rather focus on implementing something in the kernel that we have 
> some clarity about the vendors supporting, especially when it comes with 
> user visible interfaces, as opposed to something that may not be used long 
> term.  I think that's a fair ask and that vendor feedback is required 
> here?

Agreed and happy to have feedback from Intel and AMD + all the other CPU
vendors who make use of ACPI + all the OEMs who add stuff well beyond what
Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the
kernel covers stuff not used on mainstream x86 platforms because they are
doing something custom and we didn't want 2 + X custom implementations...

Some other interfaces for scrub control (beyond existing embedded ones)
will surface in the next few months where RAS2 is not appropriate.

Jonathan



^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-06 13:02           ` Jonathan Cameron
@ 2023-10-06 13:06             ` Sridharan, Vilas
  2023-10-11 16:35               ` Jonathan Cameron
  0 siblings, 1 reply; 32+ messages in thread
From: Sridharan, Vilas @ 2023-10-06 13:06 UTC (permalink / raw)
  To: Jonathan Cameron, David Rientjes
  Cc: Jiaqi Yan, Luck, Tony, Grimm, Jon, dave.hansen, linuxarm,
	shiju.jose, linux-acpi, linux-mm, linux-kernel, rafael, lenb,
	naoya.horiguchi, james.morse, david, jthoughton, somasundaram.a,
	erdemaktas, pgonda, duenwen, mike.malvestuto, gthelen,
	tanxiaofei, prime.zeng

[AMD Official Use Only - General]

I do not believe AMD has implemented RASF/RAS2 at all.

We are looking at it, but our initial impression is that it is insufficiently flexible for general use. (Not just for this feature, but for others in the future.)

    -Vilas

-----Original Message-----
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Sent: Friday, October 6, 2023 9:02 AM
To: David Rientjes <rientjes@google.com>
Cc: Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; Sridharan, Vilas <Vilas.Sridharan@amd.com>; linuxarm@huawei.com; shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com; gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Wed, 4 Oct 2023 20:18:12 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Wed, 27 Sep 2023, Jiaqi Yan wrote:
>
> > > > 1. I am not aware of any chip/platform hardware that implemented
> > > > the hw ps part defined in ACPI RASF/RAS2 spec. So I am curious
> > > > what the RAS experts from different hardware vendors think about
> > > > this. For example, Tony and Dave from Intel, Jon and Vilas from
> > > > AMD. Is there any hardware platform (if allowed to disclose)
> > > > that implemented ACPI RASF/RAS2? If so, will vendors continue to
> > > > support the control of patrol scrubber using the ACPI spec? If
> > > > not (as Tony said in [1], will the vendor consider starting some future platform?
> > > >
> > > > If we are unlikely to get the vendor support, creating this ACPI
> > > > specific sysfs API (and the driver implementations) in Linux
> > > > seems to have limited meaning.
> > >
> > > There is a bit of a chicken and egg problem here. Until there is
> > > reasonable support in kernel (or it looks like there will be),
> > > BIOS teams push back on a requirement to add the tables.
> > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > ambiguous.
> >
> > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD
> > (Jon and Vilas) for your opinion on RAS2.
> >
>
> We'll need to know from vendors, ideally at minimum from both Intel
> and AMD, whether RAS2 is the long-term vision here.  Nothing is set in
> stone, of course, but deciding whether RAS2 is the standard that we
> should be rallying around will help to guide future development
> including in the kernel.
>
> If RAS2 is insufficient for future use cases or we would need to
> support multiple implementations in the kernel for configuring the
> patrol scrubber depending on vendor, that's great feedback to have.
>
> I'd much rather focus on implementing something in the kernel that we
> have some clarity about the vendors supporting, especially when it
> comes with user visible interfaces, as opposed to something that may
> not be used long term.  I think that's a fair ask and that vendor
> feedback is required here?

Agreed and happy to have feedback from Intel and AMD + all the other CPU vendors who make use of ACPI + all the OEMs who add stuff well beyond what Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the kernel covers stuff not used on mainstream x86 platforms because they are doing something custom and we didn't want 2 + X custom implementations...

Some other interfaces for scrub control (beyond existing embedded ones) will surface in the next few months where RAS2 is not appropriate.

Jonathan



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-06 13:06             ` Sridharan, Vilas
@ 2023-10-11 16:35               ` Jonathan Cameron
  2023-10-12 13:41                 ` Sridharan, Vilas
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2023-10-11 16:35 UTC (permalink / raw)
  To: Sridharan, Vilas
  Cc: David Rientjes, Jiaqi Yan, Luck, Tony, Grimm, Jon, dave.hansen,
	linuxarm, shiju.jose, linux-acpi, linux-mm, linux-kernel, rafael,
	lenb, naoya.horiguchi, james.morse, david, jthoughton,
	somasundaram.a, erdemaktas, pgonda, duenwen, mike.malvestuto,
	gthelen, tanxiaofei, prime.zeng

On Fri, 6 Oct 2023 13:06:53 +0000
"Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:

> [AMD Official Use Only - General]
> 
> I do not believe AMD has implemented RASF/RAS2 at all.
> 
> We are looking at it, but our initial impression is that it is insufficiently flexible for general use. (Not just for this feature, but for others in the future.)
> 
>     -Vilas

Hi Vilas,

So obvious question is - worth fixing?

I'm not particularly keen to see 10+ different ways of meeting this requirement.

Probably not too bad if that's 10+ drivers implementing the same userspace ABI, but
definitely don't want 10 drivers and 10 ABIs.

Jonathan

> 
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Friday, October 6, 2023 9:02 AM
> To: David Rientjes <rientjes@google.com>
> Cc: Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; Sridharan, Vilas <Vilas.Sridharan@amd.com>; linuxarm@huawei.com; shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com; gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Wed, 4 Oct 2023 20:18:12 -0700 (PDT)
> David Rientjes <rientjes@google.com> wrote:
> 
> > On Wed, 27 Sep 2023, Jiaqi Yan wrote:
> >  
> > > > > 1. I am not aware of any chip/platform hardware that implemented
> > > > > the hw ps part defined in ACPI RASF/RAS2 spec. So I am curious
> > > > > what the RAS experts from different hardware vendors think about
> > > > > this. For example, Tony and Dave from Intel, Jon and Vilas from
> > > > > AMD. Is there any hardware platform (if allowed to disclose)
> > > > > that implemented ACPI RASF/RAS2? If so, will vendors continue to
> > > > > support the control of patrol scrubber using the ACPI spec? If
> > > > > not (as Tony said in [1], will the vendor consider starting some future platform?
> > > > >
> > > > > If we are unlikely to get the vendor support, creating this ACPI
> > > > > specific sysfs API (and the driver implementations) in Linux
> > > > > seems to have limited meaning.  
> > > >
> > > > There is a bit of a chicken and egg problem here. Until there is
> > > > reasonable support in kernel (or it looks like there will be),
> > > > BIOS teams push back on a requirement to add the tables.
> > > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > > ambiguous.  
> > >
> > > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD
> > > (Jon and Vilas) for your opinion on RAS2.
> > >  
> >
> > We'll need to know from vendors, ideally at minimum from both Intel
> > and AMD, whether RAS2 is the long-term vision here.  Nothing is set in
> > stone, of course, but deciding whether RAS2 is the standard that we
> > should be rallying around will help to guide future development
> > including in the kernel.
> >
> > If RAS2 is insufficient for future use cases or we would need to
> > support multiple implementations in the kernel for configuring the
> > patrol scrubber depending on vendor, that's great feedback to have.
> >
> > I'd much rather focus on implementing something in the kernel that we
> > have some clarity about the vendors supporting, especially when it
> > comes with user visible interfaces, as opposed to something that may
> > not be used long term.  I think that's a fair ask and that vendor
> > feedback is required here?  
> 
> Agreed and happy to have feedback from Intel and AMD + all the other CPU vendors who make use of ACPI + all the OEMs who add stuff well beyond what Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the kernel covers stuff not used on mainstream x86 platforms because they are doing something custom and we didn't want 2 + X custom implementations...
> 
> Some other interfaces for scrub control (beyond existing embedded ones) will surface in the next few months where RAS2 is not appropriate.
> 
> Jonathan
> 
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-11 16:35               ` Jonathan Cameron
@ 2023-10-12 13:41                 ` Sridharan, Vilas
  2023-10-12 15:02                   ` Jonathan Cameron
  0 siblings, 1 reply; 32+ messages in thread
From: Sridharan, Vilas @ 2023-10-12 13:41 UTC (permalink / raw)
  To: Jonathan Cameron, Duran, Leo, Ghannam, Yazen
  Cc: David Rientjes, Jiaqi Yan, Luck, Tony, Grimm, Jon, dave.hansen,
	linuxarm, shiju.jose, linux-acpi, linux-mm, linux-kernel, rafael,
	lenb, naoya.horiguchi, james.morse, david, jthoughton,
	somasundaram.a, erdemaktas, pgonda, duenwen, mike.malvestuto,
	gthelen, tanxiaofei, prime.zeng

[AMD Official Use Only - General]

+ Leo and Yazen

We looked at RASF and RAS2 again. We don't think RASF is worth fixing. Our preference is to coalesce around RAS2 because we think it can be extended in interesting ways.

The patrol scrub function probably needs some changes to be more general across different types of hardware (there are some baked-in assumptions that don't always hold true).

We will look at some spec changes to fix the patrol scrub function, and we are going to start thinking about other functions that can be added to RAS2.

    -Vilas

-----Original Message-----
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Sent: Wednesday, October 11, 2023 12:36 PM
To: Sridharan, Vilas <Vilas.Sridharan@amd.com>
Cc: David Rientjes <rientjes@google.com>; Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; linuxarm@huawei.com; shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com; gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Fri, 6 Oct 2023 13:06:53 +0000
"Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:

> [AMD Official Use Only - General]
>
> I do not believe AMD has implemented RASF/RAS2 at all.
>
> We are looking at it, but our initial impression is that it is
> insufficiently flexible for general use. (Not just for this feature,
> but for others in the future.)
>
>     -Vilas

Hi Vilas,

So obvious question is - worth fixing?

I'm not particularly keen to see 10+ different ways of meeting this requirement.

Probably not too bad if that's 10+ drivers implementing the same userspace ABI, but definitely don't want 10 drivers and 10 ABIs.

Jonathan

>
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Friday, October 6, 2023 9:02 AM
> To: David Rientjes <rientjes@google.com>
> Cc: Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>;
> Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com;
> Sridharan, Vilas <Vilas.Sridharan@amd.com>; linuxarm@huawei.com;
> shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org;
> linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com;
> jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com;
> pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com;
> gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation
> entries for set of scrub attributes
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Wed, 4 Oct 2023 20:18:12 -0700 (PDT) David Rientjes
> <rientjes@google.com> wrote:
>
> > On Wed, 27 Sep 2023, Jiaqi Yan wrote:
> >
> > > > > 1. I am not aware of any chip/platform hardware that
> > > > > implemented the hw ps part defined in ACPI RASF/RAS2 spec. So
> > > > > I am curious what the RAS experts from different hardware
> > > > > vendors think about this. For example, Tony and Dave from
> > > > > Intel, Jon and Vilas from AMD. Is there any hardware platform
> > > > > (if allowed to disclose) that implemented ACPI RASF/RAS2? If
> > > > > so, will vendors continue to support the control of patrol
> > > > > scrubber using the ACPI spec? If not (as Tony said in [1], will the vendor consider starting some future platform?
> > > > >
> > > > > If we are unlikely to get the vendor support, creating this
> > > > > ACPI specific sysfs API (and the driver implementations) in
> > > > > Linux seems to have limited meaning.
> > > >
> > > > There is a bit of a chicken and egg problem here. Until there is
> > > > reasonable support in kernel (or it looks like there will be),
> > > > BIOS teams push back on a requirement to add the tables.
> > > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > > ambiguous.
> > >
> > > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD
> > > (Jon and Vilas) for your opinion on RAS2.
> > >
> >
> > We'll need to know from vendors, ideally at minimum from both Intel
> > and AMD, whether RAS2 is the long-term vision here.  Nothing is set
> > in stone, of course, but deciding whether RAS2 is the standard that
> > we should be rallying around will help to guide future development
> > including in the kernel.
> >
> > If RAS2 is insufficient for future use cases or we would need to
> > support multiple implementations in the kernel for configuring the
> > patrol scrubber depending on vendor, that's great feedback to have.
> >
> > I'd much rather focus on implementing something in the kernel that
> > we have some clarity about the vendors supporting, especially when
> > it comes with user visible interfaces, as opposed to something that
> > may not be used long term.  I think that's a fair ask and that
> > vendor feedback is required here?
>
> Agreed and happy to have feedback from Intel and AMD + all the other CPU vendors who make use of ACPI + all the OEMs who add stuff well beyond what Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the kernel covers stuff not used on mainstream x86 platforms because they are doing something custom and we didn't want 2 + X custom implementations...
>
> Some other interfaces for scrub control (beyond existing embedded ones) will surface in the next few months where RAS2 is not appropriate.
>
> Jonathan
>
>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-12 13:41                 ` Sridharan, Vilas
@ 2023-10-12 15:02                   ` Jonathan Cameron
  2023-10-12 15:44                     ` Sridharan, Vilas
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2023-10-12 15:02 UTC (permalink / raw)
  To: Sridharan, Vilas
  Cc: Duran, Leo, Ghannam, Yazen, David Rientjes, Jiaqi Yan, Luck,
	Tony, Grimm, Jon, dave.hansen, linuxarm, shiju.jose, linux-acpi,
	linux-mm, linux-kernel, rafael, lenb, naoya.horiguchi,
	james.morse, david, jthoughton, somasundaram.a, erdemaktas,
	pgonda, duenwen, mike.malvestuto, gthelen, tanxiaofei,
	prime.zeng, kangkang.shen, wanghuiqiang

On Thu, 12 Oct 2023 13:41:19 +0000
"Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:

> [AMD Official Use Only - General]
> 
> + Leo and Yazen

Hi All.

+ Kangkang and Wanghuiqiang (Henson),

> 
> We looked at RASF and RAS2 again. We don't think RASF is worth fixing. Our preference is to coalesce around RAS2 because we think it can be extended in interesting ways.

Absolutely agree. I'm guessing RAS2 was previous go at fixing RASF though I haven't
done the archaeology.

> 
> The patrol scrub function probably needs some changes to be more general across different types of hardware (there are some baked-in assumptions that don't always hold true).

Agreed. One aspect I'd love to see improved is expanded discoverability of what
the hardware can do.

> 
> We will look at some spec changes to fix the patrol scrub function, and we are going to start thinking about other functions that can be added to RAS2.

Feel free to reach out if you want some early input on this. Are you thinking
a code first proposal?  If you think doing this through the standards body is a good idea
then perhaps message back here so we know when to look for further proposals in mantis.

Thanks,

Jonathan
> 
>     -Vilas
> 
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Wednesday, October 11, 2023 12:36 PM
> To: Sridharan, Vilas <Vilas.Sridharan@amd.com>
> Cc: David Rientjes <rientjes@google.com>; Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; linuxarm@huawei.com; shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com; gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, 6 Oct 2023 13:06:53 +0000
> "Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:
> 
> > [AMD Official Use Only - General]
> >
> > I do not believe AMD has implemented RASF/RAS2 at all.
> >
> > We are looking at it, but our initial impression is that it is
> > insufficiently flexible for general use. (Not just for this feature,
> > but for others in the future.)
> >
> >     -Vilas  
> 
> Hi Vilas,
> 
> So obvious question is - worth fixing?
> 
> I'm not particularly keen to see 10+ different ways of meeting this requirement.
> 
> Probably not too bad if that's 10+ drivers implementing the same userspace ABI, but definitely don't want 10 drivers and 10 ABIs.
> 
> Jonathan
> 
> >
> > -----Original Message-----
> > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > Sent: Friday, October 6, 2023 9:02 AM
> > To: David Rientjes <rientjes@google.com>
> > Cc: Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>;
> > Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com;
> > Sridharan, Vilas <Vilas.Sridharan@amd.com>; linuxarm@huawei.com;
> > shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org;
> > linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> > naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com;
> > jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com;
> > pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com;
> > gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> > Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation
> > entries for set of scrub attributes
> >
> > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> >
> >
> > On Wed, 4 Oct 2023 20:18:12 -0700 (PDT) David Rientjes
> > <rientjes@google.com> wrote:
> >  
> > > On Wed, 27 Sep 2023, Jiaqi Yan wrote:
> > >  
> > > > > > 1. I am not aware of any chip/platform hardware that
> > > > > > implemented the hw ps part defined in ACPI RASF/RAS2 spec. So
> > > > > > I am curious what the RAS experts from different hardware
> > > > > > vendors think about this. For example, Tony and Dave from
> > > > > > Intel, Jon and Vilas from AMD. Is there any hardware platform
> > > > > > (if allowed to disclose) that implemented ACPI RASF/RAS2? If
> > > > > > so, will vendors continue to support the control of patrol
> > > > > > scrubber using the ACPI spec? If not (as Tony said in [1], will the vendor consider starting some future platform?
> > > > > >
> > > > > > If we are unlikely to get the vendor support, creating this
> > > > > > ACPI specific sysfs API (and the driver implementations) in
> > > > > > Linux seems to have limited meaning.  
> > > > >
> > > > > There is a bit of a chicken and egg problem here. Until there is
> > > > > reasonable support in kernel (or it looks like there will be),
> > > > > BIOS teams push back on a requirement to add the tables.
> > > > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > > > ambiguous.  
> > > >
> > > > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD
> > > > (Jon and Vilas) for your opinion on RAS2.
> > > >  
> > >
> > > We'll need to know from vendors, ideally at minimum from both Intel
> > > and AMD, whether RAS2 is the long-term vision here.  Nothing is set
> > > in stone, of course, but deciding whether RAS2 is the standard that
> > > we should be rallying around will help to guide future development
> > > including in the kernel.
> > >
> > > If RAS2 is insufficient for future use cases or we would need to
> > > support multiple implementations in the kernel for configuring the
> > > patrol scrubber depending on vendor, that's great feedback to have.
> > >
> > > I'd much rather focus on implementing something in the kernel that
> > > we have some clarity about the vendors supporting, especially when
> > > it comes with user visible interfaces, as opposed to something that
> > > may not be used long term.  I think that's a fair ask and that
> > > vendor feedback is required here?  
> >
> > Agreed and happy to have feedback from Intel and AMD + all the other CPU vendors who make use of ACPI + all the OEMs who add stuff well beyond what Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the kernel covers stuff not used on mainstream x86 platforms because they are doing something custom and we didn't want 2 + X custom implementations...
> >
> > Some other interfaces for scrub control (beyond existing embedded ones) will surface in the next few months where RAS2 is not appropriate.
> >
> > Jonathan
> >
> >  
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-12 15:02                   ` Jonathan Cameron
@ 2023-10-12 15:44                     ` Sridharan, Vilas
  2023-10-13  9:07                       ` Jonathan Cameron
  0 siblings, 1 reply; 32+ messages in thread
From: Sridharan, Vilas @ 2023-10-12 15:44 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Duran, Leo, Ghannam, Yazen, David Rientjes, Jiaqi Yan, Luck,
	Tony, Grimm, Jon, dave.hansen, linuxarm, shiju.jose, linux-acpi,
	linux-mm, linux-kernel, rafael, lenb, naoya.horiguchi,
	james.morse, david, jthoughton, somasundaram.a, erdemaktas,
	pgonda, duenwen, mike.malvestuto, gthelen, tanxiaofei,
	prime.zeng, kangkang.shen, wanghuiqiang

[AMD Official Use Only - General]

> Are you thinking a code first proposal?  If you think doing this through the standards body is a good idea then perhaps message back here so we know when to look for further proposals in mantis.

I am not super familiar with what you mean by 'code first proposal', but we are thinking about crafting an ECN (or a set of ECNs) for ACPI, that will be made public through ACPI's normal process.

    -Vilas

-----Original Message-----
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Sent: Thursday, October 12, 2023 11:02 AM
To: Sridharan, Vilas <Vilas.Sridharan@amd.com>
Cc: Duran, Leo <leo.duran@amd.com>; Ghannam, Yazen <Yazen.Ghannam@amd.com>; David Rientjes <rientjes@google.com>; Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; linuxarm@huawei.com; shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com; gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com; kangkang.shen@futurewei.com; wanghuiqiang@huawei.com
Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Thu, 12 Oct 2023 13:41:19 +0000
"Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:

> [AMD Official Use Only - General]
>
> + Leo and Yazen

Hi All.

+ Kangkang and Wanghuiqiang (Henson),

>
> We looked at RASF and RAS2 again. We don't think RASF is worth fixing. Our preference is to coalesce around RAS2 because we think it can be extended in interesting ways.

Absolutely agree. I'm guessing RAS2 was previous go at fixing RASF though I haven't done the archaeology.

>
> The patrol scrub function probably needs some changes to be more general across different types of hardware (there are some baked-in assumptions that don't always hold true).

Agreed. One aspect I'd love to see improved is expanded discoverability of what the hardware can do.

>
> We will look at some spec changes to fix the patrol scrub function, and we are going to start thinking about other functions that can be added to RAS2.

Feel free to reach out if you want some early input on this. Are you thinking a code first proposal?  If you think doing this through the standards body is a good idea then perhaps message back here so we know when to look for further proposals in mantis.

Thanks,

Jonathan
>
>     -Vilas
>
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Wednesday, October 11, 2023 12:36 PM
> To: Sridharan, Vilas <Vilas.Sridharan@amd.com>
> Cc: David Rientjes <rientjes@google.com>; Jiaqi Yan
> <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon
> <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; linuxarm@huawei.com;
> shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org;
> linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com;
> jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com;
> pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com;
> gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation
> entries for set of scrub attributes
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Fri, 6 Oct 2023 13:06:53 +0000
> "Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:
>
> > [AMD Official Use Only - General]
> >
> > I do not believe AMD has implemented RASF/RAS2 at all.
> >
> > We are looking at it, but our initial impression is that it is
> > insufficiently flexible for general use. (Not just for this feature,
> > but for others in the future.)
> >
> >     -Vilas
>
> Hi Vilas,
>
> So obvious question is - worth fixing?
>
> I'm not particularly keen to see 10+ different ways of meeting this requirement.
>
> Probably not too bad if that's 10+ drivers implementing the same userspace ABI, but definitely don't want 10 drivers and 10 ABIs.
>
> Jonathan
>
> >
> > -----Original Message-----
> > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > Sent: Friday, October 6, 2023 9:02 AM
> > To: David Rientjes <rientjes@google.com>
> > Cc: Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony
> > <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>;
> > dave.hansen@linux.intel.com; Sridharan, Vilas
> > <Vilas.Sridharan@amd.com>; linuxarm@huawei.com;
> > shiju.jose@huawei.com; linux-acpi@vger.kernel.org;
> > linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org;
> > lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com;
> > david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com;
> > erdemaktas@google.com; pgonda@google.com; duenwen@google.com;
> > mike.malvestuto@intel.com; gthelen@google.com;
> > tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> > Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation
> > entries for set of scrub attributes
> >
> > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> >
> >
> > On Wed, 4 Oct 2023 20:18:12 -0700 (PDT) David Rientjes
> > <rientjes@google.com> wrote:
> >
> > > On Wed, 27 Sep 2023, Jiaqi Yan wrote:
> > >
> > > > > > 1. I am not aware of any chip/platform hardware that
> > > > > > implemented the hw ps part defined in ACPI RASF/RAS2 spec.
> > > > > > So I am curious what the RAS experts from different hardware
> > > > > > vendors think about this. For example, Tony and Dave from
> > > > > > Intel, Jon and Vilas from AMD. Is there any hardware
> > > > > > platform (if allowed to disclose) that implemented ACPI
> > > > > > RASF/RAS2? If so, will vendors continue to support the
> > > > > > control of patrol scrubber using the ACPI spec? If not (as Tony said in [1], will the vendor consider starting some future platform?
> > > > > >
> > > > > > If we are unlikely to get the vendor support, creating this
> > > > > > ACPI specific sysfs API (and the driver implementations) in
> > > > > > Linux seems to have limited meaning.
> > > > >
> > > > > There is a bit of a chicken and egg problem here. Until there
> > > > > is reasonable support in kernel (or it looks like there will
> > > > > be), BIOS teams push back on a requirement to add the tables.
> > > > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > > > ambiguous.
> > > >
> > > > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD
> > > > (Jon and Vilas) for your opinion on RAS2.
> > > >
> > >
> > > We'll need to know from vendors, ideally at minimum from both
> > > Intel and AMD, whether RAS2 is the long-term vision here.  Nothing
> > > is set in stone, of course, but deciding whether RAS2 is the
> > > standard that we should be rallying around will help to guide
> > > future development including in the kernel.
> > >
> > > If RAS2 is insufficient for future use cases or we would need to
> > > support multiple implementations in the kernel for configuring the
> > > patrol scrubber depending on vendor, that's great feedback to have.
> > >
> > > I'd much rather focus on implementing something in the kernel that
> > > we have some clarity about the vendors supporting, especially when
> > > it comes with user visible interfaces, as opposed to something
> > > that may not be used long term.  I think that's a fair ask and
> > > that vendor feedback is required here?
> >
> > Agreed and happy to have feedback from Intel and AMD + all the other CPU vendors who make use of ACPI + all the OEMs who add stuff well beyond what Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the kernel covers stuff not used on mainstream x86 platforms because they are doing something custom and we didn't want 2 + X custom implementations...
> >
> > Some other interfaces for scrub control (beyond existing embedded ones) will surface in the next few months where RAS2 is not appropriate.
> >
> > Jonathan
> >
> >
>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
  2023-10-12 15:44                     ` Sridharan, Vilas
@ 2023-10-13  9:07                       ` Jonathan Cameron
  0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2023-10-13  9:07 UTC (permalink / raw)
  To: Sridharan, Vilas
  Cc: Duran, Leo, Ghannam, Yazen, David Rientjes, Jiaqi Yan, Luck,
	Tony, Grimm, Jon, dave.hansen, linuxarm, shiju.jose, linux-acpi,
	linux-mm, linux-kernel, rafael, lenb, naoya.horiguchi,
	james.morse, david, jthoughton, somasundaram.a, erdemaktas,
	pgonda, duenwen, mike.malvestuto, gthelen, tanxiaofei,
	prime.zeng, kangkang.shen, wanghuiqiang

On Thu, 12 Oct 2023 15:44:18 +0000
"Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:

> [AMD Official Use Only - General]
> 
> > Are you thinking a code first proposal?  If you think doing this through the standards body is a good idea then perhaps message back here so we know when to look for further proposals in mantis.  
> 
> I am not super familiar with what you mean by 'code first proposal', but we are thinking about crafting an ECN (or a set of ECNs) for ACPI, that will be made public through ACPI's normal process.
> 
There are two ways to go about getting an ECN into the specification
and which one is chosen affects the 'made it public' part of the ECN.

One is public from the start and is done via a proposal submitted to the Specification
Updates section of the tianocore bugzilla. This is referred to as "code
first", but actually just means the request came from discussions initially
had outside of the UEFI forum. They are still discussed in ASWG, but review
also occurs in public on the bugzilla.
https://bugzilla.tianocore.org/buglist.cgi?component=Specification%20Update&product=EDK2%20Code%20First&resolution=---

The other is the more traditional method of proposing in private. There the issue
is that the review is limited to those who both engage closely with ASWG and those
who can remember their mantis password. Before we post any software based on changes
going via that route (as its covered by UEFI forum IP rules) we have to wait for a
formal specification release.  So basically the traditional method is typically slower
and doesn't let us do helpful things like ask the kernel community to review the
proposed changes. The code first route was added a few years ago to provide the
options for companies that preferred the flexibility and openness it provides.

As you can see from the link above, there is a lot of activity via the code
first route these days.

Jonathan


>     -Vilas
> 
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Sent: Thursday, October 12, 2023 11:02 AM
> To: Sridharan, Vilas <Vilas.Sridharan@amd.com>
> Cc: Duran, Leo <leo.duran@amd.com>; Ghannam, Yazen <Yazen.Ghannam@amd.com>; David Rientjes <rientjes@google.com>; Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; linuxarm@huawei.com; shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com; pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com; gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com; kangkang.shen@futurewei.com; wanghuiqiang@huawei.com
> Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Thu, 12 Oct 2023 13:41:19 +0000
> "Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:
> 
> > [AMD Official Use Only - General]
> >
> > + Leo and Yazen  
> 
> Hi All.
> 
> + Kangkang and Wanghuiqiang (Henson),
> 
> >
> > We looked at RASF and RAS2 again. We don't think RASF is worth fixing. Our preference is to coalesce around RAS2 because we think it can be extended in interesting ways.  
> 
> Absolutely agree. I'm guessing RAS2 was previous go at fixing RASF though I haven't done the archaeology.
> 
> >
> > The patrol scrub function probably needs some changes to be more general across different types of hardware (there are some baked-in assumptions that don't always hold true).  
> 
> Agreed. One aspect I'd love to see improved is expanded discoverability of what the hardware can do.
> 
> >
> > We will look at some spec changes to fix the patrol scrub function, and we are going to start thinking about other functions that can be added to RAS2.  
> 
> Feel free to reach out if you want some early input on this. Are you thinking a code first proposal?  If you think doing this through the standards body is a good idea then perhaps message back here so we know when to look for further proposals in mantis.
> 
> Thanks,
> 
> Jonathan
> >
> >     -Vilas
> >
> > -----Original Message-----
> > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > Sent: Wednesday, October 11, 2023 12:36 PM
> > To: Sridharan, Vilas <Vilas.Sridharan@amd.com>
> > Cc: David Rientjes <rientjes@google.com>; Jiaqi Yan
> > <jiaqiyan@google.com>; Luck, Tony <tony.luck@intel.com>; Grimm, Jon
> > <Jon.Grimm@amd.com>; dave.hansen@linux.intel.com; linuxarm@huawei.com;
> > shiju.jose@huawei.com; linux-acpi@vger.kernel.org; linux-mm@kvack.org;
> > linux-kernel@vger.kernel.org; rafael@kernel.org; lenb@kernel.org;
> > naoya.horiguchi@nec.com; james.morse@arm.com; david@redhat.com;
> > jthoughton@google.com; somasundaram.a@hpe.com; erdemaktas@google.com;
> > pgonda@google.com; duenwen@google.com; mike.malvestuto@intel.com;
> > gthelen@google.com; tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> > Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation
> > entries for set of scrub attributes
> >
> > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> >
> >
> > On Fri, 6 Oct 2023 13:06:53 +0000
> > "Sridharan, Vilas" <Vilas.Sridharan@amd.com> wrote:
> >  
> > > [AMD Official Use Only - General]
> > >
> > > I do not believe AMD has implemented RASF/RAS2 at all.
> > >
> > > We are looking at it, but our initial impression is that it is
> > > insufficiently flexible for general use. (Not just for this feature,
> > > but for others in the future.)
> > >
> > >     -Vilas  
> >
> > Hi Vilas,
> >
> > So obvious question is - worth fixing?
> >
> > I'm not particularly keen to see 10+ different ways of meeting this requirement.
> >
> > Probably not too bad if that's 10+ drivers implementing the same userspace ABI, but definitely don't want 10 drivers and 10 ABIs.
> >
> > Jonathan
> >  
> > >
> > > -----Original Message-----
> > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > > Sent: Friday, October 6, 2023 9:02 AM
> > > To: David Rientjes <rientjes@google.com>
> > > Cc: Jiaqi Yan <jiaqiyan@google.com>; Luck, Tony
> > > <tony.luck@intel.com>; Grimm, Jon <Jon.Grimm@amd.com>;
> > > dave.hansen@linux.intel.com; Sridharan, Vilas
> > > <Vilas.Sridharan@amd.com>; linuxarm@huawei.com;
> > > shiju.jose@huawei.com; linux-acpi@vger.kernel.org;
> > > linux-mm@kvack.org; linux-kernel@vger.kernel.org; rafael@kernel.org;
> > > lenb@kernel.org; naoya.horiguchi@nec.com; james.morse@arm.com;
> > > david@redhat.com; jthoughton@google.com; somasundaram.a@hpe.com;
> > > erdemaktas@google.com; pgonda@google.com; duenwen@google.com;
> > > mike.malvestuto@intel.com; gthelen@google.com;
> > > tanxiaofei@huawei.com; prime.zeng@hisilicon.com
> > > Subject: Re: [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation
> > > entries for set of scrub attributes
> > >
> > > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > On Wed, 4 Oct 2023 20:18:12 -0700 (PDT) David Rientjes
> > > <rientjes@google.com> wrote:
> > >  
> > > > On Wed, 27 Sep 2023, Jiaqi Yan wrote:
> > > >  
> > > > > > > 1. I am not aware of any chip/platform hardware that
> > > > > > > implemented the hw ps part defined in ACPI RASF/RAS2 spec.
> > > > > > > So I am curious what the RAS experts from different hardware
> > > > > > > vendors think about this. For example, Tony and Dave from
> > > > > > > Intel, Jon and Vilas from AMD. Is there any hardware
> > > > > > > platform (if allowed to disclose) that implemented ACPI
> > > > > > > RASF/RAS2? If so, will vendors continue to support the
> > > > > > > control of patrol scrubber using the ACPI spec? If not (as Tony said in [1], will the vendor consider starting some future platform?
> > > > > > >
> > > > > > > If we are unlikely to get the vendor support, creating this
> > > > > > > ACPI specific sysfs API (and the driver implementations) in
> > > > > > > Linux seems to have limited meaning.  
> > > > > >
> > > > > > There is a bit of a chicken and egg problem here. Until there
> > > > > > is reasonable support in kernel (or it looks like there will
> > > > > > be), BIOS teams push back on a requirement to add the tables.
> > > > > > I'd encourage no one to bother with RASF - RAS2 is much less
> > > > > > ambiguous.  
> > > > >
> > > > > Here mainly to re-ping folks from Intel (Tony and Dave)  and AMD
> > > > > (Jon and Vilas) for your opinion on RAS2.
> > > > >  
> > > >
> > > > We'll need to know from vendors, ideally at minimum from both
> > > > Intel and AMD, whether RAS2 is the long-term vision here.  Nothing
> > > > is set in stone, of course, but deciding whether RAS2 is the
> > > > standard that we should be rallying around will help to guide
> > > > future development including in the kernel.
> > > >
> > > > If RAS2 is insufficient for future use cases or we would need to
> > > > support multiple implementations in the kernel for configuring the
> > > > patrol scrubber depending on vendor, that's great feedback to have.
> > > >
> > > > I'd much rather focus on implementing something in the kernel that
> > > > we have some clarity about the vendors supporting, especially when
> > > > it comes with user visible interfaces, as opposed to something
> > > > that may not be used long term.  I think that's a fair ask and
> > > > that vendor feedback is required here?  
> > >
> > > Agreed and happy to have feedback from Intel and AMD + all the other CPU vendors who make use of ACPI + all the OEMs who add stuff well beyond what Intel and AMD tell them to :)  I'll just note a lot of the ACPI support in the kernel covers stuff not used on mainstream x86 platforms because they are doing something custom and we didn't want 2 + X custom implementations...
> > >
> > > Some other interfaces for scrub control (beyond existing embedded ones) will surface in the next few months where RAS2 is not appropriate.
> > >
> > > Jonathan
> > >
> > >  
> >  
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2023-10-13  9:07 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-15 17:28 [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers shiju.jose
2023-09-15 17:28 ` [RFC PATCH 1/9] memory: scrub: Add scrub driver supports configuring memory scrubbers in the system shiju.jose
2023-09-15 17:28 ` [RFC PATCH 2/9] memory: scrub: sysfs: Add Documentation entries for set of scrub attributes shiju.jose
2023-09-22  0:07   ` Jiaqi Yan
2023-09-22 10:20     ` Jonathan Cameron
2023-09-28  5:25       ` Jiaqi Yan
2023-09-28 13:14         ` Jonathan Cameron
2023-10-05  3:18         ` David Rientjes
2023-10-06 13:02           ` Jonathan Cameron
2023-10-06 13:06             ` Sridharan, Vilas
2023-10-11 16:35               ` Jonathan Cameron
2023-10-12 13:41                 ` Sridharan, Vilas
2023-10-12 15:02                   ` Jonathan Cameron
2023-10-12 15:44                     ` Sridharan, Vilas
2023-10-13  9:07                       ` Jonathan Cameron
2023-09-15 17:28 ` [RFC PATCH 3/9] Documentation/scrub-configure.rst: Add documentation for scrub driver shiju.jose
2023-09-18  7:23   ` David Hildenbrand
2023-09-18 10:25     ` Shiju Jose
2023-09-18 12:15       ` David Hildenbrand
2023-09-18 12:28         ` Jonathan Cameron
2023-09-18 12:34           ` David Hildenbrand
2023-09-18 15:03             ` Shiju Jose
2023-09-15 17:28 ` [RFC PATCH 4/9] ACPI:RASF: Add extract RASF table to register RASF platform devices shiju.jose
2023-09-15 17:28 ` [RFC PATCH 5/9] ACPI:RASF: Add common library for RASF and RAS2 PCC interfaces shiju.jose
2023-09-15 17:28 ` [RFC PATCH 6/9] memory: RASF: Add memory RASF driver shiju.jose
2023-09-15 17:28 ` [RFC PATCH 7/9] ACPICA: ACPI 6.5: Add support for RAS2 table shiju.jose
2023-09-15 17:28 ` [RFC PATCH 8/9] ACPI:RAS2: Add driver for ACPI RAS2 feature table (RAS2) shiju.jose
2023-09-15 17:28 ` [RFC PATCH 9/9] memory: RAS2: Add memory RAS2 driver shiju.jose
2023-09-17 21:14 ` [RFC PATCH 0/9] ACPI:RASF: Add support for ACPI RASF, ACPI RAS2 and configure scrubbers Jiaqi Yan
2023-09-18 10:19   ` Shiju Jose
2023-09-18 17:47     ` Jiaqi Yan
2023-09-19  8:28       ` Shiju Jose

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).