Provide SEV-SNP support for running under an SVSM

arch/x86/boot/compressed/sev.c void sev_enable(struct boot_params *bp) · line 560

Quoted 6 lines · Tom Lendacky

arch/x86/boot/compressed/sev.c +1 -1

@@ -478,7 +557,7 @@ void sev_enable(struct boot_params *bp)
  	 * Setup/preliminary detection of SNP. This will be sanity-checked
  	 * against CPUID/MSR values later.
  	 */
-	snp = snp_init(bp);
+	snp = snp_setup(bp);

So if you call the compressed/sev.c version early_snp_init() or boot_snp_init() or something similar, you know immediately when it is

called vs having to grep the tree to figure out which is which - was snp_setup() early or was it in the kernel proper. And so on...

During early boot phases, check for the presence of an SVSM when running as an SEV-SNP guest.

An SVSM is present if the 64-bit value at offset 0x148 into the secrets page is non-zero. If an SVSM is present, save the SVSM Calling Area address (CAA), located at offset 0x150 into the secrets page, and set the VMPL level of the guest, which should be non-zero, to indicate the presence of an SVSM.

Signed-off-by Tom Lendacky <[email protected]>

---
 arch/x86/boot/compressed/sev.c    | 35 ++++++++---------
 arch/x86/include/asm/sev-common.h |  4 ++
 arch/x86/include/asm/sev.h        | 25 +++++++++++-
 arch/x86/kernel/sev-shared.c      | 64 +++++++++++++++++++++++++++++++
 arch/x86/kernel/sev.c             | 16 ++++++++
 5 files changed, 125 insertions(+), 19 deletions(-)

arch/x86/boot/compressed/sev.c +17 -18

index bd7222aad540..f54c0ceae1cb 100644
@@ -12,6 +12,7 @@
  */
 #include "misc.h"
 
+#include <linux/mm.h>
 #include <asm/bootparam.h>
 #include <asm/pgtable_types.h>
 #include <asm/sev.h>

@@ -29,6 +30,15 @@
 static struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
 struct ghcb *boot_ghcb;
 
+/*
+ * SVSM related information:
+ *   When running under an SVSM, the VMPL that Linux is executing at must be
+ *   non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
+ */
+static u8 vmpl __section(".data");
+static u64 boot_svsm_caa_pa __section(".data");
+static struct svsm_ca *boot_svsm_caa __section(".data");
+
 /*
  * Copy a version of this function here - insn-eval.c can't be used in
  * pre-decompression code.

@@ -335,24 +345,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
                sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_GEN_REQ);
 }
 
-static bool running_at_vmpl0(void *va)
-{
-       u64 attrs;
-
-       /*
-        * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
-        * higher) privilege level. Here, clear the VMPL1 permission mask of the
-        * GHCB page. If the guest is not running at VMPL0, this will fail.
-        *
-        * If the guest is running at VMPL0, it will succeed. Even if that operation
-        * modifies permission bits, it is still ok to do so currently because Linux
-        * SNP guests running at VMPL0 only run at VMPL0, so VMPL1 or higher
-        * permission mask changes are a don't-care.
-        */
-       attrs = 1;
-       return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
-}
-
 /*
  * SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
  * guest side implementation for proper functioning of the guest. If any

@@ -480,6 +472,13 @@ static bool snp_setup(struct boot_params *bp)
         */
        setup_cpuid_table(cc_info);
 
+       /*
+        * Record the SVSM Calling Area address (CAA) if the guest is not
+        * running at VMPL0. The CA will be used to communicate with the
+        * SVSM to perform the SVSM services.
+        */
+       setup_svsm_ca(cc_info);
+
        /*
         * Pass run-time kernel a pointer to CC info via boot_params so EFI
         * config table doesn't need to be searched again during early startup

arch/x86/include/asm/sev-common.h +4 -0

index b463fcbd4b90..68a8cdf6fd6a 100644
@@ -159,6 +159,10 @@ struct snp_psc_desc {
 #define GHCB_TERM_NOT_VMPL0            3       /* SNP guest is not running at VMPL-0 */
 #define GHCB_TERM_CPUID                        4       /* CPUID-validation failure */
 #define GHCB_TERM_CPUID_HV             5       /* CPUID failure during hypervisor fallback */
+#define GHCB_TERM_SECRETS_PAGE         6       /* Secrets page failure */
+#define GHCB_TERM_NO_SVSM              7       /* SVSM is not advertised in the secrets page */
+#define GHCB_TERM_SVSM_VMPL0           8       /* SVSM is present but has set VMPL to 0 */
+#define GHCB_TERM_SVSM_CAA             9       /* SVSM is present but the CA is not page aligned */
 
 #define GHCB_RESP_CODE(v)              ((v) & GHCB_MSR_INFO_MASK)
 

arch/x86/include/asm/sev.h +24 -1

index 9477b4053bce..891e7d9a1f66 100644
@@ -152,9 +152,32 @@ struct snp_secrets_page_layout {
        u8 vmpck2[VMPCK_KEY_LEN];
        u8 vmpck3[VMPCK_KEY_LEN];
        struct secrets_os_area os_area;
-       u8 rsvd3[3840];
+
+       u8 vmsa_tweak_bitmap[64];
+
+       /* SVSM fields */
+       u64 svsm_base;
+       u64 svsm_size;
+       u64 svsm_caa;
+       u32 svsm_max_version;
+       u8 svsm_guest_vmpl;
+       u8 rsvd3[3];
+
+       /* Remainder of page */
+       u8 rsvd4[3744];
 } __packed;
 
+/*
+ * The SVSM Calling Area (CA) related structures.
+ */
+struct svsm_ca {
+       u8 call_pending;
+       u8 mem_available;
+       u8 rsvd1[6];
+
+       u8 svsm_buffer[PAGE_SIZE - 8];
+};
+
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 extern void __sev_es_ist_enter(struct pt_regs *regs);
 extern void __sev_es_ist_exit(void);

arch/x86/kernel/sev-shared.c +18 -0

index 8b04958da5e7..00bb808be44b 100644
@@ -111,6 +111,24 @@ sev_es_terminate(unsigned int set, unsigned int reason)
                asm volatile("hlt\n" : : : "memory");
 }
 
+static bool running_at_vmpl0(void *va)
+{
+       u64 attrs;
+
+       /*
+        * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
+        * higher) privilege level. Here, clear the VMPL1 permission mask of the
+        * GHCB page. If the guest is not running at VMPL0, this will fail.
+        *
+        * If the guest is running at VMPL0, it will succeed. Even if that operation
+        * modifies permission bits, it is still ok to do so currently because Linux
+        * SNP guests running at VMPL0 only run at VMPL0, so VMPL1 or higher
+        * permission mask changes are a don't-care.
+        */
+       attrs = 1;
+       return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
+}
+
 /*
  * The hypervisor features are available from GHCB version 2 onward.
  */

arch/x86/kernel/sev-shared.c +42 -0

+ */
+static void __init setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
+{
+       struct snp_secrets_page_layout *secrets_page;
+       u64 caa;
+
+       BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
+
+       /*
+        * Use __pa() since this routine is running identity mapped when
+        * called, both by the decompressor code and the early kernel code.
+        */
+       if (running_at_vmpl0((void *)__pa(&boot_ghcb_page)))
+               return;
+
+       /*
+        * Not running at VMPL0, ensure everything has been properly supplied
+        * for running under an SVSM.
+        */
+       if (!cc_info || !cc_info->secrets_phys || cc_info->secrets_len != PAGE_SIZE)
+               sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECRETS_PAGE);
+
+       secrets_page = (struct snp_secrets_page_layout *)cc_info->secrets_phys;
+       if (!secrets_page->svsm_size)
+               sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NO_SVSM);
+
+       if (!secrets_page->svsm_guest_vmpl)
+               sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_VMPL0);
+
+       vmpl = secrets_page->svsm_guest_vmpl;
+
+       caa = secrets_page->svsm_caa;
+       if (!PAGE_ALIGNED(caa))
+               sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CAA);
+
+       /*
+        * The CA is identity mapped when this routine is called, both by the
+        * decompressor code and the early kernel code.
+        */
+       boot_svsm_caa = (struct svsm_ca *)caa;
+       boot_svsm_caa_pa = caa;
+}

arch/x86/kernel/sev.c +16 -1

index b59b09c2f284..64799a04feb4 100644
@@ -135,6 +135,15 @@ struct ghcb_state {
 static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
 static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
 
+/*
+ * SVSM related information:
+ *   When running under an SVSM, the VMPL that Linux is executing at must be
+ *   non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
+ */
+static u8 vmpl __ro_after_init;
+static struct svsm_ca *boot_svsm_caa __ro_after_init;
+static u64 boot_svsm_caa_pa __ro_after_init;
+
 struct sev_config {
        __u64 debug             : 1,
 

@@ -2122,6 +2131,13 @@ bool __head snp_init(struct boot_params *bp)
 
        setup_cpuid_table(cc_info);
 
+       /*
+        * Record the SVSM Calling Area address (CAA) if the guest is not
+        * running at VMPL0. The CA will be used to communicate with the
+        * SVSM to perform the SVSM services.
+        */
+       setup_svsm_ca(cc_info);
+
        /*
         * The CC blob will be used later to access the secrets page. Cache
         * it here like the boot kernel does.
--

2.43.2

Quoted 19 lines · Tom Lendacky

During early boot phases, check for the presence of an SVSM when running as an SEV-SNP guest.

An SVSM is present if the 64-bit value at offset 0x148 into the secrets page is non-zero. If an SVSM is present, save the SVSM Calling Area address (CAA), located at offset 0x150 into the secrets page, and set the VMPL level of the guest, which should be non-zero, to indicate the presence of an SVSM.

Signed-off-by Tom Lendacky <[email protected]>

---
  arch/x86/boot/compressed/sev.c    | 35 ++++++++---------
  arch/x86/include/asm/sev-common.h |  4 ++
  arch/x86/include/asm/sev.h        | 25 +++++++++++-
  arch/x86/kernel/sev-shared.c      | 64 +++++++++++++++++++++++++++++++
  arch/x86/kernel/sev.c             | 16 ++++++++
  5 files changed, 125 insertions(+), 19 deletions(-)

arch/x86/kernel/sev-shared.c static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt, · line 1291

Quoted 4 lines

arch/x86/kernel/sev-shared.c +4 -0

+
+/*
+ * Maintain the GPA of the SVSM Calling Area (CA) in order to utilize the SVSM
+ * services needed when not runnuing in VMPL0.

running*

otherwise lgtm

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_manifest_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 <[email protected]>

---
 Documentation/ABI/testing/configfs-tsm  |  59 ++++++++++
 arch/x86/include/asm/sev.h              |  31 ++++-
 arch/x86/kernel/sev.c                   |  50 ++++++++
 drivers/virt/coco/sev-guest/sev-guest.c | 147 ++++++++++++++++++++++++
 drivers/virt/coco/tsm.c                 |  95 ++++++++++++++-
 include/linux/tsm.h                     |  11 ++
 6 files changed, 390 insertions(+), 3 deletions(-)

Documentation/ABI/testing/configfs-tsm +15 -0

index dd24202b5ba5..a4663610bf7c 100644
@@ -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:	[email protected]
+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
+
 What:		/sys/kernel/config/tsm/report/$name/provider
 Date:		September, 2023
 KernelVersion:	v6.7

@@ -80,3 +95,47 @@ Contact:	[email protected]
 Description:
 		(RO) Indicates the minimum permissible value that can be written

Documentation/ABI/testing/configfs-tsm +11 -0

+		equivalent, e.g. "Y") implies that the attestation report
+		should come from the SVSM.
+		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
+
+What:		/sys/kernel/config/tsm/report/$name/service_guid
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	[email protected]
+Description:
+		(WO) Attribute is visible if a TSM implementation provider

Documentation/ABI/testing/configfs-tsm +14 -0

+What:		/sys/kernel/config/tsm/report/$name/service_manifest_version
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	[email protected]
+Description:
+		(WO) Attribute is visible if a TSM implementation provider
+		supports the concept of attestation reports for TVMs running
+		under an SVSM, like SEV-SNP. Indicates the service manifest
+		version requested for the attestation report. If this field
+		is not set by the user, the default manifest version of the
+		service (the service's initial/first manifest version) is
+		returned. The initial manifest version is always available.
+		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

arch/x86/include/asm/sev.h +30 -1

index 34bc84aee969..76fabc7b5e97 100644
@@ -209,6 +209,27 @@ struct svsm_pvalidate_call {
 	struct svsm_pvalidate_entry entry[];
 };
 
+/*
+ * The SVSM Attestation related structures
+ */
+struct svsm_location_entry {
+	u64 pa;
+	u32 len;
+	u8 rsvd[4];
+};
+
+struct svsm_attestation_call {
+	struct svsm_location_entry report_buffer;
+	struct svsm_location_entry nonce;
+	struct svsm_location_entry manifest_buffer;
+	struct svsm_location_entry certificates_buffer;
+
+	/* For attesting a single service */
+	u8 service_guid[16];
+	u32 service_manifest_version;
+	u8 rsvd[4];
+};
+
 /*
  * SVSM protocol structure
  */

@@ -232,6 +253,10 @@ struct svsm_call {
 #define SVSM_CORE_CREATE_VCPU		2
 #define SVSM_CORE_DELETE_VCPU		3
 
+#define SVSM_ATTEST_CALL(x)		((1ULL << 32) | (x))
+#define SVSM_ATTEST_SERVICES		0
+#define SVSM_ATTEST_SINGLE_SERVICE	1
+
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 extern void __sev_es_ist_enter(struct pt_regs *regs);
 extern void __sev_es_ist_exit(void);

@@ -302,6 +327,7 @@ void snp_set_wakeup_secondary_cpu(void);
 bool snp_init(struct boot_params *bp);
 void __noreturn snp_abort(void);
 int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
+int snp_issue_svsm_attestation_request(u64 call_id, struct svsm_attestation_call *input);
 void snp_accept_memory(phys_addr_t start, phys_addr_t end);
 u64 snp_get_unsupported_features(u64 status);
 u64 sev_get_status(void);

@@ -333,7 +359,10 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
 {
 	return -ENOTTY;
 }
-
+static inline int snp_issue_svsm_attestation_request(u64 call_id, struct svsm_attestation_call *input)
+{
+	return -ENOTTY;
+}
 static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
 static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
 static inline u64 sev_get_status(void) { return 0; }

arch/x86/kernel/sev.c +50 -0

index 8682af55802c..4e460d9eba77 100644
@@ -2402,6 +2402,56 @@ static int __init init_sev_config(char *str)
 }
 __setup("sev=", init_sev_config);
 
+static void update_attestation_input(struct svsm_call *call, struct svsm_attestation_call *input)
+{
+	/* If (new) lengths have been returned, propograte them up */
+	if (call->rcx_out != call->rcx)
+		input->manifest_buffer.len = call->rcx_out;
+
+	if (call->rdx_out != call->rdx)
+		input->certificates_buffer.len = call->rdx_out;
+
+	if (call->r8_out != call->r8)
+		input->report_buffer.len = call->r8_out;
+}
+
+int snp_issue_svsm_attestation_request(u64 call_id, struct svsm_attestation_call *input)
+{
+	struct svsm_attestation_call *attest_call;
+	struct svsm_call call = {};
+	unsigned long flags;
+	u64 attest_call_pa;
+	int ret;
+
+	if (!vmpl)
+		return -EINVAL;
+
+	local_irq_save(flags);
+
+	call.caa = __svsm_get_caa();
+
+	attest_call = (struct svsm_attestation_call *)call.caa->svsm_buffer;
+	attest_call_pa = __svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
+
+	*attest_call = *input;
+
+	/*
+	 * Set input registers for the request and set RDX and R8 to known
+	 * values in order to detect length values being returned in them.
+	 */
+	call.rax = call_id;
+	call.rcx = attest_call_pa;
+	call.rdx = -1;
+	call.r8 = -1;
+	ret = svsm_protocol(&call);
+	update_attestation_input(&call, input);
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snp_issue_svsm_attestation_request);
+
 int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)
 {
 	struct ghcb_state state;

drivers/virt/coco/sev-guest/sev-guest.c +48 -0

index bba6531cb606..9daec0ea386e 100644
@@ -38,6 +38,8 @@
 #define SNP_REQ_MAX_RETRY_DURATION	(60*HZ)
 #define SNP_REQ_RETRY_DELAY		(2*HZ)
 
+#define SVSM_MAX_RETRIES		3
+
 struct snp_guest_crypto {
 	struct crypto_aead *tfm;
 	u8 *iv, *authtag;

@@ -783,6 +785,148 @@ struct snp_msg_cert_entry {
 	u32 length;
 };
 
+static int sev_svsm_report_new(struct tsm_report *report, void *data)
+{
+	unsigned int report_len, manifest_len, certificates_len;
+	void *report_blob, *manifest_blob, *certificates_blob;
+	struct svsm_attestation_call attest_call = {};
+	struct tsm_desc *desc = &report->desc;
+	unsigned int retry_count;
+	unsigned int size;
+	bool try_again;
+	void *buffer;
+	u64 call_id;
+	int ret;
+
+	/*
+	 * Allocate pages for the request:
+	 * - Report blob (4K)
+	 * - Manifest blob (4K)
+	 * - Certificate blob (16K)
+	 *
+	 * Above addresses must be 4K aligned
+	 */
+	report_len = SZ_4K;
+	manifest_len = SZ_4K;
+	certificates_len = SEV_FW_BLOB_MAX_SIZE;
+
+	retry_count = 0;
+
+retry:
+	size = report_len + manifest_len + certificates_len;
+	buffer = alloc_pages_exact(size, __GFP_ZERO);
+	if (!buffer)
+		return -ENOMEM;
+
+	report_blob = buffer;
+	attest_call.report_buffer.pa = __pa(report_blob);
+	attest_call.report_buffer.len = report_len;
+
+	manifest_blob = report_blob + report_len;
+	attest_call.manifest_buffer.pa = __pa(manifest_blob);
+	attest_call.manifest_buffer.len = manifest_len;
+
+	certificates_blob = manifest_blob + manifest_len;
+	attest_call.certificates_buffer.pa = __pa(certificates_blob);
+	attest_call.certificates_buffer.len = certificates_len;
+
+	attest_call.nonce.pa = __pa(desc->inblob);

drivers/virt/coco/sev-guest/sev-guest.c +89 -0

+
+	ret = snp_issue_svsm_attestation_request(call_id, &attest_call);
+	switch (ret) {
+	case SVSM_SUCCESS:
+		break;
+	case SVSM_ERR_INVALID_PARAMETER:
+		ret = -EINVAL;
+
+		if (retry_count >= SVSM_MAX_RETRIES)
+			goto error;
+
+		try_again = false;
+
+		if (attest_call.report_buffer.len > report_len) {
+			report_len = PAGE_ALIGN(attest_call.report_buffer.len);
+			try_again = true;
+		}
+
+		if (attest_call.manifest_buffer.len > manifest_len) {
+			manifest_len = PAGE_ALIGN(attest_call.manifest_buffer.len);
+			try_again = true;
+		}
+
+		if (attest_call.certificates_buffer.len > certificates_len) {
+			certificates_len = PAGE_ALIGN(attest_call.certificates_buffer.len);
+			try_again = true;
+		}
+
+		/* If one of the buffers wasn't large enough, retry the request */
+		if (try_again) {
+			free_pages_exact(buffer, size);
+			retry_count++;
+			goto retry;
+		}
+
+		goto error;
+	case SVSM_ERR_BUSY:
+		ret = -EAGAIN;
+		goto error;
+	default:
+		pr_err_ratelimited("SVSM attestation request failed (%#x)\n", ret);
+		ret = -EINVAL;
+		goto error;
+	}
+
+	ret = -ENOMEM;
+
+	report_len = attest_call.report_buffer.len;
+	void *rbuf __free(kvfree) = kvzalloc(report_len, GFP_KERNEL);
+	if (!rbuf)
+		goto error;
+
+	memcpy(rbuf, report_blob, report_len);
+	report->outblob = no_free_ptr(rbuf);
+	report->outblob_len = report_len;
+
+	manifest_len = attest_call.manifest_buffer.len;
+	void *mbuf __free(kvfree) = kvzalloc(manifest_len, GFP_KERNEL);
+	if (!mbuf)
+		goto error;
+
+	memcpy(mbuf, manifest_blob, manifest_len);
+	report->manifestblob = no_free_ptr(mbuf);
+	report->manifestblob_len = manifest_len;
+
+	certificates_len = attest_call.certificates_buffer.len;
+	if (!certificates_len)
+		goto success;
+
+	void *cbuf __free(kvfree) = kvzalloc(certificates_len, GFP_KERNEL);
+	if (!cbuf)
+		goto error;
+
+	memcpy(cbuf, certificates_blob, certificates_len);
+	report->auxblob = no_free_ptr(cbuf);
+	report->auxblob_len = certificates_len;
+
+success:
+	ret = 0;
+
+error:
+	free_pages_exact(buffer, size);
+
+	return ret;
+}
+
 static int sev_report_new(struct tsm_report *report, void *data)
 {
 	struct snp_msg_cert_entry *cert_table;

@@ -797,6 +941,9 @@ static int sev_report_new(struct tsm_report *report, void *data)
 	if (desc->inblob_len != SNP_REPORT_USER_DATA_SIZE)
 		return -EINVAL;
 
+	if (desc->svsm)
+		return sev_svsm_report_new(report, data);
+
 	void *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;

drivers/virt/coco/tsm.c +30 -1

index d1c2db83a8ca..07b4c95ce704 100644
@@ -35,7 +35,7 @@ static DECLARE_RWSEM(tsm_rwsem);
  * The attestation report format is TSM provider specific, when / if a standard
  * materializes that can be published instead of the vendor layout. Until then
  * the 'provider' attribute indicates the format of 'outblob', and optionally
- * 'auxblob'.
+ * 'auxblob' and 'manifestblob'.
  */
 
 struct tsm_report_state {

@@ -48,6 +48,7 @@ struct tsm_report_state {
 enum tsm_data_select {
 	TSM_REPORT,
 	TSM_CERTS,
+	TSM_MANIFEST,
 };
 
 static struct tsm_report *to_tsm_report(struct config_item *cfg)

@@ -119,6 +120,77 @@ static ssize_t tsm_report_privlevel_floor_show(struct config_item *cfg,
 }
 CONFIGFS_ATTR_RO(tsm_report_, privlevel_floor);
 
+static ssize_t tsm_report_svsm_store(struct config_item *cfg,
+				     const char *buf, size_t len)
+{
+	struct tsm_report *report = to_tsm_report(cfg);
+	bool val;
+	int rc;
+
+	rc = kstrtobool(buf, &val);
+	if (rc)
+		return rc;
+
+	guard(rwsem_write)(&tsm_rwsem);
+	rc = try_advance_write_generation(report);
+	if (rc)
+		return rc;
+	report->desc.svsm = val;
+
+	return len;
+}
+CONFIGFS_ATTR_WO(tsm_report_, svsm);
+
+static ssize_t tsm_report_service_guid_store(struct config_item *cfg,
+					     const char *buf, size_t len)
+{
+	struct tsm_report *report = to_tsm_report(cfg);
+	size_t guid_len;
+	int rc;
+

drivers/virt/coco/tsm.c +47 -1

+	}
+
+	return len;
+}
+CONFIGFS_ATTR_WO(tsm_report_, service_guid);
+
+static ssize_t tsm_report_service_manifest_version_store(struct config_item *cfg,
+							 const char *buf, size_t len)
+{
+	struct tsm_report *report = to_tsm_report(cfg);
+	unsigned int val;
+	int rc;
+
+	rc = kstrtouint(buf, 0, &val);
+	if (rc)
+		return rc;
+
+	guard(rwsem_write)(&tsm_rwsem);
+	rc = try_advance_write_generation(report);
+	if (rc)
+		return rc;
+	report->desc.service_manifest_version = val;
+
+	return len;
+}
+CONFIGFS_ATTR_WO(tsm_report_, service_manifest_version);
+
 static ssize_t tsm_report_inblob_write(struct config_item *cfg,
 				       const void *buf, size_t count)
 {

@@ -163,6 +235,9 @@ static ssize_t __read_report(struct tsm_report *report, void *buf, size_t count,
 	if (select == TSM_REPORT) {
 		out = report->outblob;
 		len = report->outblob_len;
+	} else if (select == TSM_MANIFEST) {
+		out = report->manifestblob;
+		len = report->manifestblob_len;
 	} else {
 		out = report->auxblob;
 		len = report->auxblob_len;

@@ -188,7 +263,7 @@ static ssize_t read_cached_report(struct tsm_report *report, void *buf,
 
 	/*
 	 * A given TSM backend always fills in ->outblob regardless of
-	 * whether the report includes an auxblob or not.
+	 * whether the report includes an auxblob/manifestblob or not.
 	 */
 	if (!report->outblob ||
 	    state->read_generation != state->write_generation)

@@ -224,8 +299,10 @@ static ssize_t tsm_report_read(struct tsm_report *report, void *buf,
 
 	kvfree(report->outblob);
 	kvfree(report->auxblob);
+	kvfree(report->manifestblob);
 	report->outblob = NULL;
 	report->auxblob = NULL;
+	report->manifestblob = NULL;
 	rc = ops->report_new(report, provider.data);
 	if (rc < 0)
 		return rc;

@@ -252,6 +329,15 @@ static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
 }
 CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_OUTBLOB_MAX);
 
+static ssize_t tsm_report_manifestblob_read(struct config_item *cfg, void *buf,
+					    size_t count)
+{
+	struct tsm_report *report = to_tsm_report(cfg);
+
+	return tsm_report_read(report, buf, count, TSM_MANIFEST);
+}
+CONFIGFS_BIN_ATTR_RO(tsm_report_, manifestblob, NULL, TSM_OUTBLOB_MAX);
+
 #define TSM_DEFAULT_ATTRS() \
 	&tsm_report_attr_generation, \
 	&tsm_report_attr_provider

@@ -265,6 +351,9 @@ static struct configfs_attribute *tsm_report_extra_attrs[] = {
 	TSM_DEFAULT_ATTRS(),
 	&tsm_report_attr_privlevel,
 	&tsm_report_attr_privlevel_floor,
+	&tsm_report_attr_svsm,
+	&tsm_report_attr_service_guid,
+	&tsm_report_attr_service_manifest_version,
 	NULL,
 };
 

@@ -280,6 +369,7 @@ static struct configfs_bin_attribute *tsm_report_bin_attrs[] = {
 static struct configfs_bin_attribute *tsm_report_bin_extra_attrs[] = {
 	TSM_DEFAULT_BIN_ATTRS(),
 	&tsm_report_attr_auxblob,
+	&tsm_report_attr_manifestblob,
 	NULL,
 };
 

@@ -288,6 +378,7 @@ static void tsm_report_item_release(struct config_item *cfg)
 	struct tsm_report *report = to_tsm_report(cfg);
 	struct tsm_report_state *state = to_state(report);
 
+	kvfree(report->manifestblob);
 	kvfree(report->auxblob);
 	kvfree(report->outblob);
 	kfree(state);

include/linux/tsm.h +11 -0

index 50c5769657d8..c4aed3059500 100644
@@ -4,6 +4,7 @@
 
 #include <linux/sizes.h>
 #include <linux/types.h>
+#include <linux/uuid.h>
 
 #define TSM_INBLOB_MAX 64
 #define TSM_OUTBLOB_MAX SZ_32K

@@ -19,11 +20,17 @@
  * @privlevel: optional privilege level to associate with @outblob
  * @inblob_len: sizeof @inblob
  * @inblob: arbitrary input data
+ * @svsm: optional indicator of where to obtain the tsm report blob
+ * @service_guid: optional SVSM service guid to attest
+ * @service_manifest_version: optional SVSM service manifest version requested
  */
 struct tsm_desc {
 	unsigned int privlevel;
 	size_t inblob_len;
 	u8 inblob[TSM_INBLOB_MAX];
+	bool svsm;
+	guid_t service_guid;
+	unsigned int service_manifest_version;
 };
 
 /**

@@ -33,6 +40,8 @@ struct tsm_desc {
  * @outblob: generated evidence to provider to the attestation agent
  * @auxblob_len: sizeof(@auxblob)
  * @auxblob: (optional) auxiliary data to the report (e.g. certificate data)
+ * @manifestblob_len: sizeof(@manifestblob)
+ * @manifestblob: (optional) manifest data associated with the report
  */
 struct tsm_report {
 	struct tsm_desc desc;

@@ -40,6 +49,8 @@ struct tsm_report {
 	u8 *outblob;
 	size_t auxblob_len;
 	u8 *auxblob;
+	size_t manifestblob_len;
+	u8 *manifestblob;
 };
 
 /**

Quoted 40 lines · Tom Lendacky

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_manifest_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 <[email protected]>

---
  Documentation/ABI/testing/configfs-tsm  |  59 ++++++++++
  arch/x86/include/asm/sev.h              |  31 ++++-
  arch/x86/kernel/sev.c                   |  50 ++++++++
  drivers/virt/coco/sev-guest/sev-guest.c | 147 ++++++++++++++++++++++++
  drivers/virt/coco/tsm.c                 |  95 ++++++++++++++-
  include/linux/tsm.h                     |  11 ++
  6 files changed, 390 insertions(+), 3 deletions(-)

Documentation/ABI/testing/configfs-tsm +0 -0

index dd24202b5ba5..a4663610bf7c 100644

Documentation/ABI/testing/configfs-tsm Contact: [email protected] · line 106

Quoted 9 lines

Documentation/ABI/testing/configfs-tsm +9 -0

+
+What:		/sys/kernel/config/tsm/report/$name/svsm
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	[email protected]
+Description:
+		(WO) Attribute is visible if a TSM implementation provider
+		supports the concept of attestation reports for TVMs running
+		under an SVSM, like SEV-SNP. Specifying a 1 (or other boolean

Since service_guid can be used for non SVSM services as well, can we use a generic term "service" here? And let user specify the service type (like service=svsm)

Quoted 17 lines

Documentation/ABI/testing/configfs-tsm +16 -0

+		equivalent, e.g. "Y") implies that the attestation report
+		should come from the SVSM.
+		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
+
+What:		/sys/kernel/config/tsm/report/$name/service_guid
+Date:		January, 2024
+KernelVersion:	v6.9
+Contact:	[email protected]
+Description:
+		(WO) Attribute is visible if a TSM implementation provider
+		supports the concept of attestation reports for TVMs running
+		under an SVSM, like SEV-SNP. Specifying a empty or null GUID
+		(00000000-0000-0000-0000-000000) requests all active services
+		within the SVSM be part of the attestation report. Specifying
+		a non-null GUID requests an attestation report of just the

Documentation/ABI/testing/configfs-tsm Contact: [email protected] · line 127

Quoted 5 lines

Documentation/ABI/testing/configfs-tsm +5 -0

+		specified service using the manifest form specified by the
+		service_manifest_version attribute.
+		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 think it will be useful to the user if there is a attribute to list the service GUIDs supported. It can help prevent user using incorrect or unsupported GUIDs.

Documentation/ABI/testing/configfs-tsm

Quoted 8 lines

Quoted 1 line

drivers/virt/coco/sev-guest/sev-guest.c +1 -0

+	if (guid_is_null(&desc->service_guid)) {

Documentation/ABI/testing/configfs-tsm +7 -0

+		call_id = SVSM_ATTEST_CALL(SVSM_ATTEST_SERVICES);
+	} else {
+		export_guid(attest_call.service_guid, &desc->service_guid);
+		attest_call.service_manifest_version = desc->service_manifest_version;
+
+		call_id = SVSM_ATTEST_CALL(SVSM_ATTEST_SINGLE_SERVICE);
+	}

Above initialization will not change during retry, right? Why not move it above retry?

Documentation/ABI/testing/configfs-tsm

Quoted 8 lines

Documentation/ABI/testing/configfs-tsm +6 -0

+
+	/* Obtain the GUID string length */
+	guid_len = (len && buf[len - 1] == '\n') ? len - 1 : len;
+	if (guid_len && guid_len != UUID_STRING_LEN)
+		return -EINVAL;
+

I don't think you need above checks. I think guid_parse will fail, if it is not a valid GUID.

Documentation/ABI/testing/configfs-tsm

Quoted 7 lines

Documentation/ABI/testing/configfs-tsm +6 -0

+	if (guid_len == UUID_STRING_LEN) {
+		rc = guid_parse(buf, &report->desc.service_guid);
+		if (rc)
+			return rc;
+	} else {
+		report->desc.service_guid = guid_null;

I think the default value will be guid_null right, why reset it to NULL for every failed attempt?

Quoted 43 lines · Sathyanarayanan

Quoted 40 lines · Tom Lendacky

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_manifest_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 <[email protected]>

---
  Documentation/ABI/testing/configfs-tsm  |  59 ++++++++++ arch/x86/include/asm/sev.h              |  31 ++++- arch/x86/kernel/sev.c                   |  50 ++++++++ drivers/virt/coco/sev-guest/sev-guest.c | 147 ++++++++++++++++++++++++ drivers/virt/coco/tsm.c                 |  95 ++++++++++++++- include/linux/tsm.h                     |  11 ++

  6 files changed, 390 insertions(+), 3 deletions(-)

Documentation/ABI/testing/configfs-tsm +0 -0

index dd24202b5ba5..a4663610bf7c 100644

Quoted 13 lines

Documentation/ABI/testing/configfs-tsm Contact: [email protected] · line 106

Quoted 9 lines

Documentation/ABI/testing/configfs-tsm +9 -0

+
+What:        /sys/kernel/config/tsm/report/$name/svsm
+Date:        January, 2024
+KernelVersion:    v6.9
+Contact:    [email protected]
+Description:
+        (WO) Attribute is visible if a TSM implementation provider
+        supports the concept of attestation reports for TVMs running
+        under an SVSM, like SEV-SNP. Specifying a 1 (or other boolean

Since service_guid can be used for non SVSM services as well, can we use a generic term "service" here? And let user specify the service type (like service=svsm)

I suppose that's possible. I think we would need a better term than just service, though, since service_guid is specific to a service within the service provider... so maybe service_provider.

Quoted 18 lines

Quoted 18 lines

+        equivalent, e.g. "Y") implies that the attestation report
+        should come from the SVSM.
+        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

Documentation/ABI/testing/configfs-tsm +12 -0

+
+What:        /sys/kernel/config/tsm/report/$name/service_guid
+Date:        January, 2024
+KernelVersion:    v6.9
+Contact:    [email protected]
+Description:
+        (WO) Attribute is visible if a TSM implementation provider
+        supports the concept of attestation reports for TVMs running
+        under an SVSM, like SEV-SNP. Specifying a empty or null GUID
+        (00000000-0000-0000-0000-000000) requests all active services
+        within the SVSM be part of the attestation report. Specifying
+        a non-null GUID requests an attestation report of just the

Quoted 8 lines

Quoted 5 lines

+        specified service using the manifest form specified by the
+        service_manifest_version attribute.
+        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 think it will be useful to the user if there is a attribute to list the service GUIDs supported. It can help prevent user using incorrect or unsupported GUIDs.

A list of supported GUIDs can be obtained from the manifest of a all-services attestation request.

Quoted 10 lines

 >> +    if (guid_is_null(&desc->service_guid)) {

Quoted 7 lines

+        call_id = SVSM_ATTEST_CALL(SVSM_ATTEST_SERVICES);
+    } else {
+        export_guid(attest_call.service_guid, &desc->service_guid); +        attest_call.service_manifest_version = desc->service_manifest_version; +
+        call_id = SVSM_ATTEST_CALL(SVSM_ATTEST_SINGLE_SERVICE);
+    }

Quoted 2 lines

Above initialization will not change during retry, right? Why not move it above retry?

True, will move it outside of the loop.

Quoted 11 lines

Documentation/ABI/testing/configfs-tsm

Quoted 8 lines

Documentation/ABI/testing/configfs-tsm +6 -0

+
+    /* Obtain the GUID string length */
+    guid_len = (len && buf[len - 1] == '\n') ? len - 1 : len;
+    if (guid_len && guid_len != UUID_STRING_LEN)
+        return -EINVAL;
+

I don't think you need above checks. I think guid_parse will fail, if it is not a valid GUID.

Yes and no. The guid_parse() function will succeed if the string is longer than UUID_STRING_LEN as long as it is a valid UUID up to UUID_STRING_LEN. In other words, guid_parse() of:

    aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee

and
    aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee-gg

both succeed.

I'm ok with eliminating the length calculation and check if everyone is in favor of doing that given the above behavior.

Quoted 9 lines

Quoted 8 lines

+    if (guid_len == UUID_STRING_LEN) {
+        rc = guid_parse(buf, &report->desc.service_guid); +        if (rc)
+            return rc;
+    } else {
+        report->desc.service_guid = guid_null;

Quoted 1 line

I think the default value will be guid_null right, why reset it to NULL for every failed attempt?

Default, yes. But what if it is written once, then a second time with an invalid GUID. Should the previously written GUID still be used?

Thanks,
Tom

Quoted 1 line

Quoted 66 lines · Tom Lendacky

Quoted 64 lines · Sathyanarayanan

Quoted 49 lines · Tom Lendacky

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_manifest_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 <[email protected]>

---
   Documentation/ABI/testing/configfs-tsm  |  59 ++++++++++
   arch/x86/include/asm/sev.h              |  31 ++++-
   arch/x86/kernel/sev.c                   |  50 ++++++++
   drivers/virt/coco/sev-guest/sev-guest.c | 147 ++++++++++++++++++++++++
   drivers/virt/coco/tsm.c                 |  95 ++++++++++++++-
   include/linux/tsm.h                     |  11 ++
   6 files changed, 390 insertions(+), 3 deletions(-)

Documentation/ABI/testing/configfs-tsm +7 -0

index dd24202b5ba5..a4663610bf7c 100644
 
 
+
+What:        /sys/kernel/config/tsm/report/$name/service_guid
+Date:        January, 2024
+KernelVersion:    v6.9
+Contact:    [email protected]
+Description:
+        (WO) Attribute is visible if a TSM implementation provider

Documentation/ABI/testing/configfs-tsm Contact: [email protected] · line 127

Quoted 10 lines

Documentation/ABI/testing/configfs-tsm +10 -0

+        supports the concept of attestation reports for TVMs running
+        under an SVSM, like SEV-SNP. Specifying a empty or null GUID
+        (00000000-0000-0000-0000-000000) requests all active services
+        within the SVSM be part of the attestation report. Specifying
+        a non-null GUID requests an attestation report of just the
+        specified service using the manifest form specified by the
+        service_manifest_version attribute.
+        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 think it will be useful to the user if there is a attribute to list the service GUIDs supported. It can help prevent user using incorrect or unsupported GUIDs.

Quoted 1 line

A list of supported GUIDs can be obtained from the manifest of a all-services attestation request.

So they have to make a request twice? Once with a NULL GUID to get the manifest with all service list, and another to make service-specific request? There should be a fixed list of service GUIDs, right? Why not list them by default?

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

+    if (guid_len == UUID_STRING_LEN) {
+        rc = guid_parse(buf, &report->desc.service_guid); +        if (rc)
+            return rc;
+    } else {
+        report->desc.service_guid = guid_null;

Quoted 4 lines

Quoted 1 line

I think the default value will be guid_null right, why reset it to NULL for every failed attempt?

Default, yes. But what if it is written once, then a second time with an invalid GUID. Should the previously written GUID still be used?

If the user write fails, why update the state? IMO, we can leave it at the old value. But, lets see what others think.

Quoted 5 lines

Thanks,
Tom

Quoted 1 line

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 is displayed by default.

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
  fs/configfs/file.c       |  13 +++++
  include/linux/configfs.h | 114 +++++++++++++++++++++++++++------------
  2 files changed, 93 insertions(+), 34 deletions(-)

fs/configfs/file.c +0 -0

index 0ad32150611e..c758bcc11235 100644

fs/configfs/file.c +13 -0

  	int error = 0;
 
+	if (attr->ca_is_visible) {
+		mode = attr->ca_is_visible(item, attr);
+		if (!mode)
+			return 0;
+	}
+
  	inode_lock_nested(d_inode(dir), I_MUTEX_NORMAL);
  	error = configfs_make_dirent(parent_sd, NULL, (void *) attr, mode,
  				     CONFIGFS_ITEM_ATTR, parent_sd->s_frag);

@@ -470,9 +476,16 @@ int configfs_create_bin_file(struct config_item *item,
  {
  	struct dentry *dir = item->ci_dentry;
  	struct configfs_dirent *parent_sd = dir->d_fsdata;
+	const struct configfs_attribute *attr = &bin_attr->cb_attr;
  	umode_t mode = (bin_attr->cb_attr.ca_mode & S_IALLUGO) | S_IFREG;
  	int error = 0;
 
+	if (attr->ca_is_visible) {
+		mode = attr->ca_is_visible(item, attr);
+		if (!mode)
+			return 0;
+	}
+
  	inode_lock_nested(dir->d_inode, I_MUTEX_NORMAL);
  	error = configfs_make_dirent(parent_sd, NULL, (void *) bin_attr, mode,
  				     CONFIGFS_ITEM_BIN_ATTR, parent_sd->s_frag);

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 is displayed by default.

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
 fs/configfs/file.c       |  13 +++++
 include/linux/configfs.h | 114 +++++++++++++++++++++++++++------------
 2 files changed, 93 insertions(+), 34 deletions(-)

fs/configfs/file.c +0 -0

index 0ad32150611e..c758bcc11235 100644

Quoted 4 lines · Tom Lendacky

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 is displayed by default.

I'm curious what the strong value is in this extra callback. As opposed to not generating the attribute in the absence of a TPM (why create a config_item at all?), merely having an empty response from the attribute, or having ->show() return -ENODEV or similar.

Quoted 18 lines

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
  fs/configfs/file.c       |  13 +++++
  include/linux/configfs.h | 114 +++++++++++++++++++++++++++------------
  2 files changed, 93 insertions(+), 34 deletions(-)

fs/configfs/file.c +0 -0

index 0ad32150611e..c758bcc11235 100644
@@ -451,6 +451,12 @@ int configfs_create_file(struct config_item * item, const struct configfs_attrib
  	umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG;
  	int error = 0;
 

fs/configfs/file.c int configfs_create_file(struct config_item * item, const struct configfs_attrib · line 457

Quoted 4 lines

fs/configfs/file.c +4 -0

+	if (attr->ca_is_visible) {
+		mode = attr->ca_is_visible(item, attr);
+		if (!mode)
+			return 0;

What value do we get from carrying the mode through here? The API proposed is "visible or not", which is a boolean. Overloading that with "also set the mode" is confusing, and it also can lead to the divergent codepath problem you mentioned in your response, where ->ca_is_visible() fails to return the mode correctly. If this was simpl a boolean hook, the code could read like so:

```
	umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG;
	int error = 0;

	if (attr->ca_is_visible && !attr->ca_is_visible(item, attr))
    		return 0;
```

Quoted 8 lines

include/linux/configfs.h +0 -0

index 2606711adb18..18011f78ffde 100644
@@ -112,39 +112,64 @@ static inline void configfs_add_default_group(struct config_group *new_group,
  	list_add_tail(&new_group->group_entry, &group->default_groups);
  }
 

include/linux/configfs.h static inline void configfs_add_default_group(struct config_group *new_group, · line 117

Quoted 3 lines

include/linux/configfs.h +3 -0

+typedef umode_t (*configfs_is_visible_t)(const struct config_item *item,
+					 const struct configfs_attribute *attr);
+

We don't use typedefs of op functions anywhere else in configfs or frankly the entire filesystem API. Adding one here would just introduce confusion.

include/linux/configfs.h static inline void configfs_add_default_group(struct config_group *new_group, · line 126

Quoted 10 lines

include/linux/configfs.h +1 -0

  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);
  };
 

Thanks,
Joel

Quoted 10 lines · Joel Becker

Quoted 4 lines · Tom Lendacky

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 is displayed by default.

I'm curious what the strong value is in this extra callback. As opposed to not generating the attribute in the absence of a TPM (why create a config_item at all?), merely having an empty response from the attribute, or having ->show() return -ENODEV or similar.

The value is to reduce the complexity of registering with the TSM support across multiple vendors. There is a base set of attributes that are common across vendors and some that are specific to vendors. Creating this structure in the TSM support can get unwieldy. This would make it simple to determine if support is provided since the attribute will either be present or not.

This would also make the support similar to sysfs in the ability to dynamically hide or show attributes.

Quoted 18 lines

Quoted 18 lines

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
  fs/configfs/file.c       |  13 +++++
  include/linux/configfs.h | 114 +++++++++++++++++++++++++++------------
  2 files changed, 93 insertions(+), 34 deletions(-)

fs/configfs/file.c +0 -0

index 0ad32150611e..c758bcc11235 100644
@@ -451,6 +451,12 @@ int configfs_create_file(struct config_item * item, const struct configfs_attrib
  	umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG;
  	int error = 0;
 

Quoted 11 lines

fs/configfs/file.c int configfs_create_file(struct config_item * item, const struct configfs_attrib · line 457

Quoted 4 lines

fs/configfs/file.c +4 -0

+	if (attr->ca_is_visible) {
+		mode = attr->ca_is_visible(item, attr);
+		if (!mode)
+			return 0;

What value do we get from carrying the mode through here? The API proposed is "visible or not", which is a boolean. Overloading that with "also set the mode" is confusing, and it also can lead to the divergent codepath problem you mentioned in your response, where ->ca_is_visible() fails to return the mode correctly. If this was simpl a boolean hook, the code could read like so:

A boolean would work. There was a request to make this similar to the sysfs attribute visibility. I certainly can make this a simple bool function if that is preferable.

Config-fs provides support to hide individual attribute entries. Using this support, base the display of the SVSM related entries on the presence of an SVSM.

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
 arch/x86/coco/core.c        |  4 ++++
 drivers/virt/coco/tsm.c     | 13 +++++++++----
 include/linux/cc_platform.h |  8 ++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

arch/x86/coco/core.c +4 -0

index d07be9d05cd0..efa0f648f754 100644
@@ -12,6 +12,7 @@
 
 #include <asm/coco.h>
 #include <asm/processor.h>
+#include <asm/sev.h>
 
 enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
 u64 cc_mask __ro_after_init;

@@ -78,6 +79,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr)
 	case CC_ATTR_GUEST_STATE_ENCRYPT:
 		return sev_status & MSR_AMD64_SEV_ES_ENABLED;
 
+	case CC_ATTR_GUEST_SVSM_PRESENT:
+		return snp_get_vmpl();
+
 	/*
 	 * With SEV, the rep string I/O instructions need to be unrolled
 	 * but SEV-ES supports them through the #VC handler.

drivers/virt/coco/tsm.c +0 -0

index 07b4c95ce704..2efa6e578477 100644

drivers/virt/coco/tsm.c +4 -4

 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, svsm_visibility);
 
 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, svsm_visibility);
 
 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, svsm_visibility);
 
 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, svsm_visibility);
 
 #define TSM_DEFAULT_ATTRS() \
 	&tsm_report_attr_generation, \

include/linux/cc_platform.h +8 -0

index cb0d6cd1c12f..f1b4266c1484 100644
@@ -90,6 +90,14 @@ enum cc_attr {
 	 * Examples include TDX Guest.
 	 */
 	CC_ATTR_HOTPLUG_DISABLED,
+
+	/**
+	 * @CC_ATTR_GUEST_SVSM_PRESENT: Guest is running under an SVSM
+	 *
+	 * The platform/OS is running as a guest/virtual machine and is
+	 * running under a Secure VM Service Module (SVSM).
+	 */
+	CC_ATTR_GUEST_SVSM_PRESENT,
 };
 
 #ifdef CONFIG_ARCH_HAS_CC_PLATFORM

Quoted 43 lines · Tom Lendacky

Config-fs provides support to hide individual attribute entries. Using this support, base the display of the SVSM related entries on the presence of an SVSM.

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
  arch/x86/coco/core.c        |  4 ++++
  drivers/virt/coco/tsm.c     | 13 +++++++++----
  include/linux/cc_platform.h |  8 ++++++++
  3 files changed, 21 insertions(+), 4 deletions(-)

arch/x86/coco/core.c +4 -0

index d07be9d05cd0..efa0f648f754 100644
@@ -12,6 +12,7 @@
 
  #include <asm/coco.h>
  #include <asm/processor.h>
+#include <asm/sev.h>
 
  enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
  u64 cc_mask __ro_after_init;

@@ -78,6 +79,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr)
  	case CC_ATTR_GUEST_STATE_ENCRYPT:
  		return sev_status & MSR_AMD64_SEV_ES_ENABLED;
 
+	case CC_ATTR_GUEST_SVSM_PRESENT:
+		return snp_get_vmpl();
+
  	/*
  	 * With SEV, the rep string I/O instructions need to be unrolled
  	 * but SEV-ES supports them through the #VC handler.

drivers/virt/coco/tsm.c +0 -0

index 07b4c95ce704..2efa6e578477 100644
@@ -64,6 +64,11 @@ static struct tsm_report_state *to_state(struct tsm_report *report)
  	return container_of(report, struct tsm_report_state, report);
  }
 

drivers/virt/coco/tsm.c static struct tsm_report_state *to_state(struct tsm_report *report) · line 71

Quoted 5 lines

drivers/virt/coco/tsm.c +5 -0

+static umode_t svsm_visibility(const struct config_item *item, const struct configfs_attribute *attr)
+{
+	return cc_platform_has(CC_ATTR_GUEST_SVSM_PRESENT) ? attr->ca_mode : 0;
+}
+

Instead of directly checking for CC flags here, I am wondering if it would make sense to add a callback to vendor drivers and let the callback decide whether the attribute is valid or not? We can't add a CC flag for every ConfigFS attribute, right? For example, privlevel is not used by TDX. If there is a callback, then TDX driver can make this attribute invalid for it.

Quoted 59 lines

drivers/virt/coco/tsm.c +4 -4

  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, svsm_visibility);
 
  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, svsm_visibility);
 
  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, svsm_visibility);
 
  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, svsm_visibility);
 
  #define TSM_DEFAULT_ATTRS() \
  	&tsm_report_attr_generation, \

include/linux/cc_platform.h +8 -0

index cb0d6cd1c12f..f1b4266c1484 100644
@@ -90,6 +90,14 @@ enum cc_attr {
  	 * Examples include TDX Guest.
  	 */
  	CC_ATTR_HOTPLUG_DISABLED,
+
+	/**
+	 * @CC_ATTR_GUEST_SVSM_PRESENT: Guest is running under an SVSM
+	 *
+	 * The platform/OS is running as a guest/virtual machine and is
+	 * running under a Secure VM Service Module (SVSM).
+	 */
+	CC_ATTR_GUEST_SVSM_PRESENT,
  };
 
  #ifdef CONFIG_ARCH_HAS_CC_PLATFORM

Quoted 44 lines · Sathyanarayanan

Quoted 42 lines · Tom Lendacky

Config-fs provides support to hide individual attribute entries. Using this support, base the display of the SVSM related entries on the presence of an SVSM.

Cc Joel Becker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Signed-off-by Tom Lendacky <[email protected]>

---
  arch/x86/coco/core.c        |  4 ++++
  drivers/virt/coco/tsm.c     | 13 +++++++++----
  include/linux/cc_platform.h |  8 ++++++++

  3 files changed, 21 insertions(+), 4 deletions(-)

arch/x86/coco/core.c +1 -0

index d07be9d05cd0..efa0f648f754 100644
@@ -12,6 +12,7 @@
     #include <asm/coco.h>
   #include <asm/processor.h>
+#include <asm/sev.h>
     enum cc_vendor cc_vendor __ro_after_init = CC_VENDOR_NONE;
   u64 cc_mask __ro_after_init;

@@ -78,6 +79,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr)

      case CC_ATTR_GUEST_STATE_ENCRYPT:
          return sev_status & MSR_AMD64_SEV_ES_ENABLED;
  +    case CC_ATTR_GUEST_SVSM_PRESENT:
+        return snp_get_vmpl();
+
      /*
       * With SEV, the rep string I/O instructions need to be unrolled
       * but SEV-ES supports them through the #VC handler.

drivers/virt/coco/tsm.c +2 -0

index 07b4c95ce704..2efa6e578477 100644
@@ -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 umode_t svsm_visibility(const struct config_item *item, const struct configfs_attribute *attr)
+{
+    return cc_platform_has(CC_ATTR_GUEST_SVSM_PRESENT) ? attr->ca_mode : 0;

Quoted 8 lines

drivers/virt/coco/tsm.c static struct tsm_report_state *to_state(struct tsm_report *report) · line 71

Quoted 2 lines

drivers/virt/coco/tsm.c +2 -0

+}
+

Instead of directly checking for CC flags here, I am wondering if it would make sense to add a callback to vendor drivers and let the callback decide whether the attribute is valid or not? We can't add a CC flag for every ConfigFS attribute, right? For example, privlevel is not used by TDX. If there is a callback, then TDX driver can make this attribute invalid for it.

I think that's something that can be looked at after this series.

Thanks,
Tom

Quoted 51 lines

Quoted 50 lines

drivers/virt/coco/tsm.c +4 -4

   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, svsm_visibility);
     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, svsm_visibility);
     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, svsm_visibility);
     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, svsm_visibility);
     #define TSM_DEFAULT_ATTRS() \
       &tsm_report_attr_generation, \

include/linux/cc_platform.h +8 -0

index cb0d6cd1c12f..f1b4266c1484 100644
@@ -90,6 +90,14 @@ enum cc_attr {
        * Examples include TDX Guest.
        */
       CC_ATTR_HOTPLUG_DISABLED,
+
+    /**
+     * @CC_ATTR_GUEST_SVSM_PRESENT: Guest is running under an SVSM
+     *
+     * The platform/OS is running as a guest/virtual machine and is
+     * running under a Secure VM Service Module (SVSM).
+     */
+    CC_ATTR_GUEST_SVSM_PRESENT,
   };
     #ifdef CONFIG_ARCH_HAS_CC_PLATFORM