Re: [PATCH v3 12/14] fs/configfs: Add a callback to determine attribute visibility
From: Tom Lendacky
Date: Tue Apr 16 2024 - 15:54:25 EST
On 4/16/24 13:25, Dan Williams wrote:
Tom Lendacky wrote:
On 4/16/24 00:46, Dan Williams wrote:
Tom Lendacky wrote:
In order to support dynamic decisions as to whether an attribute should be
created, add a callback that returns a bool to indicate whether the
attribute should be display. If no callback is registered, the attribute
[..]
Cc: Joel Becker <jlbec@xxxxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
---
fs/configfs/file.c | 7 +++
include/linux/configfs.h | 111 +++++++++++++++++++++++++++------------
2 files changed, 84 insertions(+), 34 deletions(-)
[..]
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 2606711adb18..c836d7bc7c9e 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -116,35 +116,57 @@ struct configfs_attribute {
const char *ca_name;
struct module *ca_owner;
umode_t ca_mode;
+ bool (*is_visible)(const struct config_item *, const struct configfs_attribute *);
ssize_t (*show)(struct config_item *, char *);
ssize_t (*store)(struct config_item *, const char *, size_t);
};
-#define CONFIGFS_ATTR(_pfx, _name) \
+#define __CONFIGFS_ATTR(_pfx, _name, _vis) \
static struct configfs_attribute _pfx##attr_##_name = { \
.ca_name = __stringify(_name), \
.ca_mode = S_IRUGO | S_IWUSR, \
.ca_owner = THIS_MODULE, \
+ .is_visible = _vis, \
.show = _pfx##_name##_show, \
.store = _pfx##_name##_store, \
Shouldn't this operation live in configfs_group_operations? That would
mirror the sysfs organization, and likely saves some memory.
I suppose it can, but then you lose the grouping of attributes within
the same directory, right? A configfs group will result in moving the
entries into a subdirectory, right? If we go with the group level, then
we will be moving the existing TSM extra attributes and the new TSM SVSM
attributes into new, separate sub-directories.
I am not following the concern about "losing the grouping"? Here is what
I was thinking with having the visibility routines in group operations.
This is just the broard strokes, it compiles, but still needs the finer
detail work to make tdx-guest skip all the attributes that do not apply
to it. Might need to be broken up a bit more, but hopefully conveys the
idea. Does this address your grouping concern?
Yes and no. Basically the is_visible()/is_bin_visible() callback will
have to check every index value for a "group" against the passed in
value. I was trying to group the values together using an enum in order
to make it a bit easier and more readable in the callback. Adding
another attribute to the group requires updates in multiple places. But
thats just how I was looking at it. I can also see where you might want
to selectively hide/show entries and this method works well for that.
I'll follow this approach (add you as Co-developed-by: or Suggested-by:,
whichever you prefer) and submit a v4.
Thanks,
Tom
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 87f241825bc3..39b8455f0ba5 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -968,7 +968,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
snp_dev->input.resp_gpa = __pa(snp_dev->response);
snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
- ret = tsm_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type);
+ ret = tsm_register(&sev_tsm_ops, snp_dev);
if (ret)
goto e_free_cert_data;
diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
index 1253bf76b570..654d20ea524a 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -301,7 +301,7 @@ static int __init tdx_guest_init(void)
goto free_misc;
}
- ret = tsm_register(&tdx_tsm_ops, NULL, NULL);
+ ret = tsm_register(&tdx_tsm_ops, NULL);
if (ret)
goto free_quote;
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index d1c2db83a8ca..b31be0e61728 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -14,7 +14,6 @@
static struct tsm_provider {
const struct tsm_ops *ops;
- const struct config_item_type *type;
void *data;
} provider;
static DECLARE_RWSEM(tsm_rwsem);
@@ -252,34 +251,18 @@ static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
}
CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_OUTBLOB_MAX);
-#define TSM_DEFAULT_ATTRS() \
- &tsm_report_attr_generation, \
- &tsm_report_attr_provider
-
static struct configfs_attribute *tsm_report_attrs[] = {
- TSM_DEFAULT_ATTRS(),
- NULL,
-};
-
-static struct configfs_attribute *tsm_report_extra_attrs[] = {
- TSM_DEFAULT_ATTRS(),
- &tsm_report_attr_privlevel,
- &tsm_report_attr_privlevel_floor,
+ [TSM_REPORT_GENERATION] = &tsm_report_attr_generation,
+ [TSM_REPORT_PROVIDER] = &tsm_report_attr_provider,
+ [TSM_REPORT_PRIVLEVEL] = &tsm_report_attr_privlevel,
+ [TSM_REPORT_PRIVLEVEL_FLOOR] = &tsm_report_attr_privlevel_floor,
NULL,
};
-#define TSM_DEFAULT_BIN_ATTRS() \
- &tsm_report_attr_inblob, \
- &tsm_report_attr_outblob
-
static struct configfs_bin_attribute *tsm_report_bin_attrs[] = {
- TSM_DEFAULT_BIN_ATTRS(),
- NULL,
-};
-
-static struct configfs_bin_attribute *tsm_report_bin_extra_attrs[] = {
- TSM_DEFAULT_BIN_ATTRS(),
- &tsm_report_attr_auxblob,
+ [TSM_REPORT_INBLOB] = &tsm_report_attr_inblob,
+ [TSM_REPORT_OUTBLOB] = &tsm_report_attr_outblob,
+ [TSM_REPORT_AUXBLOB] = &tsm_report_attr_auxblob,
NULL,
};
@@ -297,21 +280,12 @@ static struct configfs_item_operations tsm_report_item_ops = {
.release = tsm_report_item_release,
};
-const struct config_item_type tsm_report_default_type = {
+static const struct config_item_type tsm_report_type = {
.ct_owner = THIS_MODULE,
.ct_bin_attrs = tsm_report_bin_attrs,
.ct_attrs = tsm_report_attrs,
.ct_item_ops = &tsm_report_item_ops,
};
-EXPORT_SYMBOL_GPL(tsm_report_default_type);
-
-const struct config_item_type tsm_report_extra_type = {
- .ct_owner = THIS_MODULE,
- .ct_bin_attrs = tsm_report_bin_extra_attrs,
- .ct_attrs = tsm_report_extra_attrs,
- .ct_item_ops = &tsm_report_item_ops,
-};
-EXPORT_SYMBOL_GPL(tsm_report_extra_type);
static struct config_item *tsm_report_make_item(struct config_group *group,
const char *name)
@@ -326,12 +300,38 @@ static struct config_item *tsm_report_make_item(struct config_group *group,
if (!state)
return ERR_PTR(-ENOMEM);
- config_item_init_type_name(&state->cfg, name, provider.type);
+ config_item_init_type_name(&state->cfg, name, &tsm_report_type);
return &state->cfg;
}
+static bool tsm_report_attr_visible(struct configfs_attribute *attr, int n)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!provider.ops)
+ return false;
+
+ if (!provider.ops->is_visible)
+ return true;
+
+ return provider.ops->is_visible(n);
+}
+
+static bool tsm_report_bin_attr_visible(struct configfs_bin_attribute *attr,
+ int n)
+{
+ if (!provider.ops)
+ return false;
+
+ if (!provider.ops->is_bin_visible)
+ return true;
+
+ return provider.ops->is_bin_visible(n);
+}
+
static struct configfs_group_operations tsm_report_group_ops = {
.make_item = tsm_report_make_item,
+ .is_visible = tsm_report_attr_visible,
+ .is_bin_visible = tsm_report_bin_attr_visible,
};
static const struct config_item_type tsm_reports_type = {
@@ -353,16 +353,10 @@ static struct configfs_subsystem tsm_configfs = {
.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
};
-int tsm_register(const struct tsm_ops *ops, void *priv,
- const struct config_item_type *type)
+int tsm_register(const struct tsm_ops *ops, void *priv)
{
const struct tsm_ops *conflict;
- if (!type)
- type = &tsm_report_default_type;
- if (!(type == &tsm_report_default_type || type == &tsm_report_extra_type))
- return -EINVAL;
-
guard(rwsem_write)(&tsm_rwsem);
conflict = provider.ops;
if (conflict) {
@@ -372,7 +366,6 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
provider.ops = ops;
provider.data = priv;
- provider.type = type;
return 0;
}
EXPORT_SYMBOL_GPL(tsm_register);
@@ -384,7 +377,6 @@ int tsm_unregister(const struct tsm_ops *ops)
return -EBUSY;
provider.ops = NULL;
provider.data = NULL;
- provider.type = NULL;
return 0;
}
EXPORT_SYMBOL_GPL(tsm_unregister);
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 18677cd4e62f..213e88f4cec2 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -580,6 +580,7 @@ static void detach_attrs(struct config_item * item)
static int populate_attrs(struct config_item *item)
{
const struct config_item_type *t = item->ci_type;
+ struct configfs_group_operations *ops = t->ct_group_ops;
struct configfs_attribute *attr;
struct configfs_bin_attribute *bin_attr;
int error = 0;
@@ -589,12 +590,17 @@ static int populate_attrs(struct config_item *item)
return -EINVAL;
if (t->ct_attrs) {
for (i = 0; (attr = t->ct_attrs[i]) != NULL; i++) {
+ if (ops && ops->is_visible && !ops->is_visible(attr, i))
+ continue;
if ((error = configfs_create_file(item, attr)))
break;
}
}
if (t->ct_bin_attrs) {
for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+ if (ops && ops->is_bin_visible &&
+ !ops->is_bin_visible(bin_attr, i))
+ continue;
error = configfs_create_bin_file(item, bin_attr);
if (error)
break;
diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index 0ad32150611e..356d23b6b9cf 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -444,7 +444,8 @@ const struct file_operations configfs_bin_file_operations = {
* @attr: atrribute descriptor.
*/
-int configfs_create_file(struct config_item * item, const struct configfs_attribute * attr)
+int configfs_create_file(struct config_item *item,
+ const struct configfs_attribute *attr)
{
struct dentry *dir = item->ci_dentry;
struct configfs_dirent *parent_sd = dir->d_fsdata;
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 2606711adb18..31553f12db7c 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -216,6 +216,8 @@ struct configfs_group_operations {
struct config_group *(*make_group)(struct config_group *group, const char *name);
void (*disconnect_notify)(struct config_group *group, struct config_item *item);
void (*drop_item)(struct config_group *group, struct config_item *item);
+ bool (*is_visible)(struct configfs_attribute *attr, int n);
+ bool (*is_bin_visible)(struct configfs_bin_attribute *attr, int n);
};
struct configfs_subsystem {
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index de8324a2223c..a45b12943223 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -42,6 +42,19 @@ struct tsm_report {
u8 *auxblob;
};
+enum tsm_attr_index {
+ TSM_REPORT_GENERATION,
+ TSM_REPORT_PROVIDER,
+ TSM_REPORT_PRIVLEVEL,
+ TSM_REPORT_PRIVLEVEL_FLOOR,
+};
+
+enum tsm_bin_attr_index {
+ TSM_REPORT_INBLOB,
+ TSM_REPORT_OUTBLOB,
+ TSM_REPORT_AUXBLOB,
+};
+
/**
* struct tsm_ops - attributes and operations for tsm instances
* @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
@@ -55,15 +68,11 @@ struct tsm_report {
struct tsm_ops {
const char *name;
const unsigned int privlevel_floor;
+ bool (*is_visible)(enum tsm_attr_index index);
+ bool (*is_bin_visible)(enum tsm_bin_attr_index index);
int (*report_new)(struct tsm_report *report, void *data);
};
-extern const struct config_item_type tsm_report_default_type;
-
-/* publish @privlevel, @privlevel_floor, and @auxblob attributes */
-extern const struct config_item_type tsm_report_extra_type;
-
-int tsm_register(const struct tsm_ops *ops, void *priv,
- const struct config_item_type *type);
+int tsm_register(const struct tsm_ops *ops, void *priv);
int tsm_unregister(const struct tsm_ops *ops);
#endif /* __TSM_H */