[MODERATED] Re: [PATCH v4 04/10] TAAv4 4

On Tue, Sep 24, 2019 at 07:04:59AM +0200, speck for Greg KH wrote:
> Either way, my original comment of "do not use sysfs_create_group() when
> you have a 'struct device'" still stands, that needs to be fixed up no
> matter what here.

Thanks for your inputs. As we have the 'struct device' are you
suggesting to use device_create_file()?

Approach 1 - sysfs_create_file() => device_create_file()

-----------------
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 40fec8ab82b1..4799c081198c 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -162,7 +162,7 @@ static void tsx_update_on_each_cpu(bool val)
 	put_online_cpus();
 }
 
-static ssize_t tsx_show(struct device *cdev,
+static ssize_t htm_show(struct device *cdev,
 			struct device_attribute *attr,
 			char *buf)
 {
@@ -171,7 +171,7 @@ static ssize_t tsx_show(struct device *cdev,
 
 static DEFINE_MUTEX(tsx_mutex);
 
-static ssize_t tsx_store(struct device *cdev,
+static ssize_t htm_store(struct device *cdev,
 			 struct device_attribute *attr,
 			 const char *buf, size_t count)
 {
@@ -197,15 +197,14 @@ static ssize_t tsx_store(struct device *cdev,
 	return count;
 }
 
-static DEVICE_ATTR_RW(tsx);
+static const DEVICE_ATTR_RW(htm);
 
 static int __init tsx_sysfs_init(void)
 {
 	if (tsx_ctrl_state == TSX_CTRL_NOT_SUPPORTED)
 		return -ENODEV;
 
-	return sysfs_create_file(&cpu_subsys.dev_root->kobj,
-				 &dev_attr_tsx.attr);
+	return device_create_file(cpu_subsys.dev_root, &dev_attr_htm);
 }
 
-late_initcall(tsx_sysfs_init);
+device_initcall(tsx_sysfs_init);



-------------------------------------------------------------------

If the first approach is not what you are expecting, is the below
patch to add the attribute to the cpu_root_attrs list okay?

Approach 2 - Add tsx attribute to cpu_root_attrs in drivers/base/cpu.c

--------------------
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index da75021daa1d..631996e129d5 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -158,17 +158,15 @@ static void tsx_update_on_each_cpu(bool val)
 	put_online_cpus();
 }
 
-static ssize_t tsx_show(struct device *cdev,
-			struct device_attribute *attr,
-			char *buf)
+ssize_t htm_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%d\n", tsx_ctrl_state == TSX_CTRL_ENABLE ? 1 : 0);
 }
 
 static DEFINE_MUTEX(tsx_mutex);
 
-static ssize_t tsx_store(struct device *cdev, struct device_attribute *attr,
-			 const char *buf, size_t count)
+ssize_t htm_store(struct device *dev, struct device_attribute *attr,
+		  const char *buf, size_t count)
 {
 	enum tsx_ctrl_states requested_state;
 	ssize_t ret;
@@ -229,32 +227,10 @@ static ssize_t tsx_store(struct device *cdev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR_RW(tsx);
-
-static struct attribute *tsx_ctrl_attrs[] = {
-	&dev_attr_tsx.attr,
-	NULL
-};
-
-static umode_t tsx_ctrl_attr_is_visible(struct kobject *kobj,
-					struct attribute *attr, int index)
+umode_t htm_is_visible(void)
 {
-	if ((attr == &dev_attr_tsx.attr) &&
-	    (tsx_ctrl_state == TSX_CTRL_NOT_SUPPORTED))
+	if (tsx_ctrl_state == TSX_CTRL_NOT_SUPPORTED)
 		return 0;
 
-	return attr->mode;
+	return 0644;
 }
-
-static struct attribute_group tsx_ctrl_attr_group = {
-	.attrs		= tsx_ctrl_attrs,
-	.is_visible	= tsx_ctrl_attr_is_visible,
-};
-
-static int __init tsx_sysfs_init(void)
-{
-	return sysfs_create_group(&cpu_subsys.dev_root->kobj,
-				  &tsx_ctrl_attr_group);
-}
-
-late_initcall(tsx_sysfs_init);
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 80d43ff9a7ec..2b1579c7f22d 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -460,6 +460,34 @@ struct device *cpu_device_create(struct device *parent, void *drvdata,
 }
 EXPORT_SYMBOL_GPL(cpu_device_create);
 
+ssize_t __weak htm_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	return -ENODEV;
+}
+
+ssize_t __weak htm_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	return -ENODEV;
+}
+
+DEVICE_ATTR_RW(htm);
+
+umode_t __weak htm_is_visible(void)
+{
+	return 0;
+}
+
+static umode_t cpu_root_attrs_is_visible(struct kobject *kobj,
+					 struct attribute *attr, int index)
+{
+	if (attr == &dev_attr_htm.attr)
+		return htm_is_visible();
+
+	return attr->mode;
+}
+
 #ifdef CONFIG_GENERIC_CPU_AUTOPROBE
 static DEVICE_ATTR(modalias, 0444, print_cpu_modalias, NULL);
 #endif
@@ -481,11 +509,13 @@ static struct attribute *cpu_root_attrs[] = {
 #ifdef CONFIG_GENERIC_CPU_AUTOPROBE
 	&dev_attr_modalias.attr,
 #endif
+	&dev_attr_htm.attr,
 	NULL
 };
 
 static struct attribute_group cpu_root_attr_group = {
-	.attrs = cpu_root_attrs,
+	.attrs		= cpu_root_attrs,
+	.is_visible	= cpu_root_attrs_is_visible,
 };
 
 static const struct attribute_group *cpu_root_attr_groups[] = {