[PATCH v3 09/14] virt: sev-guest: Choose the VMPCK key based on executing VMPL

Tom Lendacky posted 14 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH v3 09/14] virt: sev-guest: Choose the VMPCK key based on executing VMPL
Posted by Tom Lendacky 1 year, 10 months ago
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. So 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 d7be613b7372..066eb0ba3d63 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -308,6 +308,7 @@ u64 sev_get_status(void);
 void kdump_sev_callback(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) { }
@@ -339,6 +340,7 @@ static inline u64 sev_get_status(void) { return 0; }
 static inline void kdump_sev_callback(void) { }
 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 b027b00e315d..7e2b9934a95d 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2464,6 +2464,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 87f241825bc3..1ff897913bf4 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
Re: [PATCH v3 09/14] virt: sev-guest: Choose the VMPCK key based on executing VMPL
Posted by Dan Williams 1 year, 9 months ago
Hey, Tom, came looking to review the tsm_report changes and noticed
this...

Tom Lendacky 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. So choose the vmpck key based on the
> active VMPL level.

The module parameter is not mentioned in the changelog. Is it not
sufficient to always use snp_get_vmpl(), and if not should there be some
documentation about when to specify vmpck_id?

Do users know that "vmpl" and "vmpck_id" are interchangeable?
Re: [PATCH v3 09/14] virt: sev-guest: Choose the VMPCK key based on executing VMPL
Posted by Tom Lendacky 1 year, 9 months ago
On 4/15/24 23:54, Dan Williams wrote:
> Hey, Tom, came looking to review the tsm_report changes and noticed
> this...
> 
> Tom Lendacky 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. So choose the vmpck key based on the
>> active VMPL level.
> 
> The module parameter is not mentioned in the changelog. Is it not
> sufficient to always use snp_get_vmpl(), and if not should there be some
> documentation about when to specify vmpck_id?

It is possible to encounter an issue that causes the vmpck key to be 
cleared. In that situation, the guest is allowed to use a vmpck key 
associated with a lower VMPL. For that reason, the module parameter was 
added to the driver when it was initially created.

I can update the changelog to mention this.

Note that as long as the vmpck key exists, a guest running at VMPL2 
could request a VMPL0 report using the vmpck0 key, that is why it is 
important that the SVSM clear to zero any vmpck key that represents a 
higher privilege. For example, if the SVSM (running at VMPL0) launches 
the guest at VMPL2, it should zero out the vmpck0 and vmpck1 keys in the 
SNP Secrets Page supplied to the guest.

> 
> Do users know that "vmpl" and "vmpck_id" are interchangeable?

Yes, they should be aware of the relation of VMPL to VMPCK from the SNP 
specification.

Thanks,
Tom
Re: [PATCH v3 09/14] virt: sev-guest: Choose the VMPCK key based on executing VMPL
Posted by Dan Williams 1 year, 9 months ago
Tom Lendacky wrote:
> On 4/15/24 23:54, Dan Williams wrote:
> > Hey, Tom, came looking to review the tsm_report changes and noticed
> > this...
> > 
> > Tom Lendacky 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. So choose the vmpck key based on the
> >> active VMPL level.
> > 
> > The module parameter is not mentioned in the changelog. Is it not
> > sufficient to always use snp_get_vmpl(), and if not should there be some
> > documentation about when to specify vmpck_id?
> 
> It is possible to encounter an issue that causes the vmpck key to be 
> cleared. In that situation, the guest is allowed to use a vmpck key 
> associated with a lower VMPL. For that reason, the module parameter was 
> added to the driver when it was initially created.

Oh, sorry, misread that the module parameter was not new.

> I can update the changelog to mention this.

I guess it is too late now, but a proper sysfs attribute rather than a
module parameter would let you do sanity checking on the writes and
allow a natural place to document behavior in
Documentation/ABI/testing/sysfs-devices-sev-guest.