* [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM
@ 2024-04-24 15:57 Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page Tom Lendacky
` (15 more replies)
0 siblings, 16 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:57 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra, Joel Becker, Christoph Hellwig
This series adds SEV-SNP support for running Linux under an Secure VM
Service Module (SVSM) at a less privileged VM Privilege Level (VMPL).
By running at a less priviledged VMPL, the SVSM can be used to provide
services, e.g. a virtual TPM, for Linux within the SEV-SNP confidential
VM (CVM) rather than trust such services from the hypervisor.
Currently, a Linux guest expects to run at the highest VMPL, VMPL0, and
there are certain SNP related operations that require that VMPL level.
Specifically, the PVALIDATE instruction and the RMPADJUST instruction
when setting the VMSA attribute of a page (used when starting APs).
If Linux is to run at a less privileged VMPL, e.g. VMPL2, then it must
use an SVSM (which is running at VMPL0) to perform the operations that
it is no longer able to perform.
How Linux interacts with and uses the SVSM is documented in the SVSM
specification [1] and the GHCB specification [2].
This series introduces support to run Linux under an SVSM. It consists
of:
- Detecting the presence of an SVSM
- When not running at VMPL0, invoking the SVSM for page validation and
VMSA page creation/deletion
- Adding a sysfs entry that specifies the Linux VMPL
- Modifying the sev-guest driver to use the VMPCK key associated with
the Linux VMPL
- Expanding the config-fs TSM support to request attestation reports
from the SVSM and allowing attributes to be hidden
- Detecting and allowing Linux to run in a VMPL other than 0 when an
SVSM is present
The series is based off of and tested against the tip tree:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
4e2b6e891aae ("Merge branch into tip/master: 'x86/shstk'")
[1] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Christoph Hellwig <hch@lst.de>
---
Changes in v4:
- Add a pre-patch to rename the struct snp_secrets_page_layout to just
snp_secrets_page.
- Move the config-fs visibility support to be group based and referenced
by an index. Remove the macro changes that set the visibility function
for an entry.
- Make the TSM visibility support vendor specific via an ops callback.
- Use the rmpadjust() function directly and remove the enforce_vmpl0()
function.
- Consolidate common variables into arch/x86/kernel/sev-shared.c.
Changes in v3:
- Rename decompresor snp_setup() to early_snp_setup() to better indicate
when it is called.
- Rename the "svsm" config-fs attribute into the more generic
"service_provider" attribute that takes a name as input.
- Change config-fs visibility function to be a simple bool return type
instead of returning the mode.
- Switch to using new RIP_REL_REF() macro and __head notation where
appropriate.
Changes in v2:
- Define X86_FEATURE_SVSM_PRESENT and set the bit in the CPUID table,
removing the need to set the CPUID bit in the #VC handler.
- Rename the TSM service_version attribute to service_manifest_version.
- Add support to config-fs to hide attributes and hide the SVSM attributes
when an SVSM is not present.
Tom Lendacky (15):
x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page
x86/sev: Rename snp_init() in the boot/compressed/sev.c file
x86/sev: Make the VMPL0 checking more straight forward
x86/sev: Check for the presence of an SVSM in the SNP Secrets page
x86/sev: Use kernel provided SVSM Calling Areas
x86/sev: Perform PVALIDATE using the SVSM when not at VMPL0
x86/sev: Use the SVSM to create a vCPU when not in VMPL0
x86/sev: Provide SVSM discovery support
x86/sev: Provide guest VMPL level to userspace
virt: sev-guest: Choose the VMPCK key based on executing VMPL
configfs-tsm: Allow the privlevel_floor attribute to be updated
fs/configfs: Add a callback to determine attribute visibility
x86/sev: Take advantage of configfs visibility support in TSM
x86/sev: Extend the config-fs attestation support for an SVSM
x86/sev: Allow non-VMPL0 execution when an SVSM is present
Documentation/ABI/testing/configfs-tsm | 63 +++
.../arch/x86/amd-memory-encryption.rst | 22 +
arch/x86/boot/compressed/sev.c | 258 ++++++-----
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 2 +
arch/x86/include/asm/sev-common.h | 18 +
arch/x86/include/asm/sev.h | 116 ++++-
arch/x86/include/uapi/asm/svm.h | 1 +
arch/x86/kernel/sev-shared.c | 354 ++++++++++++++-
arch/x86/kernel/sev.c | 421 +++++++++++++++---
arch/x86/mm/mem_encrypt_amd.c | 8 +-
drivers/virt/coco/sev-guest/sev-guest.c | 210 ++++++++-
drivers/virt/coco/tdx-guest/tdx-guest.c | 29 +-
drivers/virt/coco/tsm.c | 173 +++++--
fs/configfs/dir.c | 20 +
include/linux/configfs.h | 3 +
include/linux/tsm.h | 62 ++-
17 files changed, 1533 insertions(+), 228 deletions(-)
--
2.43.2
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
@ 2024-04-24 15:57 ` Tom Lendacky
2024-04-25 13:30 ` Borislav Petkov
2024-04-24 15:57 ` [PATCH v4 02/15] x86/sev: Rename snp_init() in the boot/compressed/sev.c file Tom Lendacky
` (14 subsequent siblings)
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:57 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
Ending a struct name with "layout" is a little redundant, so shorten the
snp_secrets_page_layout name to just snp_secrets_page.
No functional change.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/include/asm/sev.h | 2 +-
arch/x86/kernel/sev.c | 4 ++--
drivers/virt/coco/sev-guest/sev-guest.c | 6 +++---
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 7f57382afee4..48bc397db649 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -140,7 +140,7 @@ struct secrets_os_area {
#define VMPCK_KEY_LEN 32
/* See the SNP spec version 0.9 for secrets page format */
-struct snp_secrets_page_layout {
+struct snp_secrets_page {
u32 version;
u32 imien : 1,
rsvd1 : 31;
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 995f94467101..6949fbccec40 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -648,7 +648,7 @@ static u64 __init get_secrets_page(void)
static u64 __init get_snp_jump_table_addr(void)
{
- struct snp_secrets_page_layout *layout;
+ struct snp_secrets_page *layout;
void __iomem *mem;
u64 pa, addr;
@@ -662,7 +662,7 @@ static u64 __init get_snp_jump_table_addr(void)
return 0;
}
- layout = (__force struct snp_secrets_page_layout *)mem;
+ layout = (__force struct snp_secrets_page *)mem;
addr = layout->os_area.ap_jump_table_pa;
iounmap(mem);
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 87f241825bc3..04a7bd1e4314 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -59,7 +59,7 @@ struct snp_guest_dev {
*/
struct snp_guest_msg secret_request, secret_response;
- struct snp_secrets_page_layout *layout;
+ struct snp_secrets_page *layout;
struct snp_req_data input;
union {
struct snp_report_req report;
@@ -743,7 +743,7 @@ static const struct file_operations snp_guest_fops = {
.unlocked_ioctl = snp_guest_ioctl,
};
-static u8 *get_vmpck(int id, struct snp_secrets_page_layout *layout, u32 **seqno)
+static u8 *get_vmpck(int id, struct snp_secrets_page *layout, u32 **seqno)
{
u8 *key = NULL;
@@ -897,8 +897,8 @@ static void unregister_sev_tsm(void *data)
static int __init sev_guest_probe(struct platform_device *pdev)
{
- struct snp_secrets_page_layout *layout;
struct sev_guest_platform_data *data;
+ struct snp_secrets_page *layout;
struct device *dev = &pdev->dev;
struct snp_guest_dev *snp_dev;
struct miscdevice *misc;
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 02/15] x86/sev: Rename snp_init() in the boot/compressed/sev.c file
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page Tom Lendacky
@ 2024-04-24 15:57 ` Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 03/15] x86/sev: Make the VMPL0 checking more straight forward Tom Lendacky
` (13 subsequent siblings)
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:57 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
The snp_init() function in boot/compressed/sev.c is local to that file, is
not called from outside of the file and is independent of the snp_init()
function in kernel/sev.c. Change the name to better differentiate when
each function is used.
Move the renamed snp_init() and related functions up in the file to avoid
having to add a forward declaration and make the function static.
No functional change.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/boot/compressed/sev.c | 162 ++++++++++++++++-----------------
1 file changed, 81 insertions(+), 81 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index ec71846d28c9..5ad0ff4664f1 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -413,6 +413,85 @@ void snp_check_features(void)
}
}
+/* Search for Confidential Computing blob in the EFI config table. */
+static struct cc_blob_sev_info *find_cc_blob_efi(struct boot_params *bp)
+{
+ unsigned long cfg_table_pa;
+ unsigned int cfg_table_len;
+ int ret;
+
+ ret = efi_get_conf_table(bp, &cfg_table_pa, &cfg_table_len);
+ if (ret)
+ return NULL;
+
+ return (struct cc_blob_sev_info *)efi_find_vendor_table(bp, cfg_table_pa,
+ cfg_table_len,
+ EFI_CC_BLOB_GUID);
+}
+
+/*
+ * Initial set up of SNP relies on information provided by the
+ * Confidential Computing blob, which can be passed to the boot kernel
+ * by firmware/bootloader in the following ways:
+ *
+ * - via an entry in the EFI config table
+ * - via a setup_data structure, as defined by the Linux Boot Protocol
+ *
+ * Scan for the blob in that order.
+ */
+static struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
+{
+ struct cc_blob_sev_info *cc_info;
+
+ cc_info = find_cc_blob_efi(bp);
+ if (cc_info)
+ goto found_cc_info;
+
+ cc_info = find_cc_blob_setup_data(bp);
+ if (!cc_info)
+ return NULL;
+
+found_cc_info:
+ if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
+ sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
+
+ return cc_info;
+}
+
+/*
+ * Indicate SNP based on presence of SNP-specific CC blob. Subsequent checks
+ * will verify the SNP CPUID/MSR bits.
+ */
+static bool early_snp_init(struct boot_params *bp)
+{
+ struct cc_blob_sev_info *cc_info;
+
+ if (!bp)
+ return false;
+
+ cc_info = find_cc_blob(bp);
+ if (!cc_info)
+ return false;
+
+ /*
+ * If a SNP-specific Confidential Computing blob is present, then
+ * firmware/bootloader have indicated SNP support. Verifying this
+ * involves CPUID checks which will be more reliable if the SNP
+ * CPUID table is used. See comments over snp_setup_cpuid_table() for
+ * more details.
+ */
+ setup_cpuid_table(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
+ * phase.
+ */
+ bp->cc_blob_address = (u32)(unsigned long)cc_info;
+
+ return true;
+}
+
/*
* sev_check_cpu_support - Check for SEV support in the CPU capabilities
*
@@ -463,7 +542,7 @@ void sev_enable(struct boot_params *bp)
bp->cc_blob_address = 0;
/*
- * Do an initial SEV capability check before snp_init() which
+ * Do an initial SEV capability check before early_snp_init() which
* loads the CPUID page and the same checks afterwards are done
* without the hypervisor and are trustworthy.
*
@@ -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 = early_snp_init(bp);
/* Now repeat the checks with the SNP CPUID table. */
@@ -535,85 +614,6 @@ u64 sev_get_status(void)
return m.q;
}
-/* Search for Confidential Computing blob in the EFI config table. */
-static struct cc_blob_sev_info *find_cc_blob_efi(struct boot_params *bp)
-{
- unsigned long cfg_table_pa;
- unsigned int cfg_table_len;
- int ret;
-
- ret = efi_get_conf_table(bp, &cfg_table_pa, &cfg_table_len);
- if (ret)
- return NULL;
-
- return (struct cc_blob_sev_info *)efi_find_vendor_table(bp, cfg_table_pa,
- cfg_table_len,
- EFI_CC_BLOB_GUID);
-}
-
-/*
- * Initial set up of SNP relies on information provided by the
- * Confidential Computing blob, which can be passed to the boot kernel
- * by firmware/bootloader in the following ways:
- *
- * - via an entry in the EFI config table
- * - via a setup_data structure, as defined by the Linux Boot Protocol
- *
- * Scan for the blob in that order.
- */
-static struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
-{
- struct cc_blob_sev_info *cc_info;
-
- cc_info = find_cc_blob_efi(bp);
- if (cc_info)
- goto found_cc_info;
-
- cc_info = find_cc_blob_setup_data(bp);
- if (!cc_info)
- return NULL;
-
-found_cc_info:
- if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
- sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
-
- return cc_info;
-}
-
-/*
- * Indicate SNP based on presence of SNP-specific CC blob. Subsequent checks
- * will verify the SNP CPUID/MSR bits.
- */
-bool snp_init(struct boot_params *bp)
-{
- struct cc_blob_sev_info *cc_info;
-
- if (!bp)
- return false;
-
- cc_info = find_cc_blob(bp);
- if (!cc_info)
- return false;
-
- /*
- * If a SNP-specific Confidential Computing blob is present, then
- * firmware/bootloader have indicated SNP support. Verifying this
- * involves CPUID checks which will be more reliable if the SNP
- * CPUID table is used. See comments over snp_setup_cpuid_table() for
- * more details.
- */
- setup_cpuid_table(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
- * phase.
- */
- bp->cc_blob_address = (u32)(unsigned long)cc_info;
-
- return true;
-}
-
void sev_prep_identity_maps(unsigned long top_level_pgt)
{
/*
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 03/15] x86/sev: Make the VMPL0 checking more straight forward
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 02/15] x86/sev: Rename snp_init() in the boot/compressed/sev.c file Tom Lendacky
@ 2024-04-24 15:57 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page Tom Lendacky
` (12 subsequent siblings)
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:57 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
Currently, the enforce_vmpl0() function uses a set argument when modifying
the VMPL1 permissions used to test for VMPL0. If the guest is not running
at VMPL0, the guest self-terminates.
The function is just a wrapper for a fixed RMPADJUST function. Eliminate
the function and perform the RMPADJUST directly.
No functional change.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/boot/compressed/sev.c | 35 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 5ad0ff4664f1..0457a9d7e515 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -335,26 +335,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 void enforce_vmpl0(void)
-{
- u64 attrs;
- int err;
-
- /*
- * 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 are supported only on VMPL0 so VMPL1 or higher permission masks
- * changing is a don't-care.
- */
- attrs = 1;
- if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, attrs))
- sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
-}
-
/*
* SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
* guest side implementation for proper functioning of the guest. If any
@@ -588,7 +568,20 @@ void sev_enable(struct boot_params *bp)
if (!(get_hv_features() & GHCB_HV_FT_SNP))
sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
- enforce_vmpl0();
+ /*
+ * Enforce running at VMPL0.
+ *
+ * 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.
+ */
+ if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1))
+ sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
}
if (snp && !(sev_status & MSR_AMD64_SEV_SNP_ENABLED))
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (2 preceding siblings ...)
2024-04-24 15:57 ` [PATCH v4 03/15] x86/sev: Make the VMPL0 checking more straight forward Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-05-02 9:35 ` Borislav Petkov
2024-04-24 15:58 ` [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas Tom Lendacky
` (11 subsequent siblings)
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
During early boot phases, check for the presence of an SVSM when running
as an SEV-SNP guest.
An SVSM is present if not running at VMPL0 and 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 <thomas.lendacky@amd.com>
---
.../arch/x86/amd-memory-encryption.rst | 22 ++++++
arch/x86/boot/compressed/sev.c | 8 +++
arch/x86/include/asm/sev-common.h | 4 ++
arch/x86/include/asm/sev.h | 25 ++++++-
arch/x86/kernel/sev-shared.c | 70 +++++++++++++++++++
arch/x86/kernel/sev.c | 7 ++
6 files changed, 135 insertions(+), 1 deletion(-)
diff --git a/Documentation/arch/x86/amd-memory-encryption.rst b/Documentation/arch/x86/amd-memory-encryption.rst
index 414bc7402ae7..32737718d4a2 100644
--- a/Documentation/arch/x86/amd-memory-encryption.rst
+++ b/Documentation/arch/x86/amd-memory-encryption.rst
@@ -130,4 +130,26 @@ SNP feature support.
More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR
+Secure VM Service Module (SVSM)
+===============================
+
+SNP provides a feature called Virtual Machine Privilege Levels (VMPL). The most
+privileged VMPL is 0 with numerically higher numbers having lesser privileges.
+More details in AMD64 APM[1] Vol 2: 15.35.7 Virtual Machine Privilege Levels.
+
+The VMPL feature provides the ability to run software services at a more
+privileged level than the guest OS is running at. This provides a secure
+environment for services within the guest's SNP environment, while protecting
+the service from hypervisor interference. An example of a secure service
+would be a virtual TPM (vTPM). Additionally, certain operations require the
+guest to be running at VMPL0 in order for them to be performed. For example,
+the PVALIDATE instruction is required to be executed at VMPL0.
+
+When a guest is not running at VMPL0, it needs to communicate with the software
+running at VMPL0 to perform privileged operations or to interact with secure
+services. This software running at VMPL0 is known as a Secure VM Service Module
+(SVSM). Discovery of an SVSM and the API used to communicate with it is
+documented in Secure VM Service Module for SEV-SNP Guests[2].
+
[1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
+[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 0457a9d7e515..cb771b380a6b 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -12,6 +12,7 @@
*/
#include "misc.h"
+#include <linux/mm.h>
#include <asm/bootparam.h>
#include <asm/pgtable_types.h>
#include <asm/sev.h>
@@ -462,6 +463,13 @@ static bool early_snp_init(struct boot_params *bp)
*/
setup_cpuid_table(cc_info);
+ /*
+ * Record the SVSM Calling Area (CA) address 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
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index b463fcbd4b90..1225744a069b 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -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 CAA is not page aligned */
#define GHCB_RESP_CODE(v) ((v) & GHCB_MSR_INFO_MASK)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 48bc397db649..56f7d843f7a4 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -152,9 +152,32 @@ struct snp_secrets_page {
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);
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index b4f8fa0f722c..46ea4e5e118a 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -23,6 +23,21 @@
#define sev_printk_rtl(fmt, ...)
#endif
+/*
+ * 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.
+ *
+ * During boot, the page tables are set up as identity mapped and later
+ * changed to use kernel virtual addresses. Maintain separate virtual and
+ * physical addresses for the CAA to allow SVSM functions to be used during
+ * early boot, both with identity mapped virtual addresses and proper kernel
+ * virtual addresses.
+ */
+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;
+
/* I/O parameters for CPUID-related helpers */
struct cpuid_leaf {
u32 fn;
@@ -1269,3 +1284,58 @@ static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
return ES_UNSUPPORTED;
}
+
+/*
+ * Maintain the GPA of the SVSM Calling Area (CA) in order to utilize the SVSM
+ * services needed when not running in VMPL0.
+ */
+static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
+{
+ struct snp_secrets_page *secrets_page;
+ u64 caa;
+
+ BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
+
+ /*
+ * 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.
+ *
+ * Use __pa() since this routine is running identity mapped when called,
+ * both by the decompressor code and the early kernel code.
+ */
+ if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
+ 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 *)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;
+}
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 6949fbccec40..a500df807e79 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2108,6 +2108,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
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (3 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-05-03 10:34 ` Borislav Petkov
2024-04-24 15:58 ` [PATCH v4 06/15] x86/sev: Perform PVALIDATE using the SVSM when not at VMPL0 Tom Lendacky
` (10 subsequent siblings)
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
The SVSM Calling Area (CA) is used to communicate between Linux and the
SVSM. Since the firmware supplied CA for the BSP is likely to be in
reserved memory, switch off that CA to a kernel provided CA so that access
and use of the CA is available during boot. The CA switch is done using
the SVSM core protocol SVSM_CORE_REMAP_CA call.
An SVSM call is executed by filling out the SVSM CA and setting the proper
register state as documented by the SVSM protocol. The SVSM is invoked by
by requesting the hypervisor to run VMPL0.
Once it is safe to allocate/reserve memory, allocate a CA for each CPU.
After allocating the new CAs, the BSP will switch from the boot CA to the
per-CPU CA. The CA for an AP is identified to the SVSM when creating the
VMSA in preparation for booting the AP.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/include/asm/sev-common.h | 13 ++
arch/x86/include/asm/sev.h | 32 +++++
arch/x86/include/uapi/asm/svm.h | 1 +
arch/x86/kernel/sev-shared.c | 94 +++++++++++++-
arch/x86/kernel/sev.c | 207 +++++++++++++++++++++++++-----
arch/x86/mm/mem_encrypt_amd.c | 8 +-
6 files changed, 320 insertions(+), 35 deletions(-)
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 1225744a069b..4cc716660d4b 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -96,6 +96,19 @@ enum psc_op {
/* GHCBData[63:32] */ \
(((u64)(val) & GENMASK_ULL(63, 32)) >> 32)
+/* GHCB Run at VMPL Request/Response */
+#define GHCB_MSR_VMPL_REQ 0x016
+#define GHCB_MSR_VMPL_REQ_LEVEL(v) \
+ /* GHCBData[39:32] */ \
+ (((u64)(v) & GENMASK_ULL(7, 0) << 32) | \
+ /* GHCBDdata[11:0] */ \
+ GHCB_MSR_VMPL_REQ)
+
+#define GHCB_MSR_VMPL_RESP 0x017
+#define GHCB_MSR_VMPL_RESP_VAL(v) \
+ /* GHCBData[63:32] */ \
+ (((u64)(v) & GENMASK_ULL(63, 32)) >> 32)
+
/* GHCB Hypervisor Feature Request/Response */
#define GHCB_MSR_HV_FT_REQ 0x080
#define GHCB_MSR_HV_FT_RESP 0x081
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 56f7d843f7a4..8f180fd3cbf0 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -178,6 +178,36 @@ struct svsm_ca {
u8 svsm_buffer[PAGE_SIZE - 8];
};
+#define SVSM_SUCCESS 0
+#define SVSM_ERR_INCOMPLETE 0x80000000
+#define SVSM_ERR_UNSUPPORTED_PROTOCOL 0x80000001
+#define SVSM_ERR_UNSUPPORTED_CALL 0x80000002
+#define SVSM_ERR_INVALID_ADDRESS 0x80000003
+#define SVSM_ERR_INVALID_FORMAT 0x80000004
+#define SVSM_ERR_INVALID_PARAMETER 0x80000005
+#define SVSM_ERR_INVALID_REQUEST 0x80000006
+#define SVSM_ERR_BUSY 0x80000007
+
+/*
+ * SVSM protocol structure
+ */
+struct svsm_call {
+ struct svsm_ca *caa;
+ u64 rax;
+ u64 rcx;
+ u64 rdx;
+ u64 r8;
+ u64 r9;
+ u64 rax_out;
+ u64 rcx_out;
+ u64 rdx_out;
+ u64 r8_out;
+ u64 r9_out;
+};
+
+#define SVSM_CORE_CALL(x) ((0ULL << 32) | (x))
+#define SVSM_CORE_REMAP_CA 0
+
#ifdef CONFIG_AMD_MEM_ENCRYPT
extern void __sev_es_ist_enter(struct pt_regs *regs);
extern void __sev_es_ist_exit(void);
@@ -252,6 +282,7 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end);
u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
+void snp_remap_svsm_ca(void);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
@@ -281,6 +312,7 @@ 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; }
static inline void sev_show_status(void) { }
+static inline void snp_remap_svsm_ca(void) { }
#endif
#ifdef CONFIG_KVM_AMD_SEV
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index 80e1df482337..1814b413fd57 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -115,6 +115,7 @@
#define SVM_VMGEXIT_AP_CREATE_ON_INIT 0
#define SVM_VMGEXIT_AP_CREATE 1
#define SVM_VMGEXIT_AP_DESTROY 2
+#define SVM_VMGEXIT_SNP_RUN_VMPL 0x80000018
#define SVM_VMGEXIT_HV_FEATURES 0x8000fffd
#define SVM_VMGEXIT_TERM_REQUEST 0x8000fffe
#define SVM_VMGEXIT_TERM_REASON(reason_set, reason_code) \
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 46ea4e5e118a..6f57eb804e70 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -18,9 +18,11 @@
#define sev_printk_rtl(fmt, ...) printk_ratelimited(fmt, ##__VA_ARGS__)
#else
#undef WARN
-#define WARN(condition, format...) (!!(condition))
+#define WARN(condition, format...) (!!(condition))
#define sev_printk(fmt, ...)
#define sev_printk_rtl(fmt, ...)
+#undef vc_forward_exception
+#define vc_forward_exception(c) panic("SNP: Hypervisor requested exception\n")
#endif
/*
@@ -244,6 +246,96 @@ static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt
return ES_VMM_ERROR;
}
+static __always_inline void issue_svsm_call(struct svsm_call *call, u8 *pending)
+{
+ /*
+ * Issue the VMGEXIT to run the SVSM:
+ * - Load the SVSM register state (RAX, RCX, RDX, R8 and R9)
+ * - Set the CA call pending field to 1
+ * - Issue VMGEXIT
+ * - Save the SVSM return register state (RAX, RCX, RDX, R8 and R9)
+ * - Perform atomic exchange of the CA call pending field
+ */
+ asm volatile("mov %9, %%r8\n\t"
+ "mov %10, %%r9\n\t"
+ "movb $1, %11\n\t"
+ "rep; vmmcall\n\t"
+ "mov %%r8, %3\n\t"
+ "mov %%r9, %4\n\t"
+ "xchgb %5, %11\n\t"
+ : "=a" (call->rax_out), "=c" (call->rcx_out), "=d" (call->rdx_out),
+ "=m" (call->r8_out), "=m" (call->r9_out),
+ "+r" (*pending)
+ : "a" (call->rax), "c" (call->rcx), "d" (call->rdx),
+ "r" (call->r8), "r" (call->r9),
+ "m" (call->caa->call_pending)
+ : "r8", "r9", "memory");
+}
+
+static int __svsm_msr_protocol(struct svsm_call *call)
+{
+ u64 val, resp;
+ u8 pending;
+
+ val = sev_es_rd_ghcb_msr();
+
+ sev_es_wr_ghcb_msr(GHCB_MSR_VMPL_REQ_LEVEL(0));
+
+ pending = 0;
+ issue_svsm_call(call, &pending);
+
+ resp = sev_es_rd_ghcb_msr();
+
+ sev_es_wr_ghcb_msr(val);
+
+ if (pending)
+ return -EINVAL;
+
+ if (GHCB_RESP_CODE(resp) != GHCB_MSR_VMPL_RESP)
+ return -EINVAL;
+
+ if (GHCB_MSR_VMPL_RESP_VAL(resp) != 0)
+ return -EINVAL;
+
+ return call->rax_out;
+}
+
+static int __svsm_ghcb_protocol(struct ghcb *ghcb, struct svsm_call *call)
+{
+ struct es_em_ctxt ctxt;
+ u8 pending;
+
+ vc_ghcb_invalidate(ghcb);
+
+ /* Fill in protocol and format specifiers */
+ ghcb->protocol_version = ghcb_version;
+ ghcb->ghcb_usage = GHCB_DEFAULT_USAGE;
+
+ ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_SNP_RUN_VMPL);
+ ghcb_set_sw_exit_info_1(ghcb, 0);
+ ghcb_set_sw_exit_info_2(ghcb, 0);
+
+ sev_es_wr_ghcb_msr(__pa(ghcb));
+
+ pending = 0;
+ issue_svsm_call(call, &pending);
+
+ if (pending)
+ return -EINVAL;
+
+ switch (verify_exception_info(ghcb, &ctxt)) {
+ case ES_OK:
+ break;
+ case ES_EXCEPTION:
+ vc_forward_exception(&ctxt);
+ fallthrough;
+ default:
+ return -EINVAL;
+ }
+
+ return call->rax_out;
+}
+
static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
struct es_em_ctxt *ctxt,
u64 exit_code, u64 exit_info_1,
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index a500df807e79..21f3cc40d662 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -133,8 +133,13 @@ struct ghcb_state {
struct ghcb *ghcb;
};
+/* For early boot SVSM communication */
+static struct svsm_ca boot_svsm_ca_page __aligned(PAGE_SIZE);
+
static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
+static DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
+static DEFINE_PER_CPU(u64, svsm_caa_pa);
struct sev_config {
__u64 debug : 1,
@@ -150,6 +155,15 @@ struct sev_config {
*/
ghcbs_initialized : 1,
+ /*
+ * A flag used to indicate when the per-CPU SVSM CA is to be
+ * used instead of the boot SVSM CA.
+ *
+ * For APs, the per-CPU SVSM CA is created as part of the AP
+ * bringup, so this flag can be used globally for the BSP and APs.
+ */
+ cas_initialized : 1,
+
__reserved : 62;
};
@@ -572,9 +586,42 @@ static enum es_result vc_ioio_check(struct es_em_ctxt *ctxt, u16 port, size_t si
return ES_EXCEPTION;
}
+static __always_inline void vc_forward_exception(struct es_em_ctxt *ctxt)
+{
+ long error_code = ctxt->fi.error_code;
+ int trapnr = ctxt->fi.vector;
+
+ ctxt->regs->orig_ax = ctxt->fi.error_code;
+
+ switch (trapnr) {
+ case X86_TRAP_GP:
+ exc_general_protection(ctxt->regs, error_code);
+ break;
+ case X86_TRAP_UD:
+ exc_invalid_op(ctxt->regs);
+ break;
+ case X86_TRAP_PF:
+ write_cr2(ctxt->fi.cr2);
+ exc_page_fault(ctxt->regs, error_code);
+ break;
+ case X86_TRAP_AC:
+ exc_alignment_check(ctxt->regs, error_code);
+ break;
+ default:
+ pr_emerg("Unsupported exception in #VC instruction emulation - can't continue\n");
+ BUG();
+ }
+}
+
/* Include code shared with pre-decompression boot stage */
#include "sev-shared.c"
+static struct svsm_ca *__svsm_get_caa(void)
+{
+ return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa)
+ : boot_svsm_caa;
+}
+
static noinstr void __sev_put_ghcb(struct ghcb_state *state)
{
struct sev_es_runtime_data *data;
@@ -600,6 +647,42 @@ static noinstr void __sev_put_ghcb(struct ghcb_state *state)
}
}
+static int svsm_protocol(struct svsm_call *call)
+{
+ struct ghcb_state state;
+ unsigned long flags;
+ struct ghcb *ghcb;
+ int ret;
+
+ /*
+ * This can be called very early in the boot, use native functions in
+ * order to avoid paravirt issues.
+ */
+ flags = native_save_fl();
+ if (flags & X86_EFLAGS_IF)
+ native_irq_disable();
+
+ if (sev_cfg.ghcbs_initialized)
+ ghcb = __sev_get_ghcb(&state);
+ else if (boot_ghcb)
+ ghcb = boot_ghcb;
+ else
+ ghcb = NULL;
+
+ do {
+ ret = ghcb ? __svsm_ghcb_protocol(ghcb, call)
+ : __svsm_msr_protocol(call);
+ } while (ret == SVSM_ERR_BUSY);
+
+ if (sev_cfg.ghcbs_initialized)
+ __sev_put_ghcb(&state);
+
+ if (flags & X86_EFLAGS_IF)
+ native_irq_enable();
+
+ return ret;
+}
+
void noinstr __sev_es_nmi_complete(void)
{
struct ghcb_state state;
@@ -1346,6 +1429,18 @@ static void __init alloc_runtime_data(int cpu)
panic("Can't allocate SEV-ES runtime data");
per_cpu(runtime_data, cpu) = data;
+
+ if (vmpl) {
+ struct svsm_ca *caa;
+
+ /* Allocate the SVSM CA page if an SVSM is present */
+ caa = memblock_alloc(sizeof(*caa), PAGE_SIZE);
+ if (!caa)
+ panic("Can't allocate SVSM CA page\n");
+
+ per_cpu(svsm_caa, cpu) = caa;
+ per_cpu(svsm_caa_pa, cpu) = __pa(caa);
+ }
}
static void __init init_ghcb(int cpu)
@@ -1395,6 +1490,31 @@ void __init sev_es_init_vc_handling(void)
init_ghcb(cpu);
}
+ /* If running under an SVSM, switch to the per-cpu CA */
+ if (vmpl) {
+ struct svsm_call call = {};
+ unsigned long flags;
+ int ret;
+
+ local_irq_save(flags);
+
+ /*
+ * SVSM_CORE_REMAP_CA call:
+ * RAX = 0 (Protocol=0, CallID=0)
+ * RCX = New CA GPA
+ */
+ call.caa = __svsm_get_caa();
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_REMAP_CA);
+ call.rcx = this_cpu_read(svsm_caa_pa);
+ ret = svsm_protocol(&call);
+ if (ret != SVSM_SUCCESS)
+ panic("Can't remap the SVSM CA, ret=%#x (%d)\n", ret, ret);
+
+ sev_cfg.cas_initialized = true;
+
+ local_irq_restore(flags);
+ }
+
sev_es_setup_play_dead();
/* Secondary CPUs use the runtime #VC handler */
@@ -1819,33 +1939,6 @@ static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt,
return result;
}
-static __always_inline void vc_forward_exception(struct es_em_ctxt *ctxt)
-{
- long error_code = ctxt->fi.error_code;
- int trapnr = ctxt->fi.vector;
-
- ctxt->regs->orig_ax = ctxt->fi.error_code;
-
- switch (trapnr) {
- case X86_TRAP_GP:
- exc_general_protection(ctxt->regs, error_code);
- break;
- case X86_TRAP_UD:
- exc_invalid_op(ctxt->regs);
- break;
- case X86_TRAP_PF:
- write_cr2(ctxt->fi.cr2);
- exc_page_fault(ctxt->regs, error_code);
- break;
- case X86_TRAP_AC:
- exc_alignment_check(ctxt->regs, error_code);
- break;
- default:
- pr_emerg("Unsupported exception in #VC instruction emulation - can't continue\n");
- BUG();
- }
-}
-
static __always_inline bool is_vc2_stack(unsigned long sp)
{
return (sp >= __this_cpu_ist_bottom_va(VC2) && sp < __this_cpu_ist_top_va(VC2));
@@ -2095,6 +2188,50 @@ static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
return cc_info;
}
+static __head void setup_svsm(struct cc_blob_sev_info *cc_info)
+{
+ struct svsm_call call = {};
+ int ret;
+ u64 pa;
+
+ /*
+ * 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);
+
+ /* Nothing to do if not running under an SVSM. */
+ if (!vmpl)
+ return;
+
+ /*
+ * It is very early in the boot and the kernel is running identity
+ * mapped but without having adjusted the pagetables to where the
+ * kernel was loaded (physbase), so the get the CA address using
+ * RIP-relative addressing.
+ */
+ pa = (u64)&RIP_REL_REF(boot_svsm_ca_page);
+
+ /*
+ * Switch over to the boot SVSM CA while the current CA is still
+ * addressable. There is no GHCB at this point so use the MSR protocol.
+ *
+ * SVSM_CORE_REMAP_CA call:
+ * RAX = 0 (Protocol=0, CallID=0)
+ * RCX = New CA GPA
+ */
+ call.caa = __svsm_get_caa();
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_REMAP_CA);
+ call.rcx = pa;
+ ret = svsm_protocol(&call);
+ if (ret != SVSM_SUCCESS)
+ panic("Can't remap the SVSM CA, ret=%#x (%d)\n", ret, ret);
+
+ boot_svsm_caa = (struct svsm_ca *)pa;
+ boot_svsm_caa_pa = pa;
+}
+
bool __head snp_init(struct boot_params *bp)
{
struct cc_blob_sev_info *cc_info;
@@ -2108,12 +2245,7 @@ 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);
+ setup_svsm(cc_info);
/*
* The CC blob will be used later to access the secrets page. Cache
@@ -2306,3 +2438,12 @@ void sev_show_status(void)
}
pr_cont("\n");
}
+
+void __init snp_remap_svsm_ca(void)
+{
+ if (!vmpl)
+ return;
+
+ /* Update the CAA to a proper kernel address */
+ boot_svsm_caa = &boot_svsm_ca_page;
+}
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 422602f6039b..6155020e4d2d 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -2,7 +2,7 @@
/*
* AMD Memory Encryption Support
*
- * Copyright (C) 2016 Advanced Micro Devices, Inc.
+ * Copyright (C) 2016-2024 Advanced Micro Devices, Inc.
*
* Author: Tom Lendacky <thomas.lendacky@amd.com>
*/
@@ -510,6 +510,12 @@ void __init sme_early_init(void)
*/
x86_init.resources.dmi_setup = snp_dmi_setup;
}
+
+ /*
+ * Switch the SVSM CA mapping (if active) from identity mapped to
+ * kernel mapped.
+ */
+ snp_remap_svsm_ca();
}
void __init mem_encrypt_free_decrypted_mem(void)
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 06/15] x86/sev: Perform PVALIDATE using the SVSM when not at VMPL0
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (4 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 07/15] x86/sev: Use the SVSM to create a vCPU when not in VMPL0 Tom Lendacky
` (9 subsequent siblings)
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
The PVALIDATE instruction can only be performed at VMPL0. An SVSM will
be present when running at VMPL1 or a lower privilege level.
When an SVSM is present, use the SVSM_CORE_PVALIDATE call to perform
memory validation instead of issuing the PVALIDATE instruction directly.
The validation of a single 4K page is now explicitly identified as such
in the function name, pvalidate_4k_page(). The pvalidate_pages() function
is used for validating 1 or more pages at either 4K or 2M in size. Each
function, however, determines whether it can issue the PVALIDATE directly
or whether the SVSM needs to be invoked.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/boot/compressed/sev.c | 45 ++++++++-
arch/x86/include/asm/sev.h | 22 ++++
arch/x86/kernel/sev-shared.c | 179 ++++++++++++++++++++++++++++++++-
arch/x86/kernel/sev.c | 25 +++--
4 files changed, 253 insertions(+), 18 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index cb771b380a6b..32a1e98ffaa9 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -130,6 +130,34 @@ static bool fault_in_kernel_space(unsigned long address)
/* Include code for early handlers */
#include "../../kernel/sev-shared.c"
+static struct svsm_ca *__svsm_get_caa(void)
+{
+ return boot_svsm_caa;
+}
+
+static u64 __svsm_get_caa_pa(void)
+{
+ return boot_svsm_caa_pa;
+}
+
+static int svsm_protocol(struct svsm_call *call)
+{
+ struct ghcb *ghcb;
+ int ret;
+
+ if (boot_ghcb)
+ ghcb = boot_ghcb;
+ else
+ ghcb = NULL;
+
+ do {
+ ret = ghcb ? __svsm_ghcb_protocol(ghcb, call)
+ : __svsm_msr_protocol(call);
+ } while (ret == SVSM_ERR_BUSY);
+
+ return ret;
+}
+
bool sev_snp_enabled(void)
{
return sev_status & MSR_AMD64_SEV_SNP_ENABLED;
@@ -146,8 +174,8 @@ static void __page_state_change(unsigned long paddr, enum psc_op op)
* If private -> shared then invalidate the page before requesting the
* state change in the RMP table.
*/
- if (op == SNP_PAGE_STATE_SHARED && pvalidate(paddr, RMP_PG_SIZE_4K, 0))
- sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+ if (op == SNP_PAGE_STATE_SHARED)
+ pvalidate_4k_page(paddr, paddr, 0);
/* Issue VMGEXIT to change the page state in RMP table. */
sev_es_wr_ghcb_msr(GHCB_MSR_PSC_REQ_GFN(paddr >> PAGE_SHIFT, op));
@@ -162,8 +190,8 @@ static void __page_state_change(unsigned long paddr, enum psc_op op)
* Now that page state is changed in the RMP table, validate it so that it is
* consistent with the RMP entry.
*/
- if (op == SNP_PAGE_STATE_PRIVATE && pvalidate(paddr, RMP_PG_SIZE_4K, 1))
- sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+ if (op == SNP_PAGE_STATE_PRIVATE)
+ pvalidate_4k_page(paddr, paddr, 1);
}
void snp_set_page_private(unsigned long paddr)
@@ -256,6 +284,15 @@ void sev_es_shutdown_ghcb(void)
if (!sev_es_check_cpu_features())
error("SEV-ES CPU Features missing.");
+ /*
+ * The boot_ghcb value is used to determine whether to use the GHCB MSR
+ * protocol or the GHCB shared page to perform a GHCB request. Since the
+ * GHCB page is being changed to encrypted, it can't be used to perform
+ * GHCB requests. Clear the boot_ghcb variable so that the GHCB MSR
+ * protocol is used to change the GHCB page over to an encrypted page.
+ */
+ boot_ghcb = NULL;
+
/*
* GHCB Page must be flushed from the cache and mapped encrypted again.
* Otherwise the running kernel will see strange cache effects when
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 8f180fd3cbf0..e6f1ed3f6ce3 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -187,6 +187,27 @@ struct svsm_ca {
#define SVSM_ERR_INVALID_PARAMETER 0x80000005
#define SVSM_ERR_INVALID_REQUEST 0x80000006
#define SVSM_ERR_BUSY 0x80000007
+#define SVSM_PVALIDATE_FAIL_SIZEMISMATCH 0x80001006
+
+/*
+ * The SVSM PVALIDATE related structures
+ */
+struct svsm_pvalidate_entry {
+ u64 page_size : 2,
+ action : 1,
+ ignore_cf : 1,
+ rsvd : 8,
+ pfn : 52;
+};
+
+struct svsm_pvalidate_call {
+ u16 entries;
+ u16 next;
+
+ u8 rsvd1[4];
+
+ struct svsm_pvalidate_entry entry[];
+};
/*
* SVSM protocol structure
@@ -207,6 +228,7 @@ struct svsm_call {
#define SVSM_CORE_CALL(x) ((0ULL << 32) | (x))
#define SVSM_CORE_REMAP_CA 0
+#define SVSM_CORE_PVALIDATE 1
#ifdef CONFIG_AMD_MEM_ENCRYPT
extern void __sev_es_ist_enter(struct pt_regs *regs);
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 6f57eb804e70..b415b10a0823 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -40,6 +40,9 @@ 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;
+static struct svsm_ca *__svsm_get_caa(void);
+static u64 __svsm_get_caa_pa(void);
+
/* I/O parameters for CPUID-related helpers */
struct cpuid_leaf {
u32 fn;
@@ -102,6 +105,8 @@ static u32 cpuid_std_range_max __ro_after_init;
static u32 cpuid_hyp_range_max __ro_after_init;
static u32 cpuid_ext_range_max __ro_after_init;
+static int svsm_protocol(struct svsm_call *call);
+
static bool __init sev_es_check_cpu_features(void)
{
if (!has_cpuflag(X86_FEATURE_RDRAND)) {
@@ -1186,7 +1191,65 @@ static void __head setup_cpuid_table(const struct cc_blob_sev_info *cc_info)
}
}
-static void pvalidate_pages(struct snp_psc_desc *desc)
+static int base_pvalidate_4k_page(unsigned long vaddr, bool validate)
+{
+ return pvalidate(vaddr, RMP_PG_SIZE_4K, validate);
+}
+
+static int svsm_pvalidate_4k_page(unsigned long paddr, bool validate)
+{
+ struct svsm_pvalidate_call *pvalidate_call;
+ struct svsm_call call = {};
+ u64 pvalidate_call_pa;
+ unsigned long flags;
+ int ret;
+
+ /*
+ * This can be called very early in the boot, use native functions in
+ * order to avoid paravirt issues.
+ */
+ flags = native_save_fl();
+ if (flags & X86_EFLAGS_IF)
+ native_irq_disable();
+
+ call.caa = __svsm_get_caa();
+
+ pvalidate_call = (struct svsm_pvalidate_call *)call.caa->svsm_buffer;
+ pvalidate_call_pa = __svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
+
+ pvalidate_call->entries = 1;
+ pvalidate_call->next = 0;
+ pvalidate_call->entry[0].page_size = RMP_PG_SIZE_4K;
+ pvalidate_call->entry[0].action = validate;
+ pvalidate_call->entry[0].ignore_cf = 0;
+ pvalidate_call->entry[0].pfn = paddr >> PAGE_SHIFT;
+
+ /* Protocol 0, Call ID 1 */
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_PVALIDATE);
+ call.rcx = pvalidate_call_pa;
+
+ ret = svsm_protocol(&call);
+
+ if (flags & X86_EFLAGS_IF)
+ native_irq_enable();
+
+ return ret;
+}
+
+static void pvalidate_4k_page(unsigned long vaddr, unsigned long paddr, bool validate)
+{
+ int ret;
+
+ ret = vmpl ? svsm_pvalidate_4k_page(paddr, validate)
+ : base_pvalidate_4k_page(vaddr, validate);
+
+ if (ret) {
+ WARN(1, "Failed to validate address 0x%lx ret %d", vaddr, ret);
+ sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+ }
+}
+
+static void base_pvalidate_pages(struct snp_psc_desc *desc)
{
struct psc_entry *e;
unsigned long vaddr;
@@ -1220,6 +1283,120 @@ static void pvalidate_pages(struct snp_psc_desc *desc)
}
}
+static void svsm_pvalidate_pages(struct snp_psc_desc *desc)
+{
+ struct svsm_pvalidate_call *pvalidate_call;
+ struct svsm_pvalidate_entry *pe;
+ unsigned int call_count, i;
+ struct svsm_call call = {};
+ u64 pvalidate_call_pa;
+ struct psc_entry *e;
+ unsigned long flags;
+ unsigned long vaddr;
+ bool action;
+ int ret;
+
+ /*
+ * This can be called very early in the boot, use native functions in
+ * order to avoid paravirt issues.
+ */
+ flags = native_save_fl();
+ if (flags & X86_EFLAGS_IF)
+ native_irq_disable();
+
+ call.caa = __svsm_get_caa();
+
+ pvalidate_call = (struct svsm_pvalidate_call *)call.caa->svsm_buffer;
+ pvalidate_call_pa = __svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
+
+ /* Calculate how many entries the CA buffer can hold */
+ call_count = sizeof(call.caa->svsm_buffer);
+ call_count -= offsetof(struct svsm_pvalidate_call, entry);
+ call_count /= sizeof(pvalidate_call->entry[0]);
+
+ /* Protocol 0, Call ID 1 */
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_PVALIDATE);
+ call.rcx = pvalidate_call_pa;
+
+ pvalidate_call->entries = 0;
+ pvalidate_call->next = 0;
+
+ for (i = 0; i <= desc->hdr.end_entry; i++) {
+ e = &desc->entries[i];
+ pe = &pvalidate_call->entry[pvalidate_call->entries];
+
+ pe->page_size = e->pagesize ? RMP_PG_SIZE_2M : RMP_PG_SIZE_4K;
+ pe->action = e->operation == SNP_PAGE_STATE_PRIVATE;
+ pe->ignore_cf = 0;
+ pe->pfn = e->gfn;
+
+ pvalidate_call->entries++;
+ if (pvalidate_call->entries < call_count && i != desc->hdr.end_entry)
+ continue;
+
+ ret = svsm_protocol(&call);
+ if (ret == SVSM_PVALIDATE_FAIL_SIZEMISMATCH &&
+ pvalidate_call->entry[pvalidate_call->next].page_size == RMP_PG_SIZE_2M) {
+ u64 pfn, pfn_end;
+
+ /*
+ * The "next" field is the index of the failed entry. Calculate the
+ * index of the entry after the failed entry before the fields are
+ * cleared so that processing can continue on from that point (take
+ * into account the for loop adding 1 to the entry).
+ */
+ i -= pvalidate_call->entries - pvalidate_call->next;
+ i += 1;
+
+ action = pvalidate_call->entry[pvalidate_call->next].action;
+ pfn = pvalidate_call->entry[pvalidate_call->next].pfn;
+ pfn_end = pfn + 511;
+
+ pvalidate_call->entries = 0;
+ pvalidate_call->next = 0;
+ for (; pfn <= pfn_end; pfn++) {
+ pe = &pvalidate_call->entry[pvalidate_call->entries];
+
+ pe->page_size = RMP_PG_SIZE_4K;
+ pe->action = action;
+ pe->ignore_cf = 0;
+ pe->pfn = pfn;
+
+ pvalidate_call->entries++;
+ if (pvalidate_call->entries < call_count && pfn != pfn_end)
+ continue;
+
+ ret = svsm_protocol(&call);
+ if (ret != SVSM_SUCCESS)
+ break;
+
+ pvalidate_call->entries = 0;
+ pvalidate_call->next = 0;
+ }
+ }
+
+ if (ret != SVSM_SUCCESS) {
+ pe = &pvalidate_call->entry[pvalidate_call->next];
+ vaddr = (unsigned long)pfn_to_kaddr(pe->pfn);
+
+ WARN(1, "Failed to validate address %lx ret=%#x (%d)", vaddr, ret, ret);
+ sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+ }
+
+ pvalidate_call->entries = 0;
+ pvalidate_call->next = 0;
+ }
+
+ if (flags & X86_EFLAGS_IF)
+ native_irq_enable();
+}
+
+static void pvalidate_pages(struct snp_psc_desc *desc)
+{
+ vmpl ? svsm_pvalidate_pages(desc)
+ : base_pvalidate_pages(desc);
+}
+
static int vmgexit_psc(struct ghcb *ghcb, struct snp_psc_desc *desc)
{
int cur_entry, end_entry, ret = 0;
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 21f3cc40d662..49cf4a6f1f31 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -622,6 +622,12 @@ static struct svsm_ca *__svsm_get_caa(void)
: boot_svsm_caa;
}
+static u64 __svsm_get_caa_pa(void)
+{
+ return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa_pa)
+ : boot_svsm_caa_pa;
+}
+
static noinstr void __sev_put_ghcb(struct ghcb_state *state)
{
struct sev_es_runtime_data *data;
@@ -792,7 +798,6 @@ early_set_pages_state(unsigned long vaddr, unsigned long paddr,
{
unsigned long paddr_end;
u64 val;
- int ret;
vaddr = vaddr & PAGE_MASK;
@@ -800,12 +805,9 @@ early_set_pages_state(unsigned long vaddr, unsigned long paddr,
paddr_end = paddr + (npages << PAGE_SHIFT);
while (paddr < paddr_end) {
- if (op == SNP_PAGE_STATE_SHARED) {
- /* Page validation must be rescinded before changing to shared */
- ret = pvalidate(vaddr, RMP_PG_SIZE_4K, false);
- if (WARN(ret, "Failed to validate address 0x%lx ret %d", paddr, ret))
- goto e_term;
- }
+ /* Page validation must be rescinded before changing to shared */
+ if (op == SNP_PAGE_STATE_SHARED)
+ pvalidate_4k_page(vaddr, paddr, false);
/*
* Use the MSR protocol because this function can be called before
@@ -827,12 +829,9 @@ early_set_pages_state(unsigned long vaddr, unsigned long paddr,
paddr, GHCB_MSR_PSC_RESP_VAL(val)))
goto e_term;
- if (op == SNP_PAGE_STATE_PRIVATE) {
- /* Page validation must be performed after changing to private */
- ret = pvalidate(vaddr, RMP_PG_SIZE_4K, true);
- if (WARN(ret, "Failed to validate address 0x%lx ret %d", paddr, ret))
- goto e_term;
- }
+ /* Page validation must be performed after changing to private */
+ if (op == SNP_PAGE_STATE_PRIVATE)
+ pvalidate_4k_page(vaddr, paddr, true);
vaddr += PAGE_SIZE;
paddr += PAGE_SIZE;
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 07/15] x86/sev: Use the SVSM to create a vCPU when not in VMPL0
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (5 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 06/15] x86/sev: Perform PVALIDATE using the SVSM when not at VMPL0 Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 08/15] x86/sev: Provide SVSM discovery support Tom Lendacky
` (8 subsequent siblings)
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
Using the RMPADJUST instruction, the VSMA attribute can only be changed
at VMPL0. An SVSM will be present when running at VMPL1 or a lower
privilege level.
When an SVSM is present, use the SVSM_CORE_CREATE_VCPU call or the
SVSM_CORE_DESTROY_VCPU call to perform VMSA attribute changes. Use the
VMPL level supplied by the SVSM for the VMSA when starting the AP.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/include/asm/sev.h | 2 ++
arch/x86/kernel/sev.c | 60 +++++++++++++++++++++++++++++++++-----
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index e6f1ed3f6ce3..a7312b936d16 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -229,6 +229,8 @@ struct svsm_call {
#define SVSM_CORE_CALL(x) ((0ULL << 32) | (x))
#define SVSM_CORE_REMAP_CA 0
#define SVSM_CORE_PVALIDATE 1
+#define SVSM_CORE_CREATE_VCPU 2
+#define SVSM_CORE_DELETE_VCPU 3
#ifdef CONFIG_AMD_MEM_ENCRYPT
extern void __sev_es_ist_enter(struct pt_regs *regs);
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 49cf4a6f1f31..3f4342b31736 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -995,7 +995,7 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
}
-static int snp_set_vmsa(void *va, bool vmsa)
+static int base_snp_set_vmsa(void *va, bool vmsa)
{
u64 attrs;
@@ -1013,6 +1013,40 @@ static int snp_set_vmsa(void *va, bool vmsa)
return rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
}
+static int svsm_snp_set_vmsa(void *va, void *caa, int apic_id, bool vmsa)
+{
+ struct svsm_call call = {};
+ unsigned long flags;
+ int ret;
+
+ local_irq_save(flags);
+
+ call.caa = this_cpu_read(svsm_caa);
+ call.rcx = __pa(va);
+
+ if (vmsa) {
+ /* Protocol 0, Call ID 2 */
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_CREATE_VCPU);
+ call.rdx = __pa(caa);
+ call.r8 = apic_id;
+ } else {
+ /* Protocol 0, Call ID 3 */
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_DELETE_VCPU);
+ }
+
+ ret = svsm_protocol(&call);
+
+ local_irq_restore(flags);
+
+ return ret;
+}
+
+static int snp_set_vmsa(void *va, void *caa, int apic_id, bool vmsa)
+{
+ return vmpl ? svsm_snp_set_vmsa(va, caa, apic_id, vmsa)
+ : base_snp_set_vmsa(va, vmsa);
+}
+
#define __ATTR_BASE (SVM_SELECTOR_P_MASK | SVM_SELECTOR_S_MASK)
#define INIT_CS_ATTRIBS (__ATTR_BASE | SVM_SELECTOR_READ_MASK | SVM_SELECTOR_CODE_MASK)
#define INIT_DS_ATTRIBS (__ATTR_BASE | SVM_SELECTOR_WRITE_MASK)
@@ -1044,11 +1078,11 @@ static void *snp_alloc_vmsa_page(int cpu)
return page_address(p + 1);
}
-static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa)
+static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa, int apic_id)
{
int err;
- err = snp_set_vmsa(vmsa, false);
+ err = snp_set_vmsa(vmsa, NULL, apic_id, false);
if (err)
pr_err("clear VMSA page failed (%u), leaking page\n", err);
else
@@ -1059,6 +1093,7 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
{
struct sev_es_save_area *cur_vmsa, *vmsa;
struct ghcb_state state;
+ struct svsm_ca *caa;
unsigned long flags;
struct ghcb *ghcb;
u8 sipi_vector;
@@ -1105,6 +1140,12 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
if (!vmsa)
return -ENOMEM;
+ /*
+ * If an SVSM is present, then the SVSM CAA per-CPU variable will
+ * have a value, otherwise it will be NULL.
+ */
+ caa = per_cpu(svsm_caa, cpu);
+
/* CR4 should maintain the MCE value */
cr4 = native_read_cr4() & X86_CR4_MCE;
@@ -1152,11 +1193,11 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
* VMPL level
* SEV_FEATURES (matches the SEV STATUS MSR right shifted 2 bits)
*/
- vmsa->vmpl = 0;
+ vmsa->vmpl = vmpl;
vmsa->sev_features = sev_status >> 2;
/* Switch the page over to a VMSA page now that it is initialized */
- ret = snp_set_vmsa(vmsa, true);
+ ret = snp_set_vmsa(vmsa, caa, apic_id, true);
if (ret) {
pr_err("set VMSA page failed (%u)\n", ret);
free_page((unsigned long)vmsa);
@@ -1172,7 +1213,10 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
vc_ghcb_invalidate(ghcb);
ghcb_set_rax(ghcb, vmsa->sev_features);
ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_AP_CREATION);
- ghcb_set_sw_exit_info_1(ghcb, ((u64)apic_id << 32) | SVM_VMGEXIT_AP_CREATE);
+ ghcb_set_sw_exit_info_1(ghcb,
+ ((u64)apic_id << 32) |
+ ((u64)vmpl << 16) |
+ SVM_VMGEXIT_AP_CREATE);
ghcb_set_sw_exit_info_2(ghcb, __pa(vmsa));
sev_es_wr_ghcb_msr(__pa(ghcb));
@@ -1190,13 +1234,13 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
/* Perform cleanup if there was an error */
if (ret) {
- snp_cleanup_vmsa(vmsa);
+ snp_cleanup_vmsa(vmsa, apic_id);
vmsa = NULL;
}
/* Free up any previous VMSA page */
if (cur_vmsa)
- snp_cleanup_vmsa(cur_vmsa);
+ snp_cleanup_vmsa(cur_vmsa, apic_id);
/* Record the current VMSA page */
per_cpu(sev_vmsa, cpu) = vmsa;
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 08/15] x86/sev: Provide SVSM discovery support
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (6 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 07/15] x86/sev: Use the SVSM to create a vCPU when not in VMPL0 Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 09/15] x86/sev: Provide guest VMPL level to userspace Tom Lendacky
` (7 subsequent siblings)
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
The SVSM specification documents an alternative method of discovery for
the SVSM using a reserved CPUID bit and a reserved MSR.
For the CPUID support, the SNP CPUID table is updated to set bit 28 of
the EAX register of the 0x8000001f leaf when an SVSM is present. This bit
has been reserved for use in this capacity.
For the MSR support, a new reserved MSR 0xc001f000 has been defined. A #VC
should be generated when accessing this MSR. The #VC handler is expected
to ignore writes to this MSR and return the physical calling area address
(CAA) on reads of this MSR.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 2 ++
arch/x86/kernel/sev-shared.c | 11 +++++++++++
arch/x86/kernel/sev.c | 17 +++++++++++++++++
4 files changed, 31 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 3c7434329661..a17a81b3189b 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -446,6 +446,7 @@
#define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
#define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */
#define X86_FEATURE_DEBUG_SWAP (19*32+14) /* AMD SEV-ES full debug state swap support */
+#define X86_FEATURE_SVSM_PRESENT (19*32+28) /* "" SNP SVSM is present */
/* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
#define X86_FEATURE_NO_NESTED_DATA_BP (20*32+ 0) /* "" No Nested Data Breakpoints */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e022e6eb766c..45ffa27569f4 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -660,6 +660,8 @@
#define MSR_AMD64_RMP_BASE 0xc0010132
#define MSR_AMD64_RMP_END 0xc0010133
+#define MSR_SVSM_CAA 0xc001f000
+
/* AMD Collaborative Processor Performance Control MSRs */
#define MSR_AMD_CPPC_CAP1 0xc00102b0
#define MSR_AMD_CPPC_ENABLE 0xc00102b1
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index b415b10a0823..50db783f151e 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -1561,6 +1561,8 @@ static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
{
struct snp_secrets_page *secrets_page;
+ struct snp_cpuid_table *cpuid_table;
+ unsigned int i;
u64 caa;
BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
@@ -1607,4 +1609,13 @@ static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
*/
boot_svsm_caa = (struct svsm_ca *)caa;
boot_svsm_caa_pa = caa;
+
+ /* Advertise the SVSM presence via CPUID. */
+ cpuid_table = (struct snp_cpuid_table *)snp_cpuid_get_table();
+ for (i = 0; i < cpuid_table->count; i++) {
+ struct snp_cpuid_fn *fn = &cpuid_table->fn[i];
+
+ if (fn->eax_in == 0x8000001f)
+ fn->eax |= BIT(28);
+ }
}
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 3f4342b31736..69a756781d90 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -1326,12 +1326,29 @@ int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
return 0;
}
+static enum es_result vc_handle_svsm_caa_msr(struct es_em_ctxt *ctxt)
+{
+ struct pt_regs *regs = ctxt->regs;
+
+ /* Writes to the SVSM CAA msr are ignored */
+ if (ctxt->insn.opcode.bytes[1] == 0x30)
+ return ES_OK;
+
+ regs->ax = lower_32_bits(this_cpu_read(svsm_caa_pa));
+ regs->dx = upper_32_bits(this_cpu_read(svsm_caa_pa));
+
+ return ES_OK;
+}
+
static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
{
struct pt_regs *regs = ctxt->regs;
enum es_result ret;
u64 exit_info_1;
+ if (regs->cx == MSR_SVSM_CAA)
+ return vc_handle_svsm_caa_msr(ctxt);
+
/* Is it a WRMSR? */
exit_info_1 = (ctxt->insn.opcode.bytes[1] == 0x30) ? 1 : 0;
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 09/15] x86/sev: Provide guest VMPL level to userspace
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (7 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 08/15] x86/sev: Provide SVSM discovery support Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL Tom Lendacky
` (6 subsequent siblings)
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
Requesting an attestation report from userspace involves providing the
VMPL level for the report. Currently any value from 0-3 is valid because
Linux enforces running at VMPL0.
When an SVSM is present, though, Linux will not be running at VMPL0 and
only VMPL values starting at the VMPL level Linux is running at to 3 are
valid. In order to allow userspace to determine the minimum VMPL value
that can be supplied to an attestation report, create a sysfs entry that
can be used to retrieve the current VMPL level of Linux.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/kernel/sev.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 69a756781d90..8edf7362136b 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2507,3 +2507,40 @@ void __init snp_remap_svsm_ca(void)
/* Update the CAA to a proper kernel address */
boot_svsm_caa = &boot_svsm_ca_page;
}
+
+static ssize_t vmpl_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%d\n", vmpl);
+}
+
+static struct kobj_attribute vmpl_attr = __ATTR_RO(vmpl);
+
+static struct attribute *vmpl_attrs[] = {
+ &vmpl_attr.attr,
+ NULL
+};
+
+static struct attribute_group sev_attr_group = {
+ .attrs = vmpl_attrs,
+};
+
+static int __init sev_sysfs_init(void)
+{
+ struct kobject *sev_kobj;
+ int ret;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return -ENODEV;
+
+ sev_kobj = kobject_create_and_add("sev", kernel_kobj);
+ if (!sev_kobj)
+ return -ENOMEM;
+
+ ret = sysfs_create_group(sev_kobj, &sev_attr_group);
+ if (ret)
+ kobject_put(sev_kobj);
+
+ return ret;
+}
+arch_initcall(sev_sysfs_init);
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (8 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 09/15] x86/sev: Provide guest VMPL level to userspace Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-05-01 23:57 ` [svsm-devel] " Jacob Xu
2024-04-24 15:58 ` [PATCH v4 11/15] configfs-tsm: Allow the privlevel_floor attribute to be updated Tom Lendacky
` (5 subsequent siblings)
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
Currently, the sev-guest driver uses the vmpck-0 key by default. When an
SVSM is present the kernel is running at a VMPL other than 0 and the
vmpck-0 key is no longer available. If a specific vmpck key has not be
requested by the user via the vmpck_id module parameter, choose the vmpck
key based on the active VMPL level.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/include/asm/sev.h | 2 ++
arch/x86/kernel/sev.c | 6 ++++++
drivers/virt/coco/sev-guest/sev-guest.c | 10 +++++++---
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index a7312b936d16..64fcadd6d602 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -307,6 +307,7 @@ u64 snp_get_unsupported_features(u64 status);
u64 sev_get_status(void);
void sev_show_status(void);
void snp_remap_svsm_ca(void);
+int snp_get_vmpl(void);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
static inline void sev_es_ist_exit(void) { }
@@ -337,6 +338,7 @@ static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
static inline u64 sev_get_status(void) { return 0; }
static inline void sev_show_status(void) { }
static inline void snp_remap_svsm_ca(void) { }
+static inline int snp_get_vmpl(void) { return 0; }
#endif
#ifdef CONFIG_KVM_AMD_SEV
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 8edf7362136b..75f11da867a3 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2454,6 +2454,12 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn
}
EXPORT_SYMBOL_GPL(snp_issue_guest_request);
+int snp_get_vmpl(void)
+{
+ return vmpl;
+}
+EXPORT_SYMBOL_GPL(snp_get_vmpl);
+
static struct platform_device sev_guest_device = {
.name = "sev-guest",
.id = -1,
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 04a7bd1e4314..e7dd7df86427 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -2,7 +2,7 @@
/*
* AMD Secure Encrypted Virtualization (SEV) guest driver interface
*
- * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ * Copyright (C) 2021-2024 Advanced Micro Devices, Inc.
*
* Author: Brijesh Singh <brijesh.singh@amd.com>
*/
@@ -70,8 +70,8 @@ struct snp_guest_dev {
u8 *vmpck;
};
-static u32 vmpck_id;
-module_param(vmpck_id, uint, 0444);
+static int vmpck_id = -1;
+module_param(vmpck_id, int, 0444);
MODULE_PARM_DESC(vmpck_id, "The VMPCK ID to use when communicating with the PSP.");
/* Mutex to serialize the shared buffer access and command handling. */
@@ -923,6 +923,10 @@ static int __init sev_guest_probe(struct platform_device *pdev)
if (!snp_dev)
goto e_unmap;
+ /* Adjust the default VMPCK key based on the executing VMPL level */
+ if (vmpck_id == -1)
+ vmpck_id = snp_get_vmpl();
+
ret = -EINVAL;
snp_dev->vmpck = get_vmpck(vmpck_id, layout, &snp_dev->os_area_msg_seqno);
if (!snp_dev->vmpck) {
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 11/15] configfs-tsm: Allow the privlevel_floor attribute to be updated
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (9 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-26 20:51 ` Dan Williams
2024-04-24 15:58 ` [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility Tom Lendacky
` (4 subsequent siblings)
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
With the introduction of an SVSM, Linux will be running at a non-zero
VMPL. Any request for an attestation report at a higher privilege VMPL
than what Linux is currently running will result in an error. Allow for
the privlevel_floor attribute to be updated dynamically so that the
attribute may be set dynamically. Set the privlevel_floor attribute to
be the value of the vmpck_id being used.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
drivers/virt/coco/sev-guest/sev-guest.c | 5 ++++-
include/linux/tsm.h | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index e7dd7df86427..2abb51bd034f 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -885,7 +885,7 @@ static int sev_report_new(struct tsm_report *report, void *data)
return 0;
}
-static const struct tsm_ops sev_tsm_ops = {
+static struct tsm_ops sev_tsm_ops = {
.name = KBUILD_MODNAME,
.report_new = sev_report_new,
};
@@ -972,6 +972,9 @@ static int __init sev_guest_probe(struct platform_device *pdev)
snp_dev->input.resp_gpa = __pa(snp_dev->response);
snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
+ /* 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);
if (ret)
goto e_free_cert_data;
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index de8324a2223c..50c5769657d8 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -54,7 +54,7 @@ struct tsm_report {
*/
struct tsm_ops {
const char *name;
- const unsigned int privlevel_floor;
+ unsigned int privlevel_floor;
int (*report_new)(struct tsm_report *report, void *data);
};
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (10 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 11/15] configfs-tsm: Allow the privlevel_floor attribute to be updated Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-26 21:48 ` Dan Williams
2024-04-24 15:58 ` [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM Tom Lendacky
` (3 subsequent siblings)
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra, Joel Becker, Christoph Hellwig
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 displayed. If no callback is registered, the attribute
is displayed by default.
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Christoph Hellwig <hch@lst.de>
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
fs/configfs/dir.c | 20 ++++++++++++++++++++
include/linux/configfs.h | 3 +++
2 files changed, 23 insertions(+)
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 18677cd4e62f..463e66258507 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -580,6 +580,7 @@ static void detach_attrs(struct config_item * item)
static int populate_attrs(struct config_item *item)
{
const struct config_item_type *t = item->ci_type;
+ struct configfs_group_operations *ops;
struct configfs_attribute *attr;
struct configfs_bin_attribute *bin_attr;
int error = 0;
@@ -587,14 +588,33 @@ static int populate_attrs(struct config_item *item)
if (!t)
return -EINVAL;
+
+ ops = t->ct_group_ops;
+ if (!ops) {
+ struct config_group *g = item->ci_group;
+
+ /*
+ * No item specific group operations, check if the item's group
+ * has group operations.
+ */
+ if (g && g->cg_item.ci_type)
+ ops = g->cg_item.ci_type->ct_group_ops;
+ }
+
if (t->ct_attrs) {
for (i = 0; (attr = t->ct_attrs[i]) != NULL; i++) {
+ if (ops && ops->is_visible && !ops->is_visible(item, attr, i))
+ continue;
+
if ((error = configfs_create_file(item, attr)))
break;
}
}
if (t->ct_bin_attrs) {
for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
+ if (ops && ops->is_bin_visible && !ops->is_bin_visible(item, bin_attr, i))
+ 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..c771e9d0d0b9 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -216,6 +216,9 @@ struct configfs_group_operations {
struct config_group *(*make_group)(struct config_group *group, const char *name);
void (*disconnect_notify)(struct config_group *group, struct config_item *item);
void (*drop_item)(struct config_group *group, struct config_item *item);
+ bool (*is_visible)(struct config_item *item, struct configfs_attribute *attr, int n);
+ bool (*is_bin_visible)(struct config_item *item, struct configfs_bin_attribute *attr,
+ int n);
};
struct configfs_subsystem {
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (11 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-26 21:58 ` Dan Williams
` (2 more replies)
2024-04-24 15:58 ` [PATCH v4 14/15] x86/sev: Extend the config-fs attestation support for an SVSM Tom Lendacky
` (2 subsequent siblings)
15 siblings, 3 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
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 <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
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
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 14/15] x86/sev: Extend the config-fs attestation support for an SVSM
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (12 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present Tom Lendacky
2024-05-03 11:38 ` [svsm-devel] [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Jörg Rödel
15 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
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:
- 'service-provider' (input)
This attribute is used to determine whether the attestation request
should be sent to the specified service provider or to the SEV
firmware. The SVSM service provider is represented by the value
'svsm'.
- 'service_guid' (input)
Used for requesting the attestation of a single service within the
service provider. 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.
Only display these new attributes when running under an SVSM.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
Documentation/ABI/testing/configfs-tsm | 63 ++++++++
arch/x86/include/asm/sev.h | 31 +++-
arch/x86/kernel/sev.c | 50 +++++++
drivers/virt/coco/sev-guest/sev-guest.c | 186 ++++++++++++++++++++++++
drivers/virt/coco/tsm.c | 93 +++++++++++-
include/linux/tsm.h | 19 +++
6 files changed, 439 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
index dd24202b5ba5..bc8f6efa5d6f 100644
--- a/Documentation/ABI/testing/configfs-tsm
+++ b/Documentation/ABI/testing/configfs-tsm
@@ -31,6 +31,18 @@ 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.10
+Contact: linux-coco@lists.linux.dev
+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.
+
+ See 'service_provider' for information on the format of the
+ manifest blob.
+
What: /sys/kernel/config/tsm/report/$name/provider
Date: September, 2023
KernelVersion: v6.7
@@ -80,3 +92,54 @@ Contact: linux-coco@lists.linux.dev
Description:
(RO) Indicates the minimum permissible value that can be written
to @privlevel.
+
+What: /sys/kernel/config/tsm/report/$name/service_provider
+Date: January, 2024
+KernelVersion: v6.10
+Contact: linux-coco@lists.linux.dev
+Description:
+ (WO) Attribute is visible if a TSM implementation provider
+ supports the concept of attestation reports from a service
+ provider for TVMs, like SEV-SNP running under an SVSM.
+ Specifying the service provider via this attribute will create
+ an attestation report as specified by the service provider.
+ Currently supported service-providers are:
+ svsm
+
+ For the "svsm" service provider, see the 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.10
+Contact: linux-coco@lists.linux.dev
+Description:
+ (WO) Attribute is visible if a TSM implementation provider
+ supports the concept of attestation reports from a service
+ provider for TVMs, like SEV-SNP running under an SVSM.
+ Specifying an empty/null GUID (00000000-0000-0000-0000-000000)
+ requests all active services within the service provider be
+ part of the attestation report. Specifying a GUID request
+ an attestation report of just the specified service using the
+ manifest form specified by the service_manifest_version
+ attribute.
+
+ See 'service_provider' for information on the format of the
+ service guid.
+
+What: /sys/kernel/config/tsm/report/$name/service_manifest_version
+Date: January, 2024
+KernelVersion: v6.10
+Contact: linux-coco@lists.linux.dev
+Description:
+ (WO) Attribute is visible if a TSM implementation provider
+ supports the concept of attestation reports from a service
+ provider for TVMs, like SEV-SNP running under an SVSM.
+ Indicates the service manifest version requested for the
+ attestation report (default 0). 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.
+
+ See 'service_provider' for information on the format of the
+ service manifest version.
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 64fcadd6d602..2c66e7f2a7c7 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -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 @@ bool snp_init(struct boot_params *bp);
void __noreturn snp_abort(void);
void snp_dmi_setup(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);
@@ -332,7 +358,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; }
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 75f11da867a3..5e71c94b952c 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2390,6 +2390,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;
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index ec3d894cfe31..d37a8b27d12a 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -39,6 +39,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;
@@ -784,6 +786,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;
+
+ if (guid_is_null(&desc->service_guid)) {
+ 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);
+ }
+
+ 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);
+ attest_call.nonce.len = desc->inblob_len;
+
+ 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;
@@ -798,6 +942,13 @@ static int sev_report_new(struct tsm_report *report, void *data)
if (desc->inblob_len != SNP_REPORT_USER_DATA_SIZE)
return -EINVAL;
+ if (desc->service_provider) {
+ if (strcmp(desc->service_provider, "svsm"))
+ return -EINVAL;
+
+ return sev_svsm_report_new(report, data);
+ }
+
void *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -886,9 +1037,44 @@ static int sev_report_new(struct tsm_report *report, void *data)
return 0;
}
+static bool sev_report_attr_visible(struct config_item *item,
+ struct configfs_attribute *attr, int n)
+{
+ switch (n) {
+ case TSM_REPORT_GENERATION:
+ case TSM_REPORT_PROVIDER:
+ case TSM_REPORT_PRIVLEVEL:
+ case TSM_REPORT_PRIVLEVEL_FLOOR:
+ return true;
+ case TSM_REPORT_SERVICE_PROVIDER:
+ case TSM_REPORT_SERVICE_GUID:
+ case TSM_REPORT_SERVICE_MANIFEST_VER:
+ return snp_get_vmpl();
+ }
+
+ return false;
+}
+
+static bool sev_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:
+ case TSM_REPORT_AUXBLOB:
+ return true;
+ case TSM_REPORT_MANIFESTBLOB:
+ return snp_get_vmpl();
+ }
+
+ return false;
+}
+
static struct tsm_ops sev_tsm_ops = {
.name = KBUILD_MODNAME,
.report_new = sev_report_new,
+ .report_attr_visible = sev_report_attr_visible,
+ .report_bin_attr_visible = sev_report_bin_attr_visible,
};
static void unregister_sev_tsm(void *data)
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index dedb4f582630..3c9587e8746b 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -34,7 +34,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 {
@@ -47,6 +47,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)
@@ -118,6 +119,74 @@ static ssize_t tsm_report_privlevel_floor_show(struct config_item *cfg,
}
CONFIGFS_ATTR_RO(tsm_report_, privlevel_floor);
+static ssize_t tsm_report_service_provider_store(struct config_item *cfg,
+ const char *buf, size_t len)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+ size_t sp_len;
+ char *sp;
+ int rc;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ rc = try_advance_write_generation(report);
+ if (rc)
+ return rc;
+
+ sp_len = (buf[len - 1] != '\n') ? len : len - 1;
+
+ sp = kstrndup(buf, sp_len, GFP_KERNEL);
+ if (!sp)
+ return -ENOMEM;
+ kfree(report->desc.service_provider);
+
+ report->desc.service_provider = sp;
+
+ return len;
+}
+CONFIGFS_ATTR_WO(tsm_report_, service_provider);
+
+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);
+ int rc;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ rc = try_advance_write_generation(report);
+ if (rc)
+ return rc;
+
+ report->desc.service_guid = guid_null;
+
+ rc = guid_parse(buf, &report->desc.service_guid);
+ if (rc)
+ return rc;
+
+ 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)
{
@@ -162,6 +231,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;
@@ -187,7 +259,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)
@@ -223,8 +295,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;
@@ -251,11 +325,23 @@ 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);
+
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,
+ [TSM_REPORT_SERVICE_PROVIDER] = &tsm_report_attr_service_provider,
+ [TSM_REPORT_SERVICE_GUID] = &tsm_report_attr_service_guid,
+ [TSM_REPORT_SERVICE_MANIFEST_VER] = &tsm_report_attr_service_manifest_version,
NULL,
};
@@ -263,6 +349,7 @@ 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,
+ [TSM_REPORT_MANIFESTBLOB] = &tsm_report_attr_manifestblob,
NULL,
};
@@ -271,8 +358,10 @@ 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(report->desc.service_provider);
kfree(state);
}
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index fa19291a9854..7c52a16a61c3 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/configfs.h>
+#include <linux/uuid.h>
#define TSM_INBLOB_MAX 64
#define TSM_OUTBLOB_MAX SZ_32K
@@ -20,11 +21,17 @@
* @privlevel: optional privilege level to associate with @outblob
* @inblob_len: sizeof @inblob
* @inblob: arbitrary input data
+ * @service_provider: optional name of where to obtain the tsm report blob
+ * @service_guid: optional service-provider service guid to attest
+ * @service_manifest_version: optional service-provider service manifest version requested
*/
struct tsm_desc {
unsigned int privlevel;
size_t inblob_len;
u8 inblob[TSM_INBLOB_MAX];
+ char *service_provider;
+ guid_t service_guid;
+ unsigned int service_manifest_version;
};
/**
@@ -34,6 +41,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;
@@ -41,6 +50,8 @@ struct tsm_report {
u8 *outblob;
size_t auxblob_len;
u8 *auxblob;
+ size_t manifestblob_len;
+ u8 *manifestblob;
};
/**
@@ -49,12 +60,18 @@ struct tsm_report {
* @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
+ * @TSM_REPORT_SERVICE_PROVIDER: index of the service provider identifier attribute
+ * @TSM_REPORT_SERVICE_GUID: index of the service GUID attribute
+ * @TSM_REPORT_SERVICE_MANIFEST_VER: index of the service manifest version attribute
*/
enum tsm_attr_index {
TSM_REPORT_GENERATION,
TSM_REPORT_PROVIDER,
TSM_REPORT_PRIVLEVEL,
TSM_REPORT_PRIVLEVEL_FLOOR,
+ TSM_REPORT_SERVICE_PROVIDER,
+ TSM_REPORT_SERVICE_GUID,
+ TSM_REPORT_SERVICE_MANIFEST_VER,
};
/**
@@ -62,11 +79,13 @@ enum tsm_attr_index {
* @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
+ * @TSM_REPORT_MANIFESTBLOB: index of the binary manifest data attribute
*/
enum tsm_bin_attr_index {
TSM_REPORT_INBLOB,
TSM_REPORT_OUTBLOB,
TSM_REPORT_AUXBLOB,
+ TSM_REPORT_MANIFESTBLOB,
};
/**
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (13 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 14/15] x86/sev: Extend the config-fs attestation support for an SVSM Tom Lendacky
@ 2024-04-24 15:58 ` Tom Lendacky
2024-05-03 11:37 ` [svsm-devel] " Jörg Rödel
2024-05-03 11:38 ` [svsm-devel] [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Jörg Rödel
15 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-24 15:58 UTC (permalink / raw)
To: linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
To allow execution at a level other than VMPL0, an SVSM must be present.
Allow the SEV-SNP guest to continue booting if an SVSM is detected and
the hypervisor supports the SVSM feature as indicated in the GHCB
hypervisor features bitmap.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/boot/compressed/sev.c | 12 +++++++++---
arch/x86/include/asm/sev-common.h | 1 +
arch/x86/kernel/sev.c | 20 +++++++++++++++++---
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 32a1e98ffaa9..fb1e60165cd1 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -610,11 +610,13 @@ void sev_enable(struct boot_params *bp)
* features.
*/
if (sev_status & MSR_AMD64_SEV_SNP_ENABLED) {
- if (!(get_hv_features() & GHCB_HV_FT_SNP))
+ u64 hv_features = get_hv_features();
+
+ if (!(hv_features & GHCB_HV_FT_SNP))
sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
/*
- * Enforce running at VMPL0.
+ * Enforce running at VMPL0 or with an SVSM.
*
* RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
* higher) privilege level. Here, clear the VMPL1 permission mask of the
@@ -624,8 +626,12 @@ void sev_enable(struct boot_params *bp)
* 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.
+ *
+ * Running at VMPL0 is not required if an SVSM is present and the hypervisor
+ * supports the required SVSM GHCB events.
*/
- if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1))
+ if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1) &&
+ !(vmpl && (hv_features & GHCB_HV_FT_SNP_MULTI_VMPL)))
sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
}
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 4cc716660d4b..7a9d09458989 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -118,6 +118,7 @@ enum psc_op {
#define GHCB_HV_FT_SNP BIT_ULL(0)
#define GHCB_HV_FT_SNP_AP_CREATION BIT_ULL(1)
+#define GHCB_HV_FT_SNP_MULTI_VMPL BIT_ULL(5)
/*
* SNP Page State Change NAE event
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 5e71c94b952c..50754cc45161 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2356,22 +2356,36 @@ static void dump_cpuid_table(void)
* sort of indicator, and there's not really any other good place to do it,
* so do it here.
*/
-static int __init report_cpuid_table(void)
+static void __init report_cpuid_table(void)
{
const struct snp_cpuid_table *cpuid_table = snp_cpuid_get_table();
if (!cpuid_table->count)
- return 0;
+ return;
pr_info("Using SNP CPUID table, %d entries present.\n",
cpuid_table->count);
if (sev_cfg.debug)
dump_cpuid_table();
+}
+
+static void __init report_vmpl_level(void)
+{
+ if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ return;
+
+ pr_info("SNP running at VMPL%u.\n", vmpl);
+}
+
+static int __init report_snp_info(void)
+{
+ report_vmpl_level();
+ report_cpuid_table();
return 0;
}
-arch_initcall(report_cpuid_table);
+arch_initcall(report_snp_info);
static int __init init_sev_config(char *str)
{
--
2.43.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page
2024-04-24 15:57 ` [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page Tom Lendacky
@ 2024-04-25 13:30 ` Borislav Petkov
0 siblings, 0 replies; 41+ messages in thread
From: Borislav Petkov @ 2024-04-25 13:30 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
On Wed, Apr 24, 2024 at 10:57:57AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 995f94467101..6949fbccec40 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -648,7 +648,7 @@ static u64 __init get_secrets_page(void)
>
> static u64 __init get_snp_jump_table_addr(void)
> {
> - struct snp_secrets_page_layout *layout;
> + struct snp_secrets_page *layout;
Yes, and I'd go change that "layout" name to "secrets" too because
layout doesn't make any sense when talking about a secrets page.
This, OTOH:
addr = secrets->os_area.ap_jump_table_pa;
means something: the address comes from the secrets page. Not from the
"layout". :-)
IOW, diff ontop:
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 25056346bc18..790e4818f7c6 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -648,7 +648,7 @@ static u64 __init get_secrets_page(void)
static u64 __init get_snp_jump_table_addr(void)
{
- struct snp_secrets_page *layout;
+ struct snp_secrets_page *secrets;
void __iomem *mem;
u64 pa, addr;
@@ -662,9 +662,9 @@ static u64 __init get_snp_jump_table_addr(void)
return 0;
}
- layout = (__force struct snp_secrets_page *)mem;
+ secrets = (__force struct snp_secrets_page *)mem;
- addr = layout->os_area.ap_jump_table_pa;
+ addr = secrets->os_area.ap_jump_table_pa;
iounmap(mem);
return addr;
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 04a7bd1e4314..654290a8e1ba 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -59,7 +59,7 @@ struct snp_guest_dev {
*/
struct snp_guest_msg secret_request, secret_response;
- struct snp_secrets_page *layout;
+ struct snp_secrets_page *secrets;
struct snp_req_data input;
union {
struct snp_report_req report;
@@ -743,26 +743,26 @@ static const struct file_operations snp_guest_fops = {
.unlocked_ioctl = snp_guest_ioctl,
};
-static u8 *get_vmpck(int id, struct snp_secrets_page *layout, u32 **seqno)
+static u8 *get_vmpck(int id, struct snp_secrets_page *secrets, u32 **seqno)
{
u8 *key = NULL;
switch (id) {
case 0:
- *seqno = &layout->os_area.msg_seqno_0;
- key = layout->vmpck0;
+ *seqno = &secrets->os_area.msg_seqno_0;
+ key = secrets->vmpck0;
break;
case 1:
- *seqno = &layout->os_area.msg_seqno_1;
- key = layout->vmpck1;
+ *seqno = &secrets->os_area.msg_seqno_1;
+ key = secrets->vmpck1;
break;
case 2:
- *seqno = &layout->os_area.msg_seqno_2;
- key = layout->vmpck2;
+ *seqno = &secrets->os_area.msg_seqno_2;
+ key = secrets->vmpck2;
break;
case 3:
- *seqno = &layout->os_area.msg_seqno_3;
- key = layout->vmpck3;
+ *seqno = &secrets->os_area.msg_seqno_3;
+ key = secrets->vmpck3;
break;
default:
break;
@@ -898,7 +898,7 @@ static void unregister_sev_tsm(void *data)
static int __init sev_guest_probe(struct platform_device *pdev)
{
struct sev_guest_platform_data *data;
- struct snp_secrets_page *layout;
+ struct snp_secrets_page *secrets;
struct device *dev = &pdev->dev;
struct snp_guest_dev *snp_dev;
struct miscdevice *misc;
@@ -916,7 +916,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
if (!mapping)
return -ENODEV;
- layout = (__force void *)mapping;
+ secrets = (__force void *)mapping;
ret = -ENOMEM;
snp_dev = devm_kzalloc(&pdev->dev, sizeof(struct snp_guest_dev), GFP_KERNEL);
@@ -924,7 +924,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
goto e_unmap;
ret = -EINVAL;
- snp_dev->vmpck = get_vmpck(vmpck_id, layout, &snp_dev->os_area_msg_seqno);
+ snp_dev->vmpck = get_vmpck(vmpck_id, secrets, &snp_dev->os_area_msg_seqno);
if (!snp_dev->vmpck) {
dev_err(dev, "invalid vmpck id %d\n", vmpck_id);
goto e_unmap;
@@ -938,7 +938,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, snp_dev);
snp_dev->dev = dev;
- snp_dev->layout = layout;
+ snp_dev->secrets = secrets;
/* Allocate the shared page used for the request and response message. */
snp_dev->request = alloc_shared_pages(dev, sizeof(struct snp_guest_msg));
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v4 11/15] configfs-tsm: Allow the privlevel_floor attribute to be updated
2024-04-24 15:58 ` [PATCH v4 11/15] configfs-tsm: Allow the privlevel_floor attribute to be updated Tom Lendacky
@ 2024-04-26 20:51 ` Dan Williams
0 siblings, 0 replies; 41+ messages in thread
From: Dan Williams @ 2024-04-26 20:51 UTC (permalink / raw)
To: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
Tom Lendacky wrote:
> With the introduction of an SVSM, Linux will be running at a non-zero
> VMPL. Any request for an attestation report at a higher privilege VMPL
> than what Linux is currently running will result in an error. Allow for
> the privlevel_floor attribute to be updated dynamically so that the
> attribute may be set dynamically. Set the privlevel_floor attribute to
> be the value of the vmpck_id being used.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
This addresses my comment on the last version.
Acked-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility
2024-04-24 15:58 ` [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility Tom Lendacky
@ 2024-04-26 21:48 ` Dan Williams
2024-04-29 13:26 ` Tom Lendacky
0 siblings, 1 reply; 41+ messages in thread
From: Dan Williams @ 2024-04-26 21:48 UTC (permalink / raw)
To: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra, Joel Becker, Christoph Hellwig
Tom Lendacky wrote:
> In order to support dynamic decisions as to whether an attribute should be
> created, add a callback that returns a bool to indicate whether the
> attribute should be displayed. If no callback is registered, the attribute
> is displayed by default.
>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> fs/configfs/dir.c | 20 ++++++++++++++++++++
> include/linux/configfs.h | 3 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 18677cd4e62f..463e66258507 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -580,6 +580,7 @@ static void detach_attrs(struct config_item * item)
> static int populate_attrs(struct config_item *item)
> {
> const struct config_item_type *t = item->ci_type;
> + struct configfs_group_operations *ops;
> struct configfs_attribute *attr;
> struct configfs_bin_attribute *bin_attr;
> int error = 0;
> @@ -587,14 +588,33 @@ static int populate_attrs(struct config_item *item)
>
> if (!t)
> return -EINVAL;
> +
> + ops = t->ct_group_ops;
> + if (!ops) {
> + struct config_group *g = item->ci_group;
> +
> + /*
> + * No item specific group operations, check if the item's group
> + * has group operations.
> + */
> + if (g && g->cg_item.ci_type)
> + ops = g->cg_item.ci_type->ct_group_ops;
Oh, I would not have expected to need to consider any alternate group
ops for attribute visibility beyond t->ct_group_ops. However in my RFC
example I made this mistake:
static struct configfs_group_operations tsm_report_group_ops = {
.make_item = tsm_report_make_item,
+ .is_visible = tsm_report_attr_visible,
+ .is_bin_visible = tsm_report_bin_attr_visible,
};
Which in retrospect is the wrong level, and I suspect only reachable if
you do the the above awkward indirection ("ops =
g->cg_item.ci_type->ct_group_ops"). Instead, I was expecting symmetry
with sysfs where the object that carries ->attrs also carries
->is_visible, so something like this:
+ static struct configfs_group_operations tsm_report_attr_group_ops = {
+ .is_visible = tsm_report_attr_visible,
+ .is_bin_visible = tsm_report_bin_attr_visible,
+ };
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,
+ .ct_group_ops = &tsm_report_attr_group_ops
};
EXPORT_SYMBOL_GPL(tsm_report_default_type);
...because is_visible() at the g->cg_item.ci_type->ct_group_ops level
would seem to mean parent directory visibility which is mismatched.
However as I stare at this a bit more it sinks in that configfs "group"
!= sysfs "group". So I am open to the suggestion that ci_item_ops is the
right place to house item attribute visibility callbacks, or even a new
"ci_attr_ops" expressly for this purpose. Either way my expectation is
that config_item_type can get to the visibilty callbacks for its
attributes without needing to traverse any other groups or items.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-24 15:58 ` [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM Tom Lendacky
@ 2024-04-26 21:58 ` Dan Williams
2024-04-29 13:35 ` Tom Lendacky
2024-05-01 5:18 ` Kuppuswamy Sathyanarayanan
2024-05-03 16:10 ` Kuppuswamy Sathyanarayanan
2 siblings, 1 reply; 41+ messages in thread
From: Dan Williams @ 2024-04-26 21:58 UTC (permalink / raw)
To: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
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 <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility
2024-04-26 21:48 ` Dan Williams
@ 2024-04-29 13:26 ` Tom Lendacky
0 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-04-29 13:26 UTC (permalink / raw)
To: Dan Williams, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Michael Roth,
Ashish Kalra, Joel Becker, Christoph Hellwig
On 4/26/24 16:48, Dan Williams wrote:
> Tom Lendacky wrote:
>> In order to support dynamic decisions as to whether an attribute should be
>> created, add a callback that returns a bool to indicate whether the
>> attribute should be displayed. If no callback is registered, the attribute
>> is displayed by default.
>>
>> Cc: Joel Becker <jlbec@evilplan.org>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> fs/configfs/dir.c | 20 ++++++++++++++++++++
>> include/linux/configfs.h | 3 +++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index 18677cd4e62f..463e66258507 100644
>> --- a/fs/configfs/dir.c
>> +++ b/fs/configfs/dir.c
>> @@ -580,6 +580,7 @@ static void detach_attrs(struct config_item * item)
>> static int populate_attrs(struct config_item *item)
>> {
>> const struct config_item_type *t = item->ci_type;
>> + struct configfs_group_operations *ops;
>> struct configfs_attribute *attr;
>> struct configfs_bin_attribute *bin_attr;
>> int error = 0;
>> @@ -587,14 +588,33 @@ static int populate_attrs(struct config_item *item)
>>
>> if (!t)
>> return -EINVAL;
>> +
>> + ops = t->ct_group_ops;
>> + if (!ops) {
>> + struct config_group *g = item->ci_group;
>> +
>> + /*
>> + * No item specific group operations, check if the item's group
>> + * has group operations.
>> + */
>> + if (g && g->cg_item.ci_type)
>> + ops = g->cg_item.ci_type->ct_group_ops;
>
> Oh, I would not have expected to need to consider any alternate group
> ops for attribute visibility beyond t->ct_group_ops. However in my RFC
> example I made this mistake:
Right, that's the question. Do you use the default group ops that are
created for TSM report or do you require the ops be part of the
attributes. It is definitely easy to do the latter.
>
> static struct configfs_group_operations tsm_report_group_ops = {
> .make_item = tsm_report_make_item,
> + .is_visible = tsm_report_attr_visible,
> + .is_bin_visible = tsm_report_bin_attr_visible,
> };
>
> Which in retrospect is the wrong level, and I suspect only reachable if
> you do the the above awkward indirection ("ops =
> g->cg_item.ci_type->ct_group_ops"). Instead, I was expecting symmetry
> with sysfs where the object that carries ->attrs also carries
> ->is_visible, so something like this:
>
> + static struct configfs_group_operations tsm_report_attr_group_ops = {
> + .is_visible = tsm_report_attr_visible,
> + .is_bin_visible = tsm_report_bin_attr_visible,
> + };
>
> 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,
> + .ct_group_ops = &tsm_report_attr_group_ops
> };
> EXPORT_SYMBOL_GPL(tsm_report_default_type);
>
> ..because is_visible() at the g->cg_item.ci_type->ct_group_ops level
> would seem to mean parent directory visibility which is mismatched.
>
> However as I stare at this a bit more it sinks in that configfs "group"
> != sysfs "group". So I am open to the suggestion that ci_item_ops is the
> right place to house item attribute visibility callbacks, or even a new
> "ci_attr_ops" expressly for this purpose. Either way my expectation is
> that config_item_type can get to the visibilty callbacks for its
> attributes without needing to traverse any other groups or items.
I'll update the patchset to do that.
Thanks,
Tom
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-26 21:58 ` Dan Williams
@ 2024-04-29 13:35 ` Tom Lendacky
2024-04-29 14:28 ` Tom Lendacky
0 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-29 13:35 UTC (permalink / raw)
To: Dan Williams, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Michael Roth,
Ashish Kalra
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 <dan.j.williams@intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-29 13:35 ` Tom Lendacky
@ 2024-04-29 14:28 ` Tom Lendacky
2024-05-01 19:28 ` Dan Williams
0 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-04-29 14:28 UTC (permalink / raw)
To: Dan Williams, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Michael Roth,
Ashish Kalra
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 <dan.j.williams@intel.com>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>> ---
>>> 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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-24 15:58 ` [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM Tom Lendacky
2024-04-26 21:58 ` Dan Williams
@ 2024-05-01 5:18 ` Kuppuswamy Sathyanarayanan
2024-05-01 20:15 ` Dan Williams
2024-05-03 16:10 ` Kuppuswamy Sathyanarayanan
2 siblings, 1 reply; 41+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-05-01 5:18 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Andy Lutomirski, Peter Zijlstra, Dan Williams, Michael Roth,
Ashish Kalra
On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <thomas.lendacky@amd.com> 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 <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-29 14:28 ` Tom Lendacky
@ 2024-05-01 19:28 ` Dan Williams
0 siblings, 0 replies; 41+ messages in thread
From: Dan Williams @ 2024-05-01 19:28 UTC (permalink / raw)
To: Tom Lendacky, Dan Williams, linux-kernel, x86, linux-coco,
svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Michael Roth,
Ashish Kalra
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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-05-01 5:18 ` Kuppuswamy Sathyanarayanan
@ 2024-05-01 20:15 ` Dan Williams
2024-05-02 3:40 ` Kuppuswamy Sathyanarayanan
0 siblings, 1 reply; 41+ messages in thread
From: Dan Williams @ 2024-05-01 20:15 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan, Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Andy Lutomirski, Peter Zijlstra, Dan Williams, Michael Roth,
Ashish Kalra
Kuppuswamy Sathyanarayanan wrote:
> On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <thomas.lendacky@amd.com> 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 <dan.j.williams@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
[..]
> > + */
> > +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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [svsm-devel] [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL
2024-04-24 15:58 ` [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL Tom Lendacky
@ 2024-05-01 23:57 ` Jacob Xu
2024-05-02 13:17 ` Tom Lendacky
0 siblings, 1 reply; 41+ messages in thread
From: Jacob Xu @ 2024-05-01 23:57 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Peter Zijlstra,
Michael Roth, Dave Hansen, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, H. Peter Anvin, Thomas Gleixner, Dan Williams
General question on VMPCKs: the secrets page defines them listed all
together one at a time, does that mean any guest at any VMPL can
observe all the VMPCKs? Or is SVSM running at VMPL0 supposed to clear
VMPCK0 before it hands off control to edk2?
On Wed, Apr 24, 2024 at 8:59 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> Currently, the sev-guest driver uses the vmpck-0 key by default. When an
> SVSM is present the kernel is running at a VMPL other than 0 and the
> vmpck-0 key is no longer available. If a specific vmpck key has not be
> requested by the user via the vmpck_id module parameter, choose the vmpck
> key based on the active VMPL level.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/include/asm/sev.h | 2 ++
> arch/x86/kernel/sev.c | 6 ++++++
> drivers/virt/coco/sev-guest/sev-guest.c | 10 +++++++---
> 3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index a7312b936d16..64fcadd6d602 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -307,6 +307,7 @@ u64 snp_get_unsupported_features(u64 status);
> u64 sev_get_status(void);
> void sev_show_status(void);
> void snp_remap_svsm_ca(void);
> +int snp_get_vmpl(void);
> #else
> static inline void sev_es_ist_enter(struct pt_regs *regs) { }
> static inline void sev_es_ist_exit(void) { }
> @@ -337,6 +338,7 @@ static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> static inline u64 sev_get_status(void) { return 0; }
> static inline void sev_show_status(void) { }
> static inline void snp_remap_svsm_ca(void) { }
> +static inline int snp_get_vmpl(void) { return 0; }
> #endif
>
> #ifdef CONFIG_KVM_AMD_SEV
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 8edf7362136b..75f11da867a3 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -2454,6 +2454,12 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn
> }
> EXPORT_SYMBOL_GPL(snp_issue_guest_request);
>
> +int snp_get_vmpl(void)
> +{
> + return vmpl;
> +}
> +EXPORT_SYMBOL_GPL(snp_get_vmpl);
> +
> static struct platform_device sev_guest_device = {
> .name = "sev-guest",
> .id = -1,
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 04a7bd1e4314..e7dd7df86427 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -2,7 +2,7 @@
> /*
> * AMD Secure Encrypted Virtualization (SEV) guest driver interface
> *
> - * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + * Copyright (C) 2021-2024 Advanced Micro Devices, Inc.
> *
> * Author: Brijesh Singh <brijesh.singh@amd.com>
> */
> @@ -70,8 +70,8 @@ struct snp_guest_dev {
> u8 *vmpck;
> };
>
> -static u32 vmpck_id;
> -module_param(vmpck_id, uint, 0444);
> +static int vmpck_id = -1;
> +module_param(vmpck_id, int, 0444);
> MODULE_PARM_DESC(vmpck_id, "The VMPCK ID to use when communicating with the PSP.");
>
> /* Mutex to serialize the shared buffer access and command handling. */
> @@ -923,6 +923,10 @@ static int __init sev_guest_probe(struct platform_device *pdev)
> if (!snp_dev)
> goto e_unmap;
>
> + /* Adjust the default VMPCK key based on the executing VMPL level */
> + if (vmpck_id == -1)
> + vmpck_id = snp_get_vmpl();
> +
> ret = -EINVAL;
> snp_dev->vmpck = get_vmpck(vmpck_id, layout, &snp_dev->os_area_msg_seqno);
> if (!snp_dev->vmpck) {
> --
> 2.43.2
>
> --
> Svsm-devel mailing list
> Svsm-devel@coconut-svsm.dev
> https://mail.8bytes.org/cgi-bin/mailman/listinfo/svsm-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-05-01 20:15 ` Dan Williams
@ 2024-05-02 3:40 ` Kuppuswamy Sathyanarayanan
2024-05-02 17:29 ` Dan Williams
0 siblings, 1 reply; 41+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-05-02 3:40 UTC (permalink / raw)
To: Dan Williams
Cc: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Michael Roth,
Ashish Kalra
On Wed, May 1, 2024 at 1:15 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Kuppuswamy Sathyanarayanan wrote:
> > On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <thomas.lendacky@amd.com> 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 <dan.j.williams@intel.com>
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> [..]
> > > + */
> > > +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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page
2024-04-24 15:58 ` [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page Tom Lendacky
@ 2024-05-02 9:35 ` Borislav Petkov
2024-05-02 15:29 ` Tom Lendacky
0 siblings, 1 reply; 41+ messages in thread
From: Borislav Petkov @ 2024-05-02 9:35 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
On Wed, Apr 24, 2024 at 10:58:00AM -0500, Tom Lendacky wrote:
> During early boot phases, check for the presence of an SVSM when running
> as an SEV-SNP guest.
>
> An SVSM is present if not running at VMPL0 and 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 <thomas.lendacky@amd.com>
> ---
> .../arch/x86/amd-memory-encryption.rst | 22 ++++++
> arch/x86/boot/compressed/sev.c | 8 +++
> arch/x86/include/asm/sev-common.h | 4 ++
> arch/x86/include/asm/sev.h | 25 ++++++-
> arch/x86/kernel/sev-shared.c | 70 +++++++++++++++++++
> arch/x86/kernel/sev.c | 7 ++
> 6 files changed, 135 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/arch/x86/amd-memory-encryption.rst b/Documentation/arch/x86/amd-memory-encryption.rst
> index 414bc7402ae7..32737718d4a2 100644
> --- a/Documentation/arch/x86/amd-memory-encryption.rst
> +++ b/Documentation/arch/x86/amd-memory-encryption.rst
> @@ -130,4 +130,26 @@ SNP feature support.
>
> More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR
>
> +Secure VM Service Module (SVSM)
> +===============================
> +
> +SNP provides a feature called Virtual Machine Privilege Levels (VMPL). The most
> +privileged VMPL is 0 with numerically higher numbers having lesser privileges.
> +More details in AMD64 APM[1] Vol 2: 15.35.7 Virtual Machine Privilege Levels.
> +
> +The VMPL feature provides the ability to run software services at a more
> +privileged level than the guest OS is running at. This provides a secure
Too many "provides".
> +environment for services within the guest's SNP environment, while protecting
> +the service from hypervisor interference. An example of a secure service
> +would be a virtual TPM (vTPM). Additionally, certain operations require the
> +guest to be running at VMPL0 in order for them to be performed. For example,
> +the PVALIDATE instruction is required to be executed at VMPL0.
> +
> +When a guest is not running at VMPL0, it needs to communicate with the software
> +running at VMPL0 to perform privileged operations or to interact with secure
> +services. This software running at VMPL0 is known as a Secure VM Service Module
> +(SVSM). Discovery of an SVSM and the API used to communicate with it is
> +documented in Secure VM Service Module for SEV-SNP Guests[2].
This paragraph needs to go second, not third.
Somehow that text is missing "restraint" and is all over the place.
Lemme try to restructure it:
"SNP provides a feature called Virtual Machine Privilege Levels (VMPL) which
defines four privilege levels at which guest software can run. The most
privileged level is 0 and numerically higher numbers have lesser privileges.
More details in the AMD64 APM[1] Vol 2, section "15.35.7 Virtual Machine
Privilege Levels", docID: 24593.
When using that feature, different services can run at different protection
levels, apart from the guest OS but still within the secure SNP environment.
They can provide services to the guest, like a vTPM, for example.
When a guest is not running at VMPL0, it needs to communicate with the software
running at VMPL0 to perform privileged operations or to interact with secure
services. An example fur such a privileged operation is PVALIDATE which is
*required* to be executed at VMPL0.
In this scenario, the software running at VMPL0 is usually called a Secure VM
Service Module (SVSM). Discovery of an SVSM and the API used to communicate
with it is documented in "Secure VM Service Module for SEV-SNP Guests", docID:
58019."
How's that?
> +
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
> +[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
Yeah, about those links - they get stale pretty quickly. I think it suffices to
explain what the document is and what it is called so that one can find it by
searching the web. See what I did above.
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 0457a9d7e515..cb771b380a6b 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -12,6 +12,7 @@
> */
> #include "misc.h"
>
> +#include <linux/mm.h>
Please do not include a kernel-proper header into the decompresssor.
Those things are solved by exposing the shared *minimal* functionality
into
arch/x86/include/asm/shared/
There are examples there.
By the looks of it:
In file included from arch/x86/boot/compressed/sev.c:130:
arch/x86/boot/compressed/../../kernel/sev-shared.c: In function ‘setup_svsm_ca’:
arch/x86/boot/compressed/../../kernel/sev-shared.c:1332:14: warning: implicit declaration of function ‘PAGE_ALIGNED’; did you mean ‘IS_ALIGNED’? [-Wimplicit-function-declaration]
1332 | if (!PAGE_ALIGNED(caa))
| ^~~~~~~~~~~~
| IS_ALIGNED
it'll need PAGE_ALIGNED and IS_ALIGNED into an arch/x86/include/asm/shared/mm.h
header.
> #include <asm/bootparam.h>
> #include <asm/pgtable_types.h>
> #include <asm/sev.h>
...
> +static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
> +{
> + struct snp_secrets_page *secrets_page;
> + u64 caa;
> +
> + BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
> +
> + /*
> + * 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.
> + *
> + * Use __pa() since this routine is running identity mapped when called,
> + * both by the decompressor code and the early kernel code.
> + */
Let's not replicate that comment. Diff ontop:
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index cb771b380a6b..cde1890c8843 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -576,18 +576,7 @@ void sev_enable(struct boot_params *bp)
if (!(get_hv_features() & GHCB_HV_FT_SNP))
sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
- /*
- * Enforce running at VMPL0.
- *
- * 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.
- */
+ /* Enforce running at VMPL0 - see comment above rmpadjust(). */
if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1))
sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
}
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 350db22e66be..b168403c07be 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -204,6 +204,17 @@ static __always_inline void sev_es_nmi_complete(void)
extern int __init sev_es_efi_map_ghcbs(pgd_t *pgd);
extern void sev_enable(struct boot_params *bp);
+/*
+ * RMPADJUST modifies RMP permissions of a lesser-privileged
+ * (numerically higher) privilege level. If @attrs==0, it will attempt
+ * to clear the VMPL1 permission mask of @vaddr. 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.
+ */
static inline int rmpadjust(unsigned long vaddr, bool rmp_psize, unsigned long attrs)
{
int rc;
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 46ea4e5e118a..9ca54bcf0e99 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -1297,17 +1297,9 @@ static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
/*
- * 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.
- *
- * Use __pa() since this routine is running identity mapped when called,
- * both by the decompressor code and the early kernel code.
+ * See comment above rmpadjust() for details. Use __pa() since
+ * this routine is running identity mapped when called both by
+ * the decompressor code and the early kernel code.
*/
if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
return;
> + if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
> + 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 *)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);
0x15C 1 byte SVSM_GUEST_VMPL Indicates the VMPL at which the guest is executing.
Do I understand it correctly that this contains the VMPL of the guest and the
SVSM is running below it?
IOW, SVSM should be at VMPL0 and the guest should be a at a level determined by
that value and it cannot be 0.
Just making sure I'm reading it right.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [svsm-devel] [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL
2024-05-01 23:57 ` [svsm-devel] " Jacob Xu
@ 2024-05-02 13:17 ` Tom Lendacky
0 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-05-02 13:17 UTC (permalink / raw)
To: Jacob Xu
Cc: linux-kernel, x86, linux-coco, svsm-devel, Peter Zijlstra,
Michael Roth, Dave Hansen, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, H. Peter Anvin, Thomas Gleixner, Dan Williams
On 5/1/24 18:57, Jacob Xu wrote:
> General question on VMPCKs: the secrets page defines them listed all
> together one at a time, does that mean any guest at any VMPL can
> observe all the VMPCKs? Or is SVSM running at VMPL0 supposed to clear
> VMPCK0 before it hands off control to edk2?
The SVSM should clear VMPCK0 in the copy of the secrets page provided to
the guest BIOS/OS.
Thanks,
Tom
>
>
> On Wed, Apr 24, 2024 at 8:59 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> Currently, the sev-guest driver uses the vmpck-0 key by default. When an
>> SVSM is present the kernel is running at a VMPL other than 0 and the
>> vmpck-0 key is no longer available. If a specific vmpck key has not be
>> requested by the user via the vmpck_id module parameter, choose the vmpck
>> key based on the active VMPL level.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> arch/x86/include/asm/sev.h | 2 ++
>> arch/x86/kernel/sev.c | 6 ++++++
>> drivers/virt/coco/sev-guest/sev-guest.c | 10 +++++++---
>> 3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>> index a7312b936d16..64fcadd6d602 100644
>> --- a/arch/x86/include/asm/sev.h
>> +++ b/arch/x86/include/asm/sev.h
>> @@ -307,6 +307,7 @@ u64 snp_get_unsupported_features(u64 status);
>> u64 sev_get_status(void);
>> void sev_show_status(void);
>> void snp_remap_svsm_ca(void);
>> +int snp_get_vmpl(void);
>> #else
>> static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>> static inline void sev_es_ist_exit(void) { }
>> @@ -337,6 +338,7 @@ static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
>> static inline u64 sev_get_status(void) { return 0; }
>> static inline void sev_show_status(void) { }
>> static inline void snp_remap_svsm_ca(void) { }
>> +static inline int snp_get_vmpl(void) { return 0; }
>> #endif
>>
>> #ifdef CONFIG_KVM_AMD_SEV
>> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
>> index 8edf7362136b..75f11da867a3 100644
>> --- a/arch/x86/kernel/sev.c
>> +++ b/arch/x86/kernel/sev.c
>> @@ -2454,6 +2454,12 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn
>> }
>> EXPORT_SYMBOL_GPL(snp_issue_guest_request);
>>
>> +int snp_get_vmpl(void)
>> +{
>> + return vmpl;
>> +}
>> +EXPORT_SYMBOL_GPL(snp_get_vmpl);
>> +
>> static struct platform_device sev_guest_device = {
>> .name = "sev-guest",
>> .id = -1,
>> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
>> index 04a7bd1e4314..e7dd7df86427 100644
>> --- a/drivers/virt/coco/sev-guest/sev-guest.c
>> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
>> @@ -2,7 +2,7 @@
>> /*
>> * AMD Secure Encrypted Virtualization (SEV) guest driver interface
>> *
>> - * Copyright (C) 2021 Advanced Micro Devices, Inc.
>> + * Copyright (C) 2021-2024 Advanced Micro Devices, Inc.
>> *
>> * Author: Brijesh Singh <brijesh.singh@amd.com>
>> */
>> @@ -70,8 +70,8 @@ struct snp_guest_dev {
>> u8 *vmpck;
>> };
>>
>> -static u32 vmpck_id;
>> -module_param(vmpck_id, uint, 0444);
>> +static int vmpck_id = -1;
>> +module_param(vmpck_id, int, 0444);
>> MODULE_PARM_DESC(vmpck_id, "The VMPCK ID to use when communicating with the PSP.");
>>
>> /* Mutex to serialize the shared buffer access and command handling. */
>> @@ -923,6 +923,10 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>> if (!snp_dev)
>> goto e_unmap;
>>
>> + /* Adjust the default VMPCK key based on the executing VMPL level */
>> + if (vmpck_id == -1)
>> + vmpck_id = snp_get_vmpl();
>> +
>> ret = -EINVAL;
>> snp_dev->vmpck = get_vmpck(vmpck_id, layout, &snp_dev->os_area_msg_seqno);
>> if (!snp_dev->vmpck) {
>> --
>> 2.43.2
>>
>> --
>> Svsm-devel mailing list
>> Svsm-devel@coconut-svsm.dev
>> https://mail.8bytes.org/cgi-bin/mailman/listinfo/svsm-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page
2024-05-02 9:35 ` Borislav Petkov
@ 2024-05-02 15:29 ` Tom Lendacky
0 siblings, 0 replies; 41+ messages in thread
From: Tom Lendacky @ 2024-05-02 15:29 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
On 5/2/24 04:35, Borislav Petkov wrote:
> On Wed, Apr 24, 2024 at 10:58:00AM -0500, Tom Lendacky wrote:
>> During early boot phases, check for the presence of an SVSM when running
>> as an SEV-SNP guest.
>>
>> An SVSM is present if not running at VMPL0 and 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 <thomas.lendacky@amd.com>
>> ---
>> .../arch/x86/amd-memory-encryption.rst | 22 ++++++
>> arch/x86/boot/compressed/sev.c | 8 +++
>> arch/x86/include/asm/sev-common.h | 4 ++
>> arch/x86/include/asm/sev.h | 25 ++++++-
>> arch/x86/kernel/sev-shared.c | 70 +++++++++++++++++++
>> arch/x86/kernel/sev.c | 7 ++
>> 6 files changed, 135 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arch/x86/amd-memory-encryption.rst b/Documentation/arch/x86/amd-memory-encryption.rst
>> index 414bc7402ae7..32737718d4a2 100644
>> --- a/Documentation/arch/x86/amd-memory-encryption.rst
>> +++ b/Documentation/arch/x86/amd-memory-encryption.rst
>> @@ -130,4 +130,26 @@ SNP feature support.
>>
>> More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR
>>
>> +Secure VM Service Module (SVSM)
>> +===============================
>> +
>> +SNP provides a feature called Virtual Machine Privilege Levels (VMPL). The most
>> +privileged VMPL is 0 with numerically higher numbers having lesser privileges.
>> +More details in AMD64 APM[1] Vol 2: 15.35.7 Virtual Machine Privilege Levels.
>> +
>> +The VMPL feature provides the ability to run software services at a more
>> +privileged level than the guest OS is running at. This provides a secure
>
> Too many "provides".
>
>> +environment for services within the guest's SNP environment, while protecting
>> +the service from hypervisor interference. An example of a secure service
>> +would be a virtual TPM (vTPM). Additionally, certain operations require the
>> +guest to be running at VMPL0 in order for them to be performed. For example,
>> +the PVALIDATE instruction is required to be executed at VMPL0.
>> +
>> +When a guest is not running at VMPL0, it needs to communicate with the software
>> +running at VMPL0 to perform privileged operations or to interact with secure
>> +services. This software running at VMPL0 is known as a Secure VM Service Module
>> +(SVSM). Discovery of an SVSM and the API used to communicate with it is
>> +documented in Secure VM Service Module for SEV-SNP Guests[2].
>
> This paragraph needs to go second, not third.
>
> Somehow that text is missing "restraint" and is all over the place.
> Lemme try to restructure it:
>
> "SNP provides a feature called Virtual Machine Privilege Levels (VMPL) which
> defines four privilege levels at which guest software can run. The most
> privileged level is 0 and numerically higher numbers have lesser privileges.
> More details in the AMD64 APM[1] Vol 2, section "15.35.7 Virtual Machine
> Privilege Levels", docID: 24593.
>
> When using that feature, different services can run at different protection
> levels, apart from the guest OS but still within the secure SNP environment.
> They can provide services to the guest, like a vTPM, for example.
>
> When a guest is not running at VMPL0, it needs to communicate with the software
> running at VMPL0 to perform privileged operations or to interact with secure
> services. An example fur such a privileged operation is PVALIDATE which is
> *required* to be executed at VMPL0.
>
> In this scenario, the software running at VMPL0 is usually called a Secure VM
> Service Module (SVSM). Discovery of an SVSM and the API used to communicate
> with it is documented in "Secure VM Service Module for SEV-SNP Guests", docID:
> 58019."
>
> How's that?
Works for me.
>
>> +
>> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
>> +[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
>
> Yeah, about those links - they get stale pretty quickly. I think it suffices to
> explain what the document is and what it is called so that one can find it by
> searching the web. See what I did above.
>
>> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
>> index 0457a9d7e515..cb771b380a6b 100644
>> --- a/arch/x86/boot/compressed/sev.c
>> +++ b/arch/x86/boot/compressed/sev.c
>> @@ -12,6 +12,7 @@
>> */
>> #include "misc.h"
>>
>> +#include <linux/mm.h>
>
> Please do not include a kernel-proper header into the decompresssor.
> Those things are solved by exposing the shared *minimal* functionality
> into
Right, should've known that.
>
> arch/x86/include/asm/shared/
>
> There are examples there.
>
> By the looks of it:
>
> In file included from arch/x86/boot/compressed/sev.c:130:
> arch/x86/boot/compressed/../../kernel/sev-shared.c: In function ‘setup_svsm_ca’:
> arch/x86/boot/compressed/../../kernel/sev-shared.c:1332:14: warning: implicit declaration of function ‘PAGE_ALIGNED’; did you mean ‘IS_ALIGNED’? [-Wimplicit-function-declaration]
> 1332 | if (!PAGE_ALIGNED(caa))
> | ^~~~~~~~~~~~
> | IS_ALIGNED
>
> it'll need PAGE_ALIGNED and IS_ALIGNED into an arch/x86/include/asm/shared/mm.h
> header.
PAGE_ALIGNED and IS_ALIGNED are from two separate header files (mm.h and
align.h) which seems like a lot of extra changes for just one check.
Any objection to either adding this define to sev-shared.c on the "else"
patch of the "#ifndef __BOOT_COMPRESSED" check:
#define PAGE_ALIGNED(x) IS_ALIGNED((x), PAGE_SIZE)
or just changing the above check to:
if (!IS_ALIGNED(caa, PAGE_SIZE))
>
>> #include <asm/bootparam.h>
>> #include <asm/pgtable_types.h>
>> #include <asm/sev.h>
>
> ..
>
>> +static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
>> +{
>> + struct snp_secrets_page *secrets_page;
>> + u64 caa;
>> +
>> + BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
>> +
>> + /*
>> + * 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.
>> + *
>> + * Use __pa() since this routine is running identity mapped when called,
>> + * both by the decompressor code and the early kernel code.
>> + */
>
> Let's not replicate that comment. Diff ontop:
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index cb771b380a6b..cde1890c8843 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -576,18 +576,7 @@ void sev_enable(struct boot_params *bp)
> if (!(get_hv_features() & GHCB_HV_FT_SNP))
> sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
>
> - /*
> - * Enforce running at VMPL0.
> - *
> - * 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.
> - */
> + /* Enforce running at VMPL0 - see comment above rmpadjust(). */
Not sure I agree. I'd prefer to keep the comment here because it is
specific to this rmpadjust() call. See below.
> if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1))
> sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
> }
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 350db22e66be..b168403c07be 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -204,6 +204,17 @@ static __always_inline void sev_es_nmi_complete(void)
> extern int __init sev_es_efi_map_ghcbs(pgd_t *pgd);
> extern void sev_enable(struct boot_params *bp);
>
> +/*
> + * RMPADJUST modifies RMP permissions of a lesser-privileged
> + * (numerically higher) privilege level. If @attrs==0, it will attempt
> + * to clear the VMPL1 permission mask of @vaddr. 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.
If you want to put a comment here, then it needs to be more generic. The
attrs value would be 1 if VMPL0 was attempting to clear VMPL1
permissions. Also, you could be running at VMPL2 and successfully clear
or set VMPL3 permissions. So this comment doesn't really flow with a
generic RMPADJUST function.
/*
* RMPAJDUST modifies the RMP permissions of a lesser-privileged
* (numerically higher) VMPL. The @attrs option contains the VMPL
* level to be modified for @vaddr. The operation will succeed only
* if the guest is running at a higher-privileged (numerically lower)
* VMPL.
*/
> + */
> static inline int rmpadjust(unsigned long vaddr, bool rmp_psize, unsigned long attrs)
> {
> int rc;
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 46ea4e5e118a..9ca54bcf0e99 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -1297,17 +1297,9 @@ static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
> BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
>
> /*
> - * 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.
> - *
> - * Use __pa() since this routine is running identity mapped when called,
> - * both by the decompressor code and the early kernel code.
> + * See comment above rmpadjust() for details. Use __pa() since
> + * this routine is running identity mapped when called both by
> + * the decompressor code and the early kernel code.
> */
> if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
> return;
>
>> + if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
>> + 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 *)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);
>
> 0x15C 1 byte SVSM_GUEST_VMPL Indicates the VMPL at which the guest is executing.
>
> Do I understand it correctly that this contains the VMPL of the guest and the
> SVSM is running below it?
Right, the SVSM is supposed to place the VMPL level that it starts the
guest at in this location.
>
> IOW, SVSM should be at VMPL0 and the guest should be a at a level determined by
> that value and it cannot be 0.
Right. Not sure about the "cannot", more like "must not." The
specification states that the guest should run at a VMPL other than 0.
If an SVSM starts the guest at VMPL0, then the SVSM would not be
protected from guest.
Thanks,
Tom
>
> Just making sure I'm reading it right.
>
> Thx.
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-05-02 3:40 ` Kuppuswamy Sathyanarayanan
@ 2024-05-02 17:29 ` Dan Williams
0 siblings, 0 replies; 41+ messages in thread
From: Dan Williams @ 2024-05-02 17:29 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan, Dan Williams
Cc: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Michael Roth,
Ashish Kalra
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.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas
2024-04-24 15:58 ` [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas Tom Lendacky
@ 2024-05-03 10:34 ` Borislav Petkov
2024-05-06 10:09 ` Borislav Petkov
0 siblings, 1 reply; 41+ messages in thread
From: Borislav Petkov @ 2024-05-03 10:34 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
This one would need multiple review mails.
Lemme make this part 1.
On Wed, Apr 24, 2024 at 10:58:01AM -0500, Tom Lendacky wrote:
> arch/x86/include/asm/sev-common.h | 13 ++
> arch/x86/include/asm/sev.h | 32 +++++
> arch/x86/include/uapi/asm/svm.h | 1 +
> arch/x86/kernel/sev-shared.c | 94 +++++++++++++-
> arch/x86/kernel/sev.c | 207 +++++++++++++++++++++++++-----
Ok, now would be as good time as any to start moving the SEV guest bits
to where we want them to live:
arch/x86/coco/sev/
so pls add the new SVSM guest support bits there:
arch/x86/coco/sev/svsm.c
arch/x86/coco/sev/svsm-shared.c
I guess.
And things which touch sev.c and sev-shared.c will have to add patches
which move bits to the new location.
> arch/x86/mm/mem_encrypt_amd.c | 8 +-
> 6 files changed, 320 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index 1225744a069b..4cc716660d4b 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -96,6 +96,19 @@ enum psc_op {
> /* GHCBData[63:32] */ \
> (((u64)(val) & GENMASK_ULL(63, 32)) >> 32)
>
> +/* GHCB Run at VMPL Request/Response */
Run?
> +#define GHCB_MSR_VMPL_REQ 0x016
> +#define GHCB_MSR_VMPL_REQ_LEVEL(v) \
> + /* GHCBData[39:32] */ \
> + (((u64)(v) & GENMASK_ULL(7, 0) << 32) | \
> + /* GHCBDdata[11:0] */ \
> + GHCB_MSR_VMPL_REQ)
> +
> +#define GHCB_MSR_VMPL_RESP 0x017
> +#define GHCB_MSR_VMPL_RESP_VAL(v) \
> + /* GHCBData[63:32] */ \
> + (((u64)(v) & GENMASK_ULL(63, 32)) >> 32)
> +
> /* GHCB Hypervisor Feature Request/Response */
> #define GHCB_MSR_HV_FT_REQ 0x080
> #define GHCB_MSR_HV_FT_RESP 0x081
...
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 46ea4e5e118a..6f57eb804e70 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -18,9 +18,11 @@
> #define sev_printk_rtl(fmt, ...) printk_ratelimited(fmt, ##__VA_ARGS__)
> #else
> #undef WARN
> -#define WARN(condition, format...) (!!(condition))
> +#define WARN(condition, format...) (!!(condition))
> #define sev_printk(fmt, ...)
> #define sev_printk_rtl(fmt, ...)
> +#undef vc_forward_exception
> +#define vc_forward_exception(c) panic("SNP: Hypervisor requested exception\n")
> #endif
>
> /*
> @@ -244,6 +246,96 @@ static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt
> return ES_VMM_ERROR;
> }
>
> +static __always_inline void issue_svsm_call(struct svsm_call *call, u8 *pending)
svsm_issue_call()
I guess.
> +{
> + /*
> + * Issue the VMGEXIT to run the SVSM:
... to call the SVSM:" I guess.
> + * - Load the SVSM register state (RAX, RCX, RDX, R8 and R9)
> + * - Set the CA call pending field to 1
> + * - Issue VMGEXIT
> + * - Save the SVSM return register state (RAX, RCX, RDX, R8 and R9)
> + * - Perform atomic exchange of the CA call pending field
> + */
That goes above the function name.
> + asm volatile("mov %9, %%r8\n\t"
> + "mov %10, %%r9\n\t"
> + "movb $1, %11\n\t"
> + "rep; vmmcall\n\t"
> + "mov %%r8, %3\n\t"
> + "mov %%r9, %4\n\t"
> + "xchgb %5, %11\n\t"
> + : "=a" (call->rax_out), "=c" (call->rcx_out), "=d" (call->rdx_out),
> + "=m" (call->r8_out), "=m" (call->r9_out),
> + "+r" (*pending)
> + : "a" (call->rax), "c" (call->rcx), "d" (call->rdx),
> + "r" (call->r8), "r" (call->r9),
> + "m" (call->caa->call_pending)
> + : "r8", "r9", "memory");
> +}
Btw, where are we documenting this calling convention?
Anyway, I think you can do it this way (pasting the whole thing for
easier review):
static __always_inline void issue_svsm_call(struct svsm_call *call, u8 *pending)
{
register unsigned long r8 asm("r8") = call->r8;
register unsigned long r9 asm("r9") = call->r9;
call->caa->call_pending = 1;
/*
* Issue the VMGEXIT to run the SVSM:
* - Load the SVSM register state (RAX, RCX, RDX, R8 and R9)
* - Set the CA call pending field to 1
* - Issue VMGEXIT
* - Save the SVSM return register state (RAX, RCX, RDX, R8 and R9)
* - Perform atomic exchange of the CA call pending field
*/
asm volatile("rep; vmmcall\n\t"
"xchgb %[pending], %[call_pending]"
: "=a" (call->rax_out),
"=c" (call->rcx_out),
"=d" (call->rdx_out),
[pending] "+r" (*pending),
"+r" (r8),
"+r" (r9)
: "a" (call->rax),
"c" (call->rcx),
"d" (call->rdx),
[call_pending] "m" (call->caa->call_pending)
: "memory");
call->r8_out = r8;
call->r9_out = r9;
}
I *think* the asm is the same but it needs more looking in detail. It
probably could be simplified even more.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [svsm-devel] [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present
2024-04-24 15:58 ` [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present Tom Lendacky
@ 2024-05-03 11:37 ` Jörg Rödel
2024-05-03 16:04 ` Borislav Petkov
0 siblings, 1 reply; 41+ messages in thread
From: Jörg Rödel @ 2024-05-03 11:37 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Peter Zijlstra,
Michael Roth, Dave Hansen, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, H. Peter Anvin, Thomas Gleixner, Dan Williams
On Wed, Apr 24, 2024 at 10:58:11AM -0500, Tom Lendacky wrote:
> +static void __init report_vmpl_level(void)
> +{
> + if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> + return;
> +
> + pr_info("SNP running at VMPL%u.\n", vmpl);
Nit: Can this be formated more like "SNP running at VMPL-%u"? That makes
it easier to parse for me when looking into dmesg :)
Regards,
Joerg
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [svsm-devel] [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
` (14 preceding siblings ...)
2024-04-24 15:58 ` [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present Tom Lendacky
@ 2024-05-03 11:38 ` Jörg Rödel
15 siblings, 0 replies; 41+ messages in thread
From: Jörg Rödel @ 2024-05-03 11:38 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Peter Zijlstra,
Michael Roth, Dave Hansen, Christoph Hellwig, Ingo Molnar,
Borislav Petkov, Joel Becker, Andy Lutomirski, H. Peter Anvin,
Thomas Gleixner, Dan Williams
On Wed, Apr 24, 2024 at 10:57:56AM -0500, Tom Lendacky wrote:
> Tom Lendacky (15):
> x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page
> x86/sev: Rename snp_init() in the boot/compressed/sev.c file
> x86/sev: Make the VMPL0 checking more straight forward
> x86/sev: Check for the presence of an SVSM in the SNP Secrets page
> x86/sev: Use kernel provided SVSM Calling Areas
> x86/sev: Perform PVALIDATE using the SVSM when not at VMPL0
> x86/sev: Use the SVSM to create a vCPU when not in VMPL0
> x86/sev: Provide SVSM discovery support
> x86/sev: Provide guest VMPL level to userspace
> virt: sev-guest: Choose the VMPCK key based on executing VMPL
> configfs-tsm: Allow the privlevel_floor attribute to be updated
> fs/configfs: Add a callback to determine attribute visibility
> x86/sev: Take advantage of configfs visibility support in TSM
> x86/sev: Extend the config-fs attestation support for an SVSM
> x86/sev: Allow non-VMPL0 execution when an SVSM is present
I tested these on latest COCONUT-SVSM upstream and found no issues.
Tested-by: Joerg Roedel <jroedel@suse.de>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [svsm-devel] [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present
2024-05-03 11:37 ` [svsm-devel] " Jörg Rödel
@ 2024-05-03 16:04 ` Borislav Petkov
2024-05-06 7:43 ` Jörg Rödel
0 siblings, 1 reply; 41+ messages in thread
From: Borislav Petkov @ 2024-05-03 16:04 UTC (permalink / raw)
To: Jörg Rödel
Cc: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel,
Peter Zijlstra, Michael Roth, Dave Hansen, Ingo Molnar,
Andy Lutomirski, H. Peter Anvin, Thomas Gleixner, Dan Williams
On Fri, May 03, 2024 at 01:37:20PM +0200, Jörg Rödel wrote:
> Nit: Can this be formated more like "SNP running at VMPL-%u"? That makes
> it easier to parse for me when looking into dmesg :)
Hmm, except that all documentation is without a "-"... The APM talks
about VMPL%d everywhere...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM
2024-04-24 15:58 ` [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM Tom Lendacky
2024-04-26 21:58 ` Dan Williams
2024-05-01 5:18 ` Kuppuswamy Sathyanarayanan
@ 2024-05-03 16:10 ` Kuppuswamy Sathyanarayanan
2 siblings, 0 replies; 41+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-05-03 16:10 UTC (permalink / raw)
To: Tom Lendacky, linux-kernel, x86, linux-coco, svsm-devel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Dan Williams,
Michael Roth, Ashish Kalra
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 <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [svsm-devel] [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present
2024-05-03 16:04 ` Borislav Petkov
@ 2024-05-06 7:43 ` Jörg Rödel
0 siblings, 0 replies; 41+ messages in thread
From: Jörg Rödel @ 2024-05-06 7:43 UTC (permalink / raw)
To: Borislav Petkov
Cc: Jörg Rödel, Dave Hansen, Peter Zijlstra, Michael Roth,
x86, linux-kernel, Ingo Molnar, linux-coco, Andy Lutomirski,
H. Peter Anvin, Thomas Gleixner, Dan Williams, svsm-devel
On Fri, May 03, 2024 at 06:04:19PM +0200, Borislav Petkov wrote:
> On Fri, May 03, 2024 at 01:37:20PM +0200, Jörg Rödel wrote:
> > Nit: Can this be formated more like "SNP running at VMPL-%u"? That makes
> > it easier to parse for me when looking into dmesg :)
>
> Hmm, except that all documentation is without a "-"... The APM talks
> about VMPL%d everywhere...
No strict opinion, I just wanted to point out that there are more
readable ways of printing the VMPL level than what is used in the APM.
Regards,
--
Jörg Rödel
jroedel@suse.de
SUSE Software Solutions Germany GmbH
Frankenstraße 146
90461 Nürnberg
Germany
https://www.suse.com/
Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas
2024-05-03 10:34 ` Borislav Petkov
@ 2024-05-06 10:09 ` Borislav Petkov
2024-05-06 13:14 ` Tom Lendacky
0 siblings, 1 reply; 41+ messages in thread
From: Borislav Petkov @ 2024-05-06 10:09 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
Ok,
I think this is very readable and clear what's going on:
static __always_inline void issue_svsm_call(struct svsm_call *call, u8 *pending)
{
register unsigned long rax asm("rax") = call->rax;
register unsigned long rcx asm("rcx") = call->rcx;
register unsigned long rdx asm("rdx") = call->rdx;
register unsigned long r8 asm("r8") = call->r8;
register unsigned long r9 asm("r9") = call->r9;
call->caa->call_pending = 1;
asm volatile("rep; vmmcall\n\t"
: "+r" (rax), "+r" (rcx), "+r" (rdx), "+r" (r8), "+r" (r9));
xchg(pending, 1);
call->rax_out = rax;
call->rcx_out = rcx;
call->rdx_out = rdx;
call->r8_out = r8;
call->r9_out = r9;
}
and the asm looks ok but the devil's in the detail.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas
2024-05-06 10:09 ` Borislav Petkov
@ 2024-05-06 13:14 ` Tom Lendacky
2024-05-06 14:14 ` Borislav Petkov
0 siblings, 1 reply; 41+ messages in thread
From: Tom Lendacky @ 2024-05-06 13:14 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
On 5/6/24 05:09, Borislav Petkov wrote:
> Ok,
>
> I think this is very readable and clear what's going on:
I'll test it out.
>
> static __always_inline void issue_svsm_call(struct svsm_call *call, u8 *pending)
> {
> register unsigned long rax asm("rax") = call->rax;
> register unsigned long rcx asm("rcx") = call->rcx;
> register unsigned long rdx asm("rdx") = call->rdx;
> register unsigned long r8 asm("r8") = call->r8;
> register unsigned long r9 asm("r9") = call->r9;
>
> call->caa->call_pending = 1;
>
> asm volatile("rep; vmmcall\n\t"
> : "+r" (rax), "+r" (rcx), "+r" (rdx), "+r" (r8), "+r" (r9));
>
> xchg(pending, 1);
This isn't quite right. The xchg has to occur between pending and
call->caa->call_pending.
Thanks,
Tom
>
> call->rax_out = rax;
> call->rcx_out = rcx;
> call->rdx_out = rdx;
> call->r8_out = r8;
> call->r9_out = r9;
> }
>
> and the asm looks ok but the devil's in the detail.
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas
2024-05-06 13:14 ` Tom Lendacky
@ 2024-05-06 14:14 ` Borislav Petkov
0 siblings, 0 replies; 41+ messages in thread
From: Borislav Petkov @ 2024-05-06 14:14 UTC (permalink / raw)
To: Tom Lendacky
Cc: linux-kernel, x86, linux-coco, svsm-devel, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Andy Lutomirski,
Peter Zijlstra, Dan Williams, Michael Roth, Ashish Kalra
On Mon, May 06, 2024 at 08:14:34AM -0500, Tom Lendacky wrote:
> This isn't quite right. The xchg has to occur between pending and
> call->caa->call_pending.
Right, you can fix that up in your next revision. :-)
I only wanted to show the idea of how to make this a lot more
readable.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2024-05-06 14:14 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-24 15:57 [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 01/15] x86/sev: Shorten snp_secrets_page_layout to snp_secrets_page Tom Lendacky
2024-04-25 13:30 ` Borislav Petkov
2024-04-24 15:57 ` [PATCH v4 02/15] x86/sev: Rename snp_init() in the boot/compressed/sev.c file Tom Lendacky
2024-04-24 15:57 ` [PATCH v4 03/15] x86/sev: Make the VMPL0 checking more straight forward Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 04/15] x86/sev: Check for the presence of an SVSM in the SNP Secrets page Tom Lendacky
2024-05-02 9:35 ` Borislav Petkov
2024-05-02 15:29 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 05/15] x86/sev: Use kernel provided SVSM Calling Areas Tom Lendacky
2024-05-03 10:34 ` Borislav Petkov
2024-05-06 10:09 ` Borislav Petkov
2024-05-06 13:14 ` Tom Lendacky
2024-05-06 14:14 ` Borislav Petkov
2024-04-24 15:58 ` [PATCH v4 06/15] x86/sev: Perform PVALIDATE using the SVSM when not at VMPL0 Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 07/15] x86/sev: Use the SVSM to create a vCPU when not in VMPL0 Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 08/15] x86/sev: Provide SVSM discovery support Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 09/15] x86/sev: Provide guest VMPL level to userspace Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 10/15] virt: sev-guest: Choose the VMPCK key based on executing VMPL Tom Lendacky
2024-05-01 23:57 ` [svsm-devel] " Jacob Xu
2024-05-02 13:17 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 11/15] configfs-tsm: Allow the privlevel_floor attribute to be updated Tom Lendacky
2024-04-26 20:51 ` Dan Williams
2024-04-24 15:58 ` [PATCH v4 12/15] fs/configfs: Add a callback to determine attribute visibility Tom Lendacky
2024-04-26 21:48 ` Dan Williams
2024-04-29 13:26 ` Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM Tom Lendacky
2024-04-26 21:58 ` Dan Williams
2024-04-29 13:35 ` Tom Lendacky
2024-04-29 14:28 ` Tom Lendacky
2024-05-01 19:28 ` Dan Williams
2024-05-01 5:18 ` Kuppuswamy Sathyanarayanan
2024-05-01 20:15 ` Dan Williams
2024-05-02 3:40 ` Kuppuswamy Sathyanarayanan
2024-05-02 17:29 ` Dan Williams
2024-05-03 16:10 ` Kuppuswamy Sathyanarayanan
2024-04-24 15:58 ` [PATCH v4 14/15] x86/sev: Extend the config-fs attestation support for an SVSM Tom Lendacky
2024-04-24 15:58 ` [PATCH v4 15/15] x86/sev: Allow non-VMPL0 execution when an SVSM is present Tom Lendacky
2024-05-03 11:37 ` [svsm-devel] " Jörg Rödel
2024-05-03 16:04 ` Borislav Petkov
2024-05-06 7:43 ` Jörg Rödel
2024-05-03 11:38 ` [svsm-devel] [PATCH v4 00/15] Provide SEV-SNP support for running under an SVSM Jörg Rödel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.