Re: [PATCH v2 08/11] x86/sev: Add Secure TSC support for SNP guests
From: Tom Lendacky
Date: Mon Apr 03 2023 - 17:43:18 EST
On 3/26/23 09:46, Nikunj A Dadhania wrote:
Add support for Secure TSC in SNP enabled guests. Secure TSC
allows guest to securely use RDTSC/RDTSCP instructions as the
parameters being used cannot be changed by hypervisor once the
guest is launched.
During the boot-up of the secondary cpus, SecureTSC enabled
guests need to query TSC info from Security processor (PSP).
s/Security processor (PSP)/AMD Secure Processor/
This communication channel is encrypted between the security
here as well.
processor and the guest, hypervisor is just the conduit to
s/hypervisor/the hypervisor/
deliver the guest messages to the security processor. Each
message is protected with an AEAD (AES-256 GCM). Use minimal
GCM library to encrypt/decrypt SNP Guest messages to communicate
with the PSP.
Moreover, the hypervisor should not be intercepting RDTSC/RDTSCP
when Secure TSC is enabled. A #VC exception will be generated if
the RDTSC/RDTSCP instructions are being intercepted. If this should
occur and Secure TSC is enabled, terminate guest execution.
This seems like a separate patch.
Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
---
arch/x86/include/asm/sev-guest.h | 18 +++++++
arch/x86/include/asm/sev.h | 2 +
arch/x86/include/asm/svm.h | 6 ++-
arch/x86/kernel/sev-shared.c | 7 +++
arch/x86/kernel/sev.c | 92 +++++++++++++++++++++++++++++---
arch/x86/mm/mem_encrypt_amd.c | 6 +++
include/linux/cc_platform.h | 8 +++
7 files changed, 131 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/sev-guest.h b/arch/x86/include/asm/sev-guest.h
index 834cdae302ad..d5ed041ce06b 100644
--- a/arch/x86/include/asm/sev-guest.h
+++ b/arch/x86/include/asm/sev-guest.h
@@ -37,6 +37,8 @@ enum msg_type {
SNP_MSG_ABSORB_RSP,
SNP_MSG_VMRK_REQ,
SNP_MSG_VMRK_RSP,
+ SNP_MSG_TSC_INFO_REQ = 17,
+ SNP_MSG_TSC_INFO_RSP,
SNP_MSG_TYPE_MAX
};
@@ -75,6 +77,22 @@ struct snp_guest_req {
u8 msg_type;
};
+struct snp_tsc_info_req {
+#define SNP_TSC_INFO_REQ_SZ 128
+ /* Must be zero filled */
+ u8 rsvd[SNP_TSC_INFO_REQ_SZ];
+} __packed;
+
+struct snp_tsc_info_resp {
+ /* Status of TSC_INFO message */
+ u32 status;
+ u32 rsvd1;
+ u64 tsc_scale;
+ u64 tsc_offset;
+ u64 tsc_factor;
This should be a u32 ...
+ u8 rsvd2[96];
Which then makes this 100.
+} __packed;
+
int snp_send_guest_request(struct snp_guest_dev *dev, struct snp_guest_req *req);
bool snp_assign_vmpck(struct snp_guest_dev *dev, int vmpck_id);
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 36868e21c3e0..d05cbab5e9e0 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -218,6 +218,7 @@ void snp_set_memory_private(unsigned long vaddr, unsigned int npages);
void snp_set_wakeup_secondary_cpu(void);
bool snp_init(struct boot_params *bp);
void __init __noreturn snp_abort(void);
+bool __init snp_secure_tsc_prepare(void);
int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err);
#else
static inline void sev_es_ist_enter(struct pt_regs *regs) { }
@@ -238,6 +239,7 @@ static inline void snp_set_memory_private(unsigned long vaddr, unsigned int npag
static inline void snp_set_wakeup_secondary_cpu(void) { }
static inline bool snp_init(struct boot_params *bp) { return false; }
static inline void snp_abort(void) { }
+static inline bool __init snp_secure_tsc_prepare(void) { return false; }
static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input,
unsigned long *fw_err)
{
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 770dcf75eaa9..c781f8e0aae4 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -404,7 +404,9 @@ struct sev_es_save_area {
u8 reserved_0x298[80];
u32 pkru;
u32 tsc_aux;
- u8 reserved_0x2f0[24];
+ u64 tsc_scale;
+ u64 tsc_offset;
+ u8 reserved_0x300[8];
u64 rcx;
u64 rdx;
u64 rbx;
@@ -536,7 +538,7 @@ static inline void __unused_size_checks(void)
BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x1c0);
BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x248);
BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x298);
- BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x2f0);
+ BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x300);
BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x320);
BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x380);
BUILD_BUG_RESERVED_OFFSET(sev_es_save_area, 0x3f0);
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 3a5b0c9c4fcc..1c22025b298f 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -912,6 +912,13 @@ static enum es_result vc_handle_rdtsc(struct ghcb *ghcb,
bool rdtscp = (exit_code == SVM_EXIT_RDTSCP);
enum es_result ret;
+ /*
+ * RDTSC and RDTSCP should not be intercepted when Secure TSC is
+ * enabled. Terminate the SNP guest when the interception is enabled.
+ */
+ if (sev_status & MSR_AMD64_SNP_SECURE_TSC)
+ return ES_VMM_ERROR;
+
ret = sev_es_ghcb_hv_call(ghcb, ctxt, exit_code, 0, 0);
if (ret != ES_OK)
return ret;
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 3750e545d688..280aaa1e6aad 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -72,6 +72,10 @@ static struct ghcb *boot_ghcb __section(".data");
/* Bitmap of SEV features supported by the hypervisor */
static u64 sev_hv_features __ro_after_init;
+/* Secure TSC values read using TSC_INFO SNP Guest request */
+static u64 guest_tsc_scale __ro_after_init;
+static u64 guest_tsc_offset __ro_after_init;
+
/* #VC handler runtime per-CPU data */
struct sev_es_runtime_data {
struct ghcb ghcb_page;
@@ -1107,7 +1111,7 @@ static void *alloc_shared_pages(size_t sz)
return page_address(page);
}
-static int snp_setup_psp_messaging(struct sev_guest_platform_data *pdata)
+static int __init snp_setup_psp_messaging(struct sev_guest_platform_data *pdata)
{
u64 gpa;
int ret;
@@ -1406,6 +1410,80 @@ bool snp_assign_vmpck(struct snp_guest_dev *dev, int vmpck_id)
}
EXPORT_SYMBOL_GPL(snp_assign_vmpck);
+static int __init snp_get_tsc_info(void)
+{
+ u8 buf[SNP_TSC_INFO_REQ_SZ + AUTHTAG_LEN];
+ struct snp_tsc_info_resp tsc_resp = {0};
+ struct snp_tsc_info_req tsc_req;
+ struct snp_guest_req req;
+ struct snp_guest_dev dev;
+ int rc, resp_len;
+
+ /*
+ * The intermediate response buffer is used while decrypting the
+ * response payload. Make sure that it has enough space to cover the
+ * authtag.
+ */
+ resp_len = sizeof(tsc_resp) + AUTHTAG_LEN;
+ if (sizeof(buf) < resp_len)
+ return -EINVAL;
+
+ /* Zero the tsc_info_req */
+ memzero_explicit(&tsc_req, sizeof(tsc_req));
+ memzero_explicit(&req, sizeof(req));
+
+ dev.pdata = platform_data;
+ if (!snp_assign_vmpck(&dev, 0))
+ return -EINVAL;
+
+ req.msg_version = MSG_HDR_VER;
+ req.msg_type = SNP_MSG_TSC_INFO_REQ;
+ req.req_buf = &tsc_req;
+ req.req_sz = sizeof(tsc_req);
+ req.resp_buf = buf;
+ req.resp_sz = resp_len;
+ req.fw_err = NULL;
+ req.exit_code = SVM_VMGEXIT_GUEST_REQUEST;
+ rc = snp_send_guest_request(&dev, &req);
+ if (rc)
+ goto err_req;
+
+ memcpy(&tsc_resp, buf, sizeof(tsc_resp));
+ pr_debug("%s: Valid response status %x scale %llx offset %llx factor %llx\n",
+ __func__, tsc_resp.status, tsc_resp.tsc_scale, tsc_resp.tsc_offset,
+ tsc_resp.tsc_factor);
+
+ guest_tsc_scale = tsc_resp.tsc_scale;
+ guest_tsc_offset = tsc_resp.tsc_offset;
+
+err_req:
+ /* The response buffer contains the sensitive data, explicitly clear it. */
+ memzero_explicit(buf, sizeof(buf));
+ memzero_explicit(&tsc_resp, sizeof(tsc_resp));
+ memzero_explicit(&req, sizeof(req));
+
+ return rc;
+}
+
+bool __init snp_secure_tsc_prepare(void)
+{
+ platform_data = kzalloc(sizeof(*platform_data), GFP_KERNEL);
+ if (!platform_data)
+ return false;
+
+ /* Initialize the PSP channel to send snp messages */
+ if (snp_setup_psp_messaging(platform_data))
+ sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
+
+ if (cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
Should this be checked before allocating memory and calling
snp_setup_psp_messaging()?
Also, I notice here you use the cc_platform_has() function but in previous
patches you check sev_status directly. And you don't implement support for
CC_ATTR_GUEST_SECURE_TSC until the last patch instead of now.
You can't get here until SNP_FEATURES_PRESENT is updated.
+ if (snp_get_tsc_info())
+ sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
+
+ pr_info("SecureTSC enabled\n");
+ }
Blank line.
Thanks,
Tom
+ return true;
+}
+
static int wakeup_cpu_via_vmgexit(int apic_id, unsigned long start_ip)
{
struct sev_es_save_area *cur_vmsa, *vmsa;
@@ -1506,6 +1584,12 @@ static int wakeup_cpu_via_vmgexit(int apic_id, unsigned long start_ip)
vmsa->vmpl = 0;
vmsa->sev_features = sev_status >> 2;
+ /* Setting Secure TSC parameters */
+ if (cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
+ vmsa->tsc_scale = guest_tsc_scale;
+ vmsa->tsc_offset = guest_tsc_offset;
+ }
+
/* Switch the page over to a VMSA page now that it is initialized */
ret = snp_set_vmsa(vmsa, true);
if (ret) {
@@ -2698,11 +2782,7 @@ static int __init snp_init_platform_device(void)
if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
return -ENODEV;
- platform_data = kzalloc(sizeof(*platform_data), GFP_KERNEL);
- if (!platform_data)
- return -ENOMEM;
-
- if (snp_setup_psp_messaging(platform_data))
+ if (!platform_data->ctx)
return -ENODEV;
if (platform_device_add_data(&sev_guest_device, platform_data, sizeof(*platform_data)))
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 9c4d8dbcb129..7d2388e52b8f 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -215,6 +215,11 @@ void __init sme_map_bootdata(char *real_mode_data)
__sme_early_map_unmap_mem(__va(cmdline_paddr), COMMAND_LINE_SIZE, true);
}
+void __init amd_enc_init(void)
+{
+ snp_secure_tsc_prepare();
+}
+
void __init sev_setup_arch(void)
{
phys_addr_t total_mem = memblock_phys_mem_size();
@@ -501,6 +506,7 @@ void __init sme_early_init(void)
x86_platform.guest.enc_status_change_finish = amd_enc_status_change_finish;
x86_platform.guest.enc_tlb_flush_required = amd_enc_tlb_flush_required;
x86_platform.guest.enc_cache_flush_required = amd_enc_cache_flush_required;
+ x86_platform.guest.enc_init = amd_enc_init;
}
void __init mem_encrypt_free_decrypted_mem(void)
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index cb0d6cd1c12f..e081ca4d5da2 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -90,6 +90,14 @@ enum cc_attr {
* Examples include TDX Guest.
*/
CC_ATTR_HOTPLUG_DISABLED,
+
+ /**
+ * @CC_ATTR_GUEST_SECURE_TSC: Secure TSC is active.
+ *
+ * The platform/OS is running as a guest/virtual machine and actively
+ * using AMD SEV-SNP Secure TSC feature.
+ */
+ CC_ATTR_GUEST_SECURE_TSC,
};
#ifdef CONFIG_ARCH_HAS_CC_PLATFORM