The TSM attestation report support provides multiple configfs attribute
types (both for standard and binary attributes) to allow for additional
attributes to be displayed for SNP as compared to TDX. With the ability
to hide attributes via configfs, consoldate the multiple attribute groups
into a single standard attribute group and a single binary attribute
group. Modify the TDX support to hide the attributes that were previously
"hidden" as a result of registering the selective attribute groups.
Co-developed-by: Dan Williams <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
---
drivers/virt/coco/sev-guest/sev-guest.c | 3 +-
drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
drivers/virt/coco/tsm.c | 82 ++++++++++++-------------
include/linux/tsm.h | 41 ++++++++++---
4 files changed, 102 insertions(+), 53 deletions(-)
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 2abb51bd034f..ec3d894cfe31 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -23,6 +23,7 @@
#include <linux/sockptr.h>
#include <linux/cleanup.h>
#include <linux/uuid.h>
+#include <linux/configfs.h>
#include <uapi/linux/sev-guest.h>
#include <uapi/linux/psp-sev.h>
@@ -975,7 +976,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
/* Set the privlevel_floor attribute based on the vmpck_id */
sev_tsm_ops.privlevel_floor = vmpck_id;
- 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..964af57f345c 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -17,6 +17,7 @@
#include <linux/delay.h>
#include <linux/tsm.h>
#include <linux/sizes.h>
+#include <linux/configfs.h>
#include <uapi/linux/tdx-guest.h>
@@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report *report, void *data)
return ret;
}
+static bool tdx_report_attr_visible(struct config_item *item,
+ struct configfs_attribute *attr, int n)
+{
+ switch (n) {
+ case TSM_REPORT_GENERATION:
+ case TSM_REPORT_PROVIDER:
+ return true;
+ }
+
+ return false;
+}
+
+static bool tdx_report_bin_attr_visible(struct config_item *item,
+ struct configfs_bin_attribute *attr, int n)
+{
+ switch (n) {
+ case TSM_REPORT_INBLOB:
+ case TSM_REPORT_OUTBLOB:
+ return true;
+ }
+
+ return false;
+}
+
static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -281,6 +306,8 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
static const struct tsm_ops tdx_tsm_ops = {
.name = KBUILD_MODNAME,
.report_new = tdx_report_new,
+ .report_attr_visible = tdx_report_attr_visible,
+ .report_bin_attr_visible = tdx_report_bin_attr_visible,
};
static int __init tdx_guest_init(void)
@@ -301,7 +328,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..dedb4f582630 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(),
+ [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,
};
-static struct configfs_attribute *tsm_report_extra_attrs[] = {
- TSM_DEFAULT_ATTRS(),
- &tsm_report_attr_privlevel,
- &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,40 @@ 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_is_visible(struct config_item *item,
+ struct configfs_attribute *attr, int n)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!provider.ops)
+ return false;
+
+ if (!provider.ops->report_attr_visible)
+ return true;
+
+ return provider.ops->report_attr_visible(item, attr, n);
+}
+
+static bool tsm_report_is_bin_visible(struct config_item *item,
+ struct configfs_bin_attribute *attr, int n)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!provider.ops)
+ return false;
+
+ if (!provider.ops->report_bin_attr_visible)
+ return true;
+
+ return provider.ops->report_bin_attr_visible(item, attr, n);
+}
+
static struct configfs_group_operations tsm_report_group_ops = {
.make_item = tsm_report_make_item,
+ .is_visible = tsm_report_is_visible,
+ .is_bin_visible = tsm_report_is_bin_visible,
};
static const struct config_item_type tsm_reports_type = {
@@ -353,16 +355,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 +368,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 +379,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/include/linux/tsm.h b/include/linux/tsm.h
index 50c5769657d8..fa19291a9854 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -4,6 +4,7 @@
#include <linux/sizes.h>
#include <linux/types.h>
+#include <linux/configfs.h>
#define TSM_INBLOB_MAX 64
#define TSM_OUTBLOB_MAX SZ_32K
@@ -42,12 +43,40 @@ struct tsm_report {
u8 *auxblob;
};
+/**
+ * enum tsm_attr_index - index used to reference report attributes
+ * @TSM_REPORT_GENERATION: index of the report generation number attribute
+ * @TSM_REPORT_PROVIDER: index of the provider name attribute
+ * @TSM_REPORT_PRIVLEVEL: index of the desired privilege level attribute
+ * @TSM_REPORT_PRIVLEVEL_FLOOR: index of the minimum allowed privileg level attribute
+ */
+enum tsm_attr_index {
+ TSM_REPORT_GENERATION,
+ TSM_REPORT_PROVIDER,
+ TSM_REPORT_PRIVLEVEL,
+ TSM_REPORT_PRIVLEVEL_FLOOR,
+};
+
+/**
+ * enum tsm_bin_attr_index - index used to reference binary report attributes
+ * @TSM_REPORT_INBLOB: index of the binary report input attribute
+ * @TSM_REPORT_OUTBLOB: index of the binary report output attribute
+ * @TSM_REPORT_AUXBLOB: index of the binary auxiliary data attribute
+ */
+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
* @privlevel_floor: convey base privlevel for nested scenarios
* @report_new: Populate @report with the report blob and auxblob
* (optional), return 0 on successful population, or -errno otherwise
+ * @report_attr_visible: show or hide a report attribute entry
+ * @report_bin_attr_visible: show or hide a report binary attribute entry
*
* Implementation specific ops, only one is expected to be registered at
* a time i.e. only one of "sev-guest", "tdx-guest", etc.
@@ -56,14 +85,12 @@ struct tsm_ops {
const char *name;
unsigned int privlevel_floor;
int (*report_new)(struct tsm_report *report, void *data);
+ bool (*report_attr_visible)(struct config_item *item,
+ struct configfs_attribute *attr, int n);
+ bool (*report_bin_attr_visible)(struct config_item *item,
+ struct configfs_bin_attribute *attr, int n);
};
-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 */
--
2.43.2
Tom Lendacky wrote:
> The TSM attestation report support provides multiple configfs attribute
> types (both for standard and binary attributes) to allow for additional
> attributes to be displayed for SNP as compared to TDX. With the ability
> to hide attributes via configfs, consoldate the multiple attribute groups
> into a single standard attribute group and a single binary attribute
> group. Modify the TDX support to hide the attributes that were previously
> "hidden" as a result of registering the selective attribute groups.
>
> Co-developed-by: Dan Williams <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> drivers/virt/coco/sev-guest/sev-guest.c | 3 +-
> drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
> drivers/virt/coco/tsm.c | 82 ++++++++++++-------------
> include/linux/tsm.h | 41 ++++++++++---
> 4 files changed, 102 insertions(+), 53 deletions(-)
[..]
> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 1253bf76b570..964af57f345c 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
[..]
> @@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report *report, void *data)
> return ret;
> }
>
> +static bool tdx_report_attr_visible(struct config_item *item,
> + struct configfs_attribute *attr, int n)
> +{
> + switch (n) {
> + case TSM_REPORT_GENERATION:
> + case TSM_REPORT_PROVIDER:
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static bool tdx_report_bin_attr_visible(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n)
> +{
> + switch (n) {
> + case TSM_REPORT_INBLOB:
> + case TSM_REPORT_OUTBLOB:
> + return true;
> + }
> +
> + return false;
> +}
Why do these callbacks need @item and @attr?
[..]
> +static bool tsm_report_is_visible(struct config_item *item,
> + struct configfs_attribute *attr, int n)
Per the comment on where to find the is_visible() callbacks for a given
item type, I expect the need to pass @item here goes away when this can
assume that there is only one way to have is_visible() invoked for
@attr, right?
Other than that, this conversion looks good to me.
On 4/26/24 16:58, Dan Williams wrote:
> Tom Lendacky wrote:
>> The TSM attestation report support provides multiple configfs attribute
>> types (both for standard and binary attributes) to allow for additional
>> attributes to be displayed for SNP as compared to TDX. With the ability
>> to hide attributes via configfs, consoldate the multiple attribute groups
>> into a single standard attribute group and a single binary attribute
>> group. Modify the TDX support to hide the attributes that were previously
>> "hidden" as a result of registering the selective attribute groups.
>>
>> Co-developed-by: Dan Williams <[email protected]>
>> Signed-off-by: Dan Williams <[email protected]>
>> Signed-off-by: Tom Lendacky <[email protected]>
>> ---
>> drivers/virt/coco/sev-guest/sev-guest.c | 3 +-
>> drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
>> drivers/virt/coco/tsm.c | 82 ++++++++++++-------------
>> include/linux/tsm.h | 41 ++++++++++---
>> 4 files changed, 102 insertions(+), 53 deletions(-)
> [..]
>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> index 1253bf76b570..964af57f345c 100644
>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> [..]
>> @@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report *report, void *data)
>> return ret;
>> }
>>
>> +static bool tdx_report_attr_visible(struct config_item *item,
>> + struct configfs_attribute *attr, int n)
>> +{
>> + switch (n) {
>> + case TSM_REPORT_GENERATION:
>> + case TSM_REPORT_PROVIDER:
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static bool tdx_report_bin_attr_visible(struct config_item *item,
>> + struct configfs_bin_attribute *attr, int n)
>> +{
>> + switch (n) {
>> + case TSM_REPORT_INBLOB:
>> + case TSM_REPORT_OUTBLOB:
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>
> Why do these callbacks need @item and @attr?
It is a generic callback from configfs, so outside of TSM, an
implementation may find it useful to have these. But, with the code
change to require the callback at the attribute level, now, these can be
eliminated.
>
> [..]
>> +static bool tsm_report_is_visible(struct config_item *item,
>> + struct configfs_attribute *attr, int n)
>
> Per the comment on where to find the is_visible() callbacks for a given
> item type, I expect the need to pass @item here goes away when this can
> assume that there is only one way to have is_visible() invoked for
> @attr, right?
Yes.
Thanks,
Tom
>
> Other than that, this conversion looks good to me.
On 4/29/24 08:35, Tom Lendacky wrote:
> On 4/26/24 16:58, Dan Williams wrote:
>> Tom Lendacky wrote:
>>> The TSM attestation report support provides multiple configfs attribute
>>> types (both for standard and binary attributes) to allow for additional
>>> attributes to be displayed for SNP as compared to TDX. With the ability
>>> to hide attributes via configfs, consoldate the multiple attribute
>>> groups
>>> into a single standard attribute group and a single binary attribute
>>> group. Modify the TDX support to hide the attributes that were
>>> previously
>>> "hidden" as a result of registering the selective attribute groups.
>>>
>>> Co-developed-by: Dan Williams <[email protected]>
>>> Signed-off-by: Dan Williams <[email protected]>
>>> Signed-off-by: Tom Lendacky <[email protected]>
>>> ---
>>> drivers/virt/coco/sev-guest/sev-guest.c | 3 +-
>>> drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
>>> drivers/virt/coco/tsm.c | 82 ++++++++++++-------------
>>> include/linux/tsm.h | 41 ++++++++++---
>>> 4 files changed, 102 insertions(+), 53 deletions(-)
>> [..]
>>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c
>>> b/drivers/virt/coco/tdx-guest/tdx-guest.c
>>> index 1253bf76b570..964af57f345c 100644
>>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> [..]
>>> @@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report
>>> *report, void *data)
>>> return ret;
>>> }
>>> +static bool tdx_report_attr_visible(struct config_item *item,
>>> + struct configfs_attribute *attr, int n)
>>> +{
>>> + switch (n) {
>>> + case TSM_REPORT_GENERATION:
>>> + case TSM_REPORT_PROVIDER:
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static bool tdx_report_bin_attr_visible(struct config_item *item,
>>> + struct configfs_bin_attribute *attr, int n)
>>> +{
>>> + switch (n) {
>>> + case TSM_REPORT_INBLOB:
>>> + case TSM_REPORT_OUTBLOB:
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>
>> Why do these callbacks need @item and @attr?
>
> It is a generic callback from configfs, so outside of TSM, an
> implementation may find it useful to have these. But, with the code
> change to require the callback at the attribute level, now, these can be
> eliminated.
>
>>
>> [..]
>>> +static bool tsm_report_is_visible(struct config_item *item,
>>> + struct configfs_attribute *attr, int n)
>>
>> Per the comment on where to find the is_visible() callbacks for a given
>> item type, I expect the need to pass @item here goes away when this can
>> assume that there is only one way to have is_visible() invoked for
>> @attr, right?
>
> Yes.
But as I look closer, there is only a single ops callback pair
(is_visible() and is_bin_visible()), so as long there is never another
group / subdir defined under the TSM report, this works. But if another
group is added, then the item parameter would likely be needed or the
ops callback would have to be updated to differentiate for the vendor
(SNP/TDX).
Thanks,
Tom
>
> Thanks,
> Tom
>
>>
>> Other than that, this conversion looks good to me.
On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <[email protected]> wrote:
>
> The TSM attestation report support provides multiple configfs attribute
> types (both for standard and binary attributes) to allow for additional
> attributes to be displayed for SNP as compared to TDX. With the ability
> to hide attributes via configfs, consoldate the multiple attribute groups
> into a single standard attribute group and a single binary attribute
> group. Modify the TDX support to hide the attributes that were previously
> "hidden" as a result of registering the selective attribute groups.
>
> Co-developed-by: Dan Williams <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> drivers/virt/coco/sev-guest/sev-guest.c | 3 +-
> drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
> drivers/virt/coco/tsm.c | 82 ++++++++++++-------------
> include/linux/tsm.h | 41 ++++++++++---
> 4 files changed, 102 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 2abb51bd034f..ec3d894cfe31 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -23,6 +23,7 @@
> #include <linux/sockptr.h>
> #include <linux/cleanup.h>
> #include <linux/uuid.h>
> +#include <linux/configfs.h>
> #include <uapi/linux/sev-guest.h>
> #include <uapi/linux/psp-sev.h>
>
> @@ -975,7 +976,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
> /* Set the privlevel_floor attribute based on the vmpck_id */
> sev_tsm_ops.privlevel_floor = vmpck_id;
>
> - 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..964af57f345c 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -17,6 +17,7 @@
> #include <linux/delay.h>
> #include <linux/tsm.h>
> #include <linux/sizes.h>
> +#include <linux/configfs.h>
>
> #include <uapi/linux/tdx-guest.h>
>
> @@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report *report, void *data)
> return ret;
> }
>
> +static bool tdx_report_attr_visible(struct config_item *item,
> + struct configfs_attribute *attr, int n)
> +{
> + switch (n) {
> + case TSM_REPORT_GENERATION:
> + case TSM_REPORT_PROVIDER:
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static bool tdx_report_bin_attr_visible(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n)
> +{
> + switch (n) {
> + case TSM_REPORT_INBLOB:
> + case TSM_REPORT_OUTBLOB:
> + return true;
> + }
> +
> + return false;
> +}
> +
> static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -281,6 +306,8 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
> static const struct tsm_ops tdx_tsm_ops = {
> .name = KBUILD_MODNAME,
> .report_new = tdx_report_new,
> + .report_attr_visible = tdx_report_attr_visible,
> + .report_bin_attr_visible = tdx_report_bin_attr_visible,
> };
>
> static int __init tdx_guest_init(void)
> @@ -301,7 +328,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..dedb4f582630 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(),
> + [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,
> };
>
> -static struct configfs_attribute *tsm_report_extra_attrs[] = {
> - TSM_DEFAULT_ATTRS(),
> - &tsm_report_attr_privlevel,
> - &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,40 @@ 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_is_visible(struct config_item *item,
> + struct configfs_attribute *attr, int n)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!provider.ops)
> + return false;
> +
> + if (!provider.ops->report_attr_visible)
> + return true;
> +
> + return provider.ops->report_attr_visible(item, attr, n);
> +}
> +
> +static bool tsm_report_is_bin_visible(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!provider.ops)
> + return false;
> +
> + if (!provider.ops->report_bin_attr_visible)
> + return true;
> +
> + return provider.ops->report_bin_attr_visible(item, attr, n);
> +}
> +
> static struct configfs_group_operations tsm_report_group_ops = {
> .make_item = tsm_report_make_item,
> + .is_visible = tsm_report_is_visible,
> + .is_bin_visible = tsm_report_is_bin_visible,
> };
>
> static const struct config_item_type tsm_reports_type = {
> @@ -353,16 +355,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 +368,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 +379,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/include/linux/tsm.h b/include/linux/tsm.h
> index 50c5769657d8..fa19291a9854 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -4,6 +4,7 @@
>
> #include <linux/sizes.h>
> #include <linux/types.h>
> +#include <linux/configfs.h>
>
> #define TSM_INBLOB_MAX 64
> #define TSM_OUTBLOB_MAX SZ_32K
> @@ -42,12 +43,40 @@ struct tsm_report {
> u8 *auxblob;
> };
>
> +/**
> + * enum tsm_attr_index - index used to reference report attributes
> + * @TSM_REPORT_GENERATION: index of the report generation number attribute
> + * @TSM_REPORT_PROVIDER: index of the provider name attribute
> + * @TSM_REPORT_PRIVLEVEL: index of the desired privilege level attribute
> + * @TSM_REPORT_PRIVLEVEL_FLOOR: index of the minimum allowed privileg level attribute
/s/privleg/privlege
> + */
> +enum tsm_attr_index {
> + TSM_REPORT_GENERATION,
Do we need an index for the generation attribute ? Since it is a core
function, we can allow it by default.
> + TSM_REPORT_PROVIDER,
Same as above.
> + TSM_REPORT_PRIVLEVEL,
> + TSM_REPORT_PRIVLEVEL_FLOOR,
> +};
> +
> +/**
> + * enum tsm_bin_attr_index - index used to reference binary report attributes
> + * @TSM_REPORT_INBLOB: index of the binary report input attribute
> + * @TSM_REPORT_OUTBLOB: index of the binary report output attribute
> + * @TSM_REPORT_AUXBLOB: index of the binary auxiliary data attribute
> + */
> +enum tsm_bin_attr_index {
> + TSM_REPORT_INBLOB,
> + TSM_REPORT_OUTBLOB,
> + TSM_REPORT_AUXBLOB,
> +};
Why differentiate between bin attr and regular attr? Why not use the same enum?
> +
> /**
> * struct tsm_ops - attributes and operations for tsm instances
> * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
> * @privlevel_floor: convey base privlevel for nested scenarios
> * @report_new: Populate @report with the report blob and auxblob
> * (optional), return 0 on successful population, or -errno otherwise
> + * @report_attr_visible: show or hide a report attribute entry
> + * @report_bin_attr_visible: show or hide a report binary attribute entry
> *
> * Implementation specific ops, only one is expected to be registered at
> * a time i.e. only one of "sev-guest", "tdx-guest", etc.
> @@ -56,14 +85,12 @@ struct tsm_ops {
> const char *name;
> unsigned int privlevel_floor;
> int (*report_new)(struct tsm_report *report, void *data);
> + bool (*report_attr_visible)(struct config_item *item,
> + struct configfs_attribute *attr, int n);
> + bool (*report_bin_attr_visible)(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n);
I think we can use a single callback . We don't really gain anything
with bin attr differentiation.
> };
>
> -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 */
> --
> 2.43.2
>
Tom Lendacky wrote:
[..]
> >>> +static bool tsm_report_is_visible(struct config_item *item,
> >>> +????????????????? struct configfs_attribute *attr, int n)
> >>
> >> Per the comment on where to find the is_visible() callbacks for a given
> >> item type, I expect the need to pass @item here goes away when this can
> >> assume that there is only one way to have is_visible() invoked for
> >> @attr, right?
> >
> > Yes.
>
> But as I look closer, there is only a single ops callback pair
> (is_visible() and is_bin_visible()), so as long there is never another
> group / subdir defined under the TSM report, this works. But if another
> group is added, then the item parameter would likely be needed or the
> ops callback would have to be updated to differentiate for the vendor
> (SNP/TDX).
Makes sense, for the tsm_report_is_visible() signature. Especially as
this is proposed as a generic configfs facility. It has symmetry with
sysfs that passes an @kobj parent anchor, to sysfs is_visible()
callbacks.
However, I still think neither @item nor @attr need to be passed down
from tsm_report_is_visible() to the vendor backend. Those can be added
when/if another item type is added, which I do not see on the horizon.
Kuppuswamy Sathyanarayanan wrote:
> On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <[email protected]> wrote:
> >
> > The TSM attestation report support provides multiple configfs attribute
> > types (both for standard and binary attributes) to allow for additional
> > attributes to be displayed for SNP as compared to TDX. With the ability
> > to hide attributes via configfs, consoldate the multiple attribute groups
> > into a single standard attribute group and a single binary attribute
> > group. Modify the TDX support to hide the attributes that were previously
> > "hidden" as a result of registering the selective attribute groups.
> >
> > Co-developed-by: Dan Williams <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
> > Signed-off-by: Tom Lendacky <[email protected]>
[..]
> > + */
> > +enum tsm_attr_index {
> > + TSM_REPORT_GENERATION,
>
> Do we need an index for the generation attribute ? Since it is a core
> function, we can allow it by default.
That is up to the is_visible() callback to decide the declaration of
which index corresponds to which attribute is just static information.
>
> > + TSM_REPORT_PROVIDER,
>
> Same as above.
These numbers need to match the array indices of tsm_report_attrs.
Your suggestion makes the declaration of tsm_report_attrs more
difficult:
static struct configfs_attribute *tsm_report_attrs[] = {
[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,
};
..because then the definition of TSM_REPORT_PRIVLEVEL would need to
know how many attributes precede it in the array. So, defining it this
way makes it more robust against future changes that want to
add/delete/reorder attributes in the array.
>
> > + TSM_REPORT_PRIVLEVEL,
> > + TSM_REPORT_PRIVLEVEL_FLOOR,
> > +};
> > +
> > +/**
> > + * enum tsm_bin_attr_index - index used to reference binary report attributes
> > + * @TSM_REPORT_INBLOB: index of the binary report input attribute
> > + * @TSM_REPORT_OUTBLOB: index of the binary report output attribute
> > + * @TSM_REPORT_AUXBLOB: index of the binary auxiliary data attribute
> > + */
> > +enum tsm_bin_attr_index {
> > + TSM_REPORT_INBLOB,
> > + TSM_REPORT_OUTBLOB,
> > + TSM_REPORT_AUXBLOB,
> > +};
>
> Why differentiate between bin attr and regular attr? Why not use the same enum?
I do not understand your suggestion. There are two arrays:
static struct configfs_bin_attribute *tsm_report_bin_attrs[] = {
[TSM_REPORT_INBLOB] = &tsm_report_attr_inblob,
[TSM_REPORT_OUTBLOB] = &tsm_report_attr_outblob,
[TSM_REPORT_AUXBLOB] = &tsm_report_attr_auxblob,
NULL,
};
..so there are 2 sets of indices. If only one enum is used then one of
those arrays becomes sparsely populated causing the NULL array
terminator to pop up in unexpected indices.
> > +
> > /**
> > * struct tsm_ops - attributes and operations for tsm instances
> > * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
> > * @privlevel_floor: convey base privlevel for nested scenarios
> > * @report_new: Populate @report with the report blob and auxblob
> > * (optional), return 0 on successful population, or -errno otherwise
> > + * @report_attr_visible: show or hide a report attribute entry
> > + * @report_bin_attr_visible: show or hide a report binary attribute entry
> > *
> > * Implementation specific ops, only one is expected to be registered at
> > * a time i.e. only one of "sev-guest", "tdx-guest", etc.
> > @@ -56,14 +85,12 @@ struct tsm_ops {
> > const char *name;
> > unsigned int privlevel_floor;
> > int (*report_new)(struct tsm_report *report, void *data);
> > + bool (*report_attr_visible)(struct config_item *item,
> > + struct configfs_attribute *attr, int n);
> > + bool (*report_bin_attr_visible)(struct config_item *item,
> > + struct configfs_bin_attribute *attr, int n);
>
> I think we can use a single callback . We don't really gain anything
> with bin attr differentiation.
No, the goal here is symmetry with sysfs, and the arrays are separate so
the @n argument is a different index space. It also loses some type
safety for making sure that bin_attr callbacks can safely assume that
they were not passed a text attribute by mistake.
On Wed, May 1, 2024 at 1:15 PM Dan Williams <[email protected]> wrote:
>
> Kuppuswamy Sathyanarayanan wrote:
> > On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <[email protected]> wrote:
> > >
> > > The TSM attestation report support provides multiple configfs attribute
> > > types (both for standard and binary attributes) to allow for additional
> > > attributes to be displayed for SNP as compared to TDX. With the ability
> > > to hide attributes via configfs, consoldate the multiple attribute groups
> > > into a single standard attribute group and a single binary attribute
> > > group. Modify the TDX support to hide the attributes that were previously
> > > "hidden" as a result of registering the selective attribute groups.
> > >
> > > Co-developed-by: Dan Williams <[email protected]>
> > > Signed-off-by: Dan Williams <[email protected]>
> > > Signed-off-by: Tom Lendacky <[email protected]>
> [..]
> > > + */
> > > +enum tsm_attr_index {
> > > + TSM_REPORT_GENERATION,
> >
> > Do we need an index for the generation attribute ? Since it is a core
> > function, we can allow it by default.
>
> That is up to the is_visible() callback to decide the declaration of
> which index corresponds to which attribute is just static information.
>
> >
> > > + TSM_REPORT_PROVIDER,
> >
> > Same as above.
>
> These numbers need to match the array indices of tsm_report_attrs.
>
> Your suggestion makes the declaration of tsm_report_attrs more
> difficult:
>
> static struct configfs_attribute *tsm_report_attrs[] = {
> [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,
> };
>
> ...because then the definition of TSM_REPORT_PRIVLEVEL would need to
> know how many attributes precede it in the array. So, defining it this
> way makes it more robust against future changes that want to
> add/delete/reorder attributes in the array.
Got it. Makes sense. It is simpler to do it this way. I am just
worried that the vendor driver might mistakenly disable some core
attributes like inblob, outblob, provider and generation.
Kuppuswamy Sathyanarayanan wrote:
[..]
> Got it. Makes sense. It is simpler to do it this way. I am just
> worried that the vendor driver might mistakenly disable some core
> attributes like inblob, outblob, provider and generation.
Then it gets to keep the pieces it broke.
On 4/24/24 8:58 AM, Tom Lendacky wrote:
> The TSM attestation report support provides multiple configfs attribute
> types (both for standard and binary attributes) to allow for additional
> attributes to be displayed for SNP as compared to TDX. With the ability
> to hide attributes via configfs, consoldate the multiple attribute groups
> into a single standard attribute group and a single binary attribute
> group. Modify the TDX support to hide the attributes that were previously
> "hidden" as a result of registering the selective attribute groups.
>
> Co-developed-by: Dan Williams <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
> drivers/virt/coco/sev-guest/sev-guest.c | 3 +-
> drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
> drivers/virt/coco/tsm.c | 82 ++++++++++++-------------
> include/linux/tsm.h | 41 ++++++++++---
> 4 files changed, 102 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 2abb51bd034f..ec3d894cfe31 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -23,6 +23,7 @@
> #include <linux/sockptr.h>
> #include <linux/cleanup.h>
> #include <linux/uuid.h>
> +#include <linux/configfs.h>
> #include <uapi/linux/sev-guest.h>
> #include <uapi/linux/psp-sev.h>
>
> @@ -975,7 +976,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
> /* Set the privlevel_floor attribute based on the vmpck_id */
> sev_tsm_ops.privlevel_floor = vmpck_id;
>
> - 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..964af57f345c 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -17,6 +17,7 @@
> #include <linux/delay.h>
> #include <linux/tsm.h>
> #include <linux/sizes.h>
> +#include <linux/configfs.h>
>
> #include <uapi/linux/tdx-guest.h>
>
> @@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report *report, void *data)
> return ret;
> }
>
> +static bool tdx_report_attr_visible(struct config_item *item,
> + struct configfs_attribute *attr, int n)
> +{
> + switch (n) {
> + case TSM_REPORT_GENERATION:
> + case TSM_REPORT_PROVIDER:
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static bool tdx_report_bin_attr_visible(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n)
> +{
> + switch (n) {
> + case TSM_REPORT_INBLOB:
> + case TSM_REPORT_OUTBLOB:
> + return true;
> + }
> +
> + return false;
> +}
> +
> static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -281,6 +306,8 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
> static const struct tsm_ops tdx_tsm_ops = {
> .name = KBUILD_MODNAME,
> .report_new = tdx_report_new,
> + .report_attr_visible = tdx_report_attr_visible,
> + .report_bin_attr_visible = tdx_report_bin_attr_visible,
> };
>
> static int __init tdx_guest_init(void)
> @@ -301,7 +328,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..dedb4f582630 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(),
> + [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,
> };
>
> -static struct configfs_attribute *tsm_report_extra_attrs[] = {
> - TSM_DEFAULT_ATTRS(),
> - &tsm_report_attr_privlevel,
> - &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,40 @@ 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_is_visible(struct config_item *item,
> + struct configfs_attribute *attr, int n)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!provider.ops)
> + return false;
> +
> + if (!provider.ops->report_attr_visible)
> + return true;
> +
> + return provider.ops->report_attr_visible(item, attr, n);
> +}
> +
> +static bool tsm_report_is_bin_visible(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!provider.ops)
> + return false;
> +
> + if (!provider.ops->report_bin_attr_visible)
> + return true;
> +
> + return provider.ops->report_bin_attr_visible(item, attr, n);
> +}
> +
> static struct configfs_group_operations tsm_report_group_ops = {
> .make_item = tsm_report_make_item,
> + .is_visible = tsm_report_is_visible,
> + .is_bin_visible = tsm_report_is_bin_visible,
> };
>
> static const struct config_item_type tsm_reports_type = {
> @@ -353,16 +355,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 +368,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 +379,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/include/linux/tsm.h b/include/linux/tsm.h
> index 50c5769657d8..fa19291a9854 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -4,6 +4,7 @@
>
> #include <linux/sizes.h>
> #include <linux/types.h>
> +#include <linux/configfs.h>
>
> #define TSM_INBLOB_MAX 64
> #define TSM_OUTBLOB_MAX SZ_32K
> @@ -42,12 +43,40 @@ struct tsm_report {
> u8 *auxblob;
> };
>
> +/**
> + * enum tsm_attr_index - index used to reference report attributes
> + * @TSM_REPORT_GENERATION: index of the report generation number attribute
> + * @TSM_REPORT_PROVIDER: index of the provider name attribute
> + * @TSM_REPORT_PRIVLEVEL: index of the desired privilege level attribute
> + * @TSM_REPORT_PRIVLEVEL_FLOOR: index of the minimum allowed privileg level attribute
> + */
> +enum tsm_attr_index {
> + TSM_REPORT_GENERATION,
> + TSM_REPORT_PROVIDER,
> + TSM_REPORT_PRIVLEVEL,
> + TSM_REPORT_PRIVLEVEL_FLOOR,
> +};
> +
> +/**
> + * enum tsm_bin_attr_index - index used to reference binary report attributes
> + * @TSM_REPORT_INBLOB: index of the binary report input attribute
> + * @TSM_REPORT_OUTBLOB: index of the binary report output attribute
> + * @TSM_REPORT_AUXBLOB: index of the binary auxiliary data attribute
> + */
> +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
> * @privlevel_floor: convey base privlevel for nested scenarios
> * @report_new: Populate @report with the report blob and auxblob
> * (optional), return 0 on successful population, or -errno otherwise
> + * @report_attr_visible: show or hide a report attribute entry
> + * @report_bin_attr_visible: show or hide a report binary attribute entry
> *
> * Implementation specific ops, only one is expected to be registered at
> * a time i.e. only one of "sev-guest", "tdx-guest", etc.
> @@ -56,14 +85,12 @@ struct tsm_ops {
> const char *name;
> unsigned int privlevel_floor;
> int (*report_new)(struct tsm_report *report, void *data);
> + bool (*report_attr_visible)(struct config_item *item,
> + struct configfs_attribute *attr, int n);
> + bool (*report_bin_attr_visible)(struct config_item *item,
> + struct configfs_bin_attribute *attr, int n);
> };
>
> -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 */
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer