Re: [PATCH 10/11] x86/sev: Extend the config-fs attestation support for an SVSM
From: Tom Lendacky
Date: Mon Feb 26 2024 - 10:16:08 EST
On 2/23/24 18:02, Dan Williams wrote:
Tom Lendacky wrote:
On 2/12/24 20:34, Dan Williams wrote:
Tom Lendacky wrote:
On 2/2/24 01:10, Dan Williams wrote:
Tom Lendacky wrote:
When an SVSM is present, the guest can also request attestation reports
from the SVSM. These SVSM attestation reports can be used to attest the
SVSM and any services running within the SVSM.
Extend the config-fs attestation support to allow for an SVSM attestation
report. This involves creating four (4) new config-fs attributes:
- 'svsm' (input)
This attribute is used to determine whether the attestation request
should be sent to the SVSM or to the SEV firmware.
- 'service_guid' (input)
Used for requesting the attestation of a single service within the
SVSM. A null GUID implies that the SVSM_ATTEST_SERVICES call should
be used to request the attestation report. A non-null GUID implies
that the SVSM_ATTEST_SINGLE_SERVICE call should be used.
- 'service_version' (input)
Used with the SVSM_ATTEST_SINGLE_SERVICE call, the service version
represents a specific service manifest version be used for the
attestation report.
- 'manifestblob' (output)
Used to return the service manifest associated with the attestation
report.
Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
---
Documentation/ABI/testing/configfs-tsm | 55 ++++++++++
arch/x86/include/asm/sev.h | 31 +++++-
arch/x86/kernel/sev.c | 50 +++++++++
drivers/virt/coco/sev-guest/sev-guest.c | 137 ++++++++++++++++++++++++
drivers/virt/coco/tsm.c | 95 +++++++++++++++-
include/linux/tsm.h | 11 ++
6 files changed, 376 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
index dd24202b5ba5..c5423987d323 100644
--- a/Documentation/ABI/testing/configfs-tsm
+++ b/Documentation/ABI/testing/configfs-tsm
@@ -31,6 +31,21 @@ Description:
Standardization v2.03 Section 4.1.8.1 MSG_REPORT_REQ.
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf
+What: /sys/kernel/config/tsm/report/$name/manifestblob
+Date: January, 2024
+KernelVersion: v6.9
+Contact: linux-coco@xxxxxxxxxxxxxxx
+Description:
+ (RO) Optional supplemental data that a TSM may emit, visibility
+ of this attribute depends on TSM, and may be empty if no
+ manifest data is available.
+
+ When @provider is "sev_guest" and the "svsm" attribute is set
+ this file contains the service manifest used for the SVSM
+ attestation report from Secure VM Service Module for SEV-SNP
+ Guests v1.00 Section 7.
+ https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
I wish configfs had better dynamic visibility so that this could be
hidden when not active... oh well.
So do I. I played with the idea of having two different structs and
registering one or the other based on whether an SVSM was present. Thoughts?
That's the status quo for the differences between TDX vs SEV
(tsm_report_default_type vs tsm_report_extra_type). I think a
"tsm_report_service_type " can be a new superset of
tsm_report_extra_type. That that just starts to get messy if
implementations start to pick and choose on a finer granularity what
they support. For example, what if TDX supports these new service
attributes, but not privlevel.
It seems straightforward to add an is_visible() callback to
'struct configfs_item_operations'. Then a common superset of all the
attributes could be specified, but with an is_visible() implementation
that consults with data registered with tsm_register() rather than the
@type argument directly.
I've been playing with this a bit.
For the configfs support I was thinking of doing a per attribute
is_visible() callback field since the TSM support is currently all in one
group. The callback field would be checked in fs/configfs/dir.c
populate_attrs(). A simple bool return value should suffice, I'm not sure
we need to get into umode changes. If the field is NULL, then the
attribute is displayed. If non-NULL, it depends on the callback return value.
In order to keep tsm.c as vendor neutral as possible, a way to
provide/override a default callback would be needed. The new SVSM related
fields would have a callback assigned that always returns false by
default, so that these attributes wouldn't be visible. A new tsm.c
interface that takes the attribute name and a callback function can be
used to override the requested attribute. For example, the SEV guest
driver could register a callback for these attributes that returns true
when running under an SVSM or false otherwise. Or it could leave the
default in place and register a callback when running under an SVSM that
always returns true.
Thoughts?
Sounds like a patch I want to see, yes. So the idea is the low-level
driver registers the is_visible() callback to the core and that gets to
filter attributes?
Hmm, as long as it ends up looking similar to sysfs is_visible()
prototype.
It could even just be a bitmask of attributes that gets passed in by the
provider, something like:
static struct configfs_attribute *tsm_report_attrs[] = {
[TSM_REPORT_ATTR_GENERATION] = &tsm_report_attr_generation,
[TSM_REPORT_ATTR_PROVIDER] = &tsm_report_attr_provider
[TSM_REPORT_ATTR_PRIVLEVEL] = &tsm_report_attr_privlevel,
[TSM_REPORT_ATTR_FLOOR] = &tsm_report_attr_privlevel_floor,
NULL,
};
bool tsm_report_is_visible(struct config_item *cfg, struct configfs_attribute *attr, int n)
{
return test_bit(n, &provider.attr_mask);
}
..and in is_bin_visible() for the binary attributes?
I was thinking something along the lines of the following for the configfs
support:
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 18677cd4e62f..c224060c1b6b 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -589,12 +589,20 @@ 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 (attr->ca_is_visible && !attr->ca_is_visible(attr))
+ continue;
+
if ((error = configfs_create_file(item, attr)))
break;
}
}
- if (t->ct_bin_attrs) {
+ if (t->ct_bin_attrs && !error) {
for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+ attr = &bin_attr->cb_attr;
+
+ if (attr->ca_is_visible && !attr->ca_is_visible(attr))
+ continue;
+
error = configfs_create_bin_file(item, bin_attr);
if (error)
break;
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 2606711adb18..93d346e2afc1 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -112,39 +112,63 @@ static inline void configfs_add_default_group(struct config_group *new_group,
list_add_tail(&new_group->group_entry, &group->default_groups);
}
+typedef bool (*configfs_is_visible_t)(const struct configfs_attribute *attr);
+
struct configfs_attribute {
const char *ca_name;
struct module *ca_owner;
umode_t ca_mode;
+ configfs_is_visible_t ca_is_visible;
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, \
+ .ca_is_visible = _vis, \
.show = _pfx##_name##_show, \
.store = _pfx##_name##_store, \
}
-#define CONFIGFS_ATTR_RO(_pfx, _name) \
+#define __CONFIGFS_ATTR_RO(_pfx, _name, _vis) \
static struct configfs_attribute _pfx##attr_##_name = { \
.ca_name = __stringify(_name), \
.ca_mode = S_IRUGO, \
.ca_owner = THIS_MODULE, \
+ .ca_is_visible = _vis, \
.show = _pfx##_name##_show, \
}
-#define CONFIGFS_ATTR_WO(_pfx, _name) \
+#define __CONFIGFS_ATTR_WO(_pfx, _name, _vis) \
static struct configfs_attribute _pfx##attr_##_name = { \
.ca_name = __stringify(_name), \
.ca_mode = S_IWUSR, \
.ca_owner = THIS_MODULE, \
+ .ca_is_visible = _vis, \
.store = _pfx##_name##_store, \
}
+#define CONFIGFS_ATTR(_pfx, _name) \
+ __CONFIGFS_ATTR(_pfx, _name, NULL)
+
+#define CONFIGFS_ATTR_RO(_pfx, _name) \
+ __CONFIGFS_ATTR_RO(_pfx, _name, NULL)
+
+#define CONFIGFS_ATTR_WO(_pfx, _name) \
+ __CONFIGFS_ATTR_WO(_pfx, _name, NULL)
+
+#define CONFIGFS_ATTR_VISIBLE(_pfx, _name, _vis) \
+ __CONFIGFS_ATTR(_pfx, _name, _vis)
+
+#define CONFIGFS_ATTR_VISIBLE_RO(_pfx, _name, _vis) \
+ __CONFIGFS_ATTR_RO(_pfx, _name, _vis)
+
+#define CONFIGFS_ATTR_VISIBLE_WO(_pfx, _name, _vis) \
+ __CONFIGFS_ATTR_WO(_pfx, _name, _vis)
+
struct file;
struct vm_area_struct;
@@ -156,43 +180,64 @@ struct configfs_bin_attribute {
ssize_t (*write)(struct config_item *, const void *, size_t);
};
-#define CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz) \
-static struct configfs_bin_attribute _pfx##attr_##_name = { \
- .cb_attr = { \
- .ca_name = __stringify(_name), \
- .ca_mode = S_IRUGO | S_IWUSR, \
- .ca_owner = THIS_MODULE, \
- }, \
- .cb_private = _priv, \
- .cb_max_size = _maxsz, \
- .read = _pfx##_name##_read, \
- .write = _pfx##_name##_write, \
+#define __CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz, _vis) \
+static struct configfs_bin_attribute _pfx##attr_##_name = { \
+ .cb_attr = { \
+ .ca_name = __stringify(_name), \
+ .ca_mode = S_IRUGO | S_IWUSR, \
+ .ca_owner = THIS_MODULE, \
+ .ca_is_visible = _vis, \
+ }, \
+ .cb_private = _priv, \
+ .cb_max_size = _maxsz, \
+ .read = _pfx##_name##_read, \
+ .write = _pfx##_name##_write, \
}
-#define CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz) \
-static struct configfs_bin_attribute _pfx##attr_##_name = { \
- .cb_attr = { \
- .ca_name = __stringify(_name), \
- .ca_mode = S_IRUGO, \
- .ca_owner = THIS_MODULE, \
- }, \
- .cb_private = _priv, \
- .cb_max_size = _maxsz, \
- .read = _pfx##_name##_read, \
+#define __CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz, _vis) \
+static struct configfs_bin_attribute _pfx##attr_##_name = { \
+ .cb_attr = { \
+ .ca_name = __stringify(_name), \
+ .ca_mode = S_IRUGO, \
+ .ca_owner = THIS_MODULE, \
+ .ca_is_visible = _vis, \
+ }, \
+ .cb_private = _priv, \
+ .cb_max_size = _maxsz, \
+ .read = _pfx##_name##_read, \
}
-#define CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz) \
-static struct configfs_bin_attribute _pfx##attr_##_name = { \
- .cb_attr = { \
- .ca_name = __stringify(_name), \
- .ca_mode = S_IWUSR, \
- .ca_owner = THIS_MODULE, \
- }, \
- .cb_private = _priv, \
- .cb_max_size = _maxsz, \
- .write = _pfx##_name##_write, \
+#define __CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz, _vis) \
+static struct configfs_bin_attribute _pfx##attr_##_name = { \
+ .cb_attr = { \
+ .ca_name = __stringify(_name), \
+ .ca_mode = S_IWUSR, \
+ .ca_owner = THIS_MODULE, \
+ .ca_is_visible = _vis, \
+ }, \
+ .cb_private = _priv, \
+ .cb_max_size = _maxsz, \
+ .write = _pfx##_name##_write, \
}
+#define CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz) \
+ __CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz, NULL)
+
+#define CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz) \
+ __CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz, NULL)
+
+#define CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz) \
+ __CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz, NULL)
+
+#define CONFIGFS_BIN_ATTR_VISIBLE(_pfx, _name, _priv, _maxs, _vis) \
+ __CONFIGFS_BIN_ATTR(_pfx, _name, _priv, _maxsz, _vis)
+
+#define CONFIGFS_BIN_ATTR_VISIBLE_RO(_pfx, _name, _priv, _maxsz, _vis) \
+ __CONFIGFS_BIN_ATTR_RO(_pfx, _name, _priv, _maxsz, _vis)
+
+#define CONFIGFS_BIN_ATTR_VISIBLE_WO(_pfx, _name, _priv, _maxsz, _vis) \
+ __CONFIGFS_BIN_ATTR_WO(_pfx, _name, _priv, _maxsz, _vis)
+
/*
* If allow_link() exists, the item can symlink(2) out to other
* items. If the item is a group, it may support mkdir(2).
And then the following for implementing it for tsm, which allows for as
simple or complicated an is_visible() function as required:
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 4ac62c896670..f29310a4a357 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -1022,6 +1022,11 @@ static int sev_report_new(struct tsm_report *report, void *data)
return 0;
}
+static bool svsm_make_visible(const struct configfs_attribute *attr)
+{
+ return true;
+}
+
static struct tsm_ops sev_tsm_ops = {
.name = KBUILD_MODNAME,
.report_new = sev_report_new,
@@ -1116,6 +1121,14 @@ static int __init sev_guest_probe(struct platform_device *pdev)
if (ret)
goto e_free_cert_data;
+ if (snp_get_vmpl()) {
+ /* Make the SVSM-related attributes visible */
+ tsm_set_visibility("svsm", svsm_make_visible);
+ tsm_set_visibility("service_guid", svsm_make_visible);
+ tsm_set_visibility("service_manifest_version", svsm_make_visible);
+ tsm_set_visibility("manifestblob", svsm_make_visible);
+ }
+
ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
if (ret)
goto e_free_cert_data;
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index 51e02001bb4d..ebb3c642f548 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -64,6 +64,11 @@ static struct tsm_report_state *to_state(struct tsm_report *report)
return container_of(report, struct tsm_report_state, report);
}
+static bool not_visible(const struct configfs_attribute *attr)
+{
+ return false;
+}
+
static int try_advance_write_generation(struct tsm_report *report)
{
struct tsm_report_state *state = to_state(report);
@@ -139,7 +144,7 @@ static ssize_t tsm_report_svsm_store(struct config_item *cfg,
return len;
}
-CONFIGFS_ATTR_WO(tsm_report_, svsm);
+CONFIGFS_ATTR_VISIBLE_WO(tsm_report_, svsm, not_visible);
static ssize_t tsm_report_service_guid_store(struct config_item *cfg,
const char *buf, size_t len)
@@ -168,7 +173,7 @@ static ssize_t tsm_report_service_guid_store(struct config_item *cfg,
return len;
}
-CONFIGFS_ATTR_WO(tsm_report_, service_guid);
+CONFIGFS_ATTR_VISIBLE_WO(tsm_report_, service_guid, not_visible);
static ssize_t tsm_report_service_manifest_version_store(struct config_item *cfg,
const char *buf, size_t len)
@@ -189,7 +194,7 @@ static ssize_t tsm_report_service_manifest_version_store(struct config_item *cfg
return len;
}
-CONFIGFS_ATTR_WO(tsm_report_, service_manifest_version);
+CONFIGFS_ATTR_VISIBLE_WO(tsm_report_, service_manifest_version, not_visible);
static ssize_t tsm_report_inblob_write(struct config_item *cfg,
const void *buf, size_t count)
@@ -336,7 +341,7 @@ static ssize_t tsm_report_manifestblob_read(struct config_item *cfg, void *buf,
return tsm_report_read(report, buf, count, TSM_MANIFEST);
}
-CONFIGFS_BIN_ATTR_RO(tsm_report_, manifestblob, NULL, TSM_OUTBLOB_MAX);
+CONFIGFS_BIN_ATTR_VISIBLE_RO(tsm_report_, manifestblob, NULL, TSM_OUTBLOB_MAX, not_visible);
#define TSM_DEFAULT_ATTRS() \
&tsm_report_attr_generation, \
@@ -444,6 +449,43 @@ static struct configfs_subsystem tsm_configfs = {
.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
};
+int tsm_set_visibility(const char *name, configfs_is_visible_t func)
+{
+ struct configfs_bin_attribute *bin_attr;
+ struct configfs_attribute *attr;
+ const struct config_item_type *t;
+ unsigned int i;
+
+ guard(rwsem_write)(&tsm_rwsem);
+
+ t = provider.type;
+
+ if (t->ct_attrs) {
+ for (i = 0; (attr = t->ct_attrs[i]) != NULL; i++) {
+ if (strcmp(attr->ca_name, name))
+ continue;
+
+ attr->ca_is_visible = func;
+ return 0;
+ }
+ }
+
+ if (t->ct_bin_attrs) {
+ for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+ attr = &bin_attr->cb_attr;
+
+ if (strcmp(attr->ca_name, name))
+ continue;
+
+ attr->ca_is_visible = func;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(tsm_set_visibility);
+
int tsm_register(const struct tsm_ops *ops, void *priv,
const struct config_item_type *type)
{
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index c4aed3059500..01b075a4debc 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -5,6 +5,7 @@
#include <linux/sizes.h>
#include <linux/types.h>
#include <linux/uuid.h>
+#include <linux/configfs.h>
#define TSM_INBLOB_MAX 64
#define TSM_OUTBLOB_MAX SZ_32K
@@ -77,4 +78,7 @@ 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_unregister(const struct tsm_ops *ops);
+
+int tsm_set_visibility(const char *name, configfs_is_visible_t func);
+
#endif /* __TSM_H */
Thanks,
Tom