| Message ID | 20241021055156.2342564-7-nikunj@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Secure TSC support for SNP guests | expand |
On 10/21/24 00:51, Nikunj A Dadhania wrote: > The hypervisor should not be intercepting GUEST_TSC_FREQ MSR(0xcOO10134) > when Secure TSC is enabled. A #VC exception will be generated if the > GUEST_TSC_FREQ MSR is being intercepted. If this should occur and SecureTSC > is enabled, terminate guest execution. > > Signed-off-by: Nikunj A Dadhania <nikunj@amd.com> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> Just a minor comment/question below. > --- > arch/x86/include/asm/msr-index.h | 1 + > arch/x86/coco/sev/core.c | 8 ++++++++ > 2 files changed, 9 insertions(+) > > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h > index 3ae84c3b8e6d..233be13cc21f 100644 > --- a/arch/x86/include/asm/msr-index.h > +++ b/arch/x86/include/asm/msr-index.h > @@ -608,6 +608,7 @@ > #define MSR_AMD_PERF_CTL 0xc0010062 > #define MSR_AMD_PERF_STATUS 0xc0010063 > #define MSR_AMD_PSTATE_DEF_BASE 0xc0010064 > +#define MSR_AMD64_GUEST_TSC_FREQ 0xc0010134 > #define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140 > #define MSR_AMD64_OSVW_STATUS 0xc0010141 > #define MSR_AMD_PPIN_CTL 0xc00102f0 > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c > index 2ad7773458c0..4e9b1cc1f26b 100644 > --- a/arch/x86/coco/sev/core.c > +++ b/arch/x86/coco/sev/core.c > @@ -1332,6 +1332,14 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt) > return ES_OK; > } > > + /* > + * GUEST_TSC_FREQ should not be intercepted when Secure TSC is > + * enabled. Terminate the SNP guest when the interception is enabled. > + */ > + if (regs->cx == MSR_AMD64_GUEST_TSC_FREQ && cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) Should the cc_platform_has() check be changed into a check against sev_status directly (similar to the DEBUG_SWAP support)? Just in case this handler ends up getting used in early code where cc_platform_has() can't be used. Thanks, Tom > + return ES_VMM_ERROR; > + > + > ghcb_set_rcx(ghcb, regs->cx); > if (exit_info_1) { > ghcb_set_rax(ghcb, regs->ax);
On 10/21/2024 7:38 PM, Tom Lendacky wrote: > On 10/21/24 00:51, Nikunj A Dadhania wrote: >> The hypervisor should not be intercepting GUEST_TSC_FREQ MSR(0xcOO10134) >> when Secure TSC is enabled. A #VC exception will be generated if the >> GUEST_TSC_FREQ MSR is being intercepted. If this should occur and SecureTSC >> is enabled, terminate guest execution. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com> > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > > Just a minor comment/question below. > .. >> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c >> index 2ad7773458c0..4e9b1cc1f26b 100644 >> --- a/arch/x86/coco/sev/core.c >> +++ b/arch/x86/coco/sev/core.c >> @@ -1332,6 +1332,14 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt) >> return ES_OK; >> } >> >> + /* >> + * GUEST_TSC_FREQ should not be intercepted when Secure TSC is >> + * enabled. Terminate the SNP guest when the interception is enabled. >> + */ >> + if (regs->cx == MSR_AMD64_GUEST_TSC_FREQ && cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) > > Should the cc_platform_has() check be changed into a check against > sev_status directly (similar to the DEBUG_SWAP support)? Just in case > this handler ends up getting used in early code where cc_platform_has() > can't be used. Sure, that makes sense. I will change it in my next version. Regards Nikunj
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index 3ae84c3b8e6d..233be13cc21f 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -608,6 +608,7 @@ #define MSR_AMD_PERF_CTL 0xc0010062 #define MSR_AMD_PERF_STATUS 0xc0010063 #define MSR_AMD_PSTATE_DEF_BASE 0xc0010064 +#define MSR_AMD64_GUEST_TSC_FREQ 0xc0010134 #define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140 #define MSR_AMD64_OSVW_STATUS 0xc0010141 #define MSR_AMD_PPIN_CTL 0xc00102f0 diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index 2ad7773458c0..4e9b1cc1f26b 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -1332,6 +1332,14 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt) return ES_OK; } + /* + * GUEST_TSC_FREQ should not be intercepted when Secure TSC is + * enabled. Terminate the SNP guest when the interception is enabled. + */ + if (regs->cx == MSR_AMD64_GUEST_TSC_FREQ && cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) + return ES_VMM_ERROR; + + ghcb_set_rcx(ghcb, regs->cx); if (exit_info_1) { ghcb_set_rax(ghcb, regs->ax);
The hypervisor should not be intercepting GUEST_TSC_FREQ MSR(0xcOO10134) when Secure TSC is enabled. A #VC exception will be generated if the GUEST_TSC_FREQ MSR is being intercepted. If this should occur and SecureTSC is enabled, terminate guest execution. Signed-off-by: Nikunj A Dadhania <nikunj@amd.com> --- arch/x86/include/asm/msr-index.h | 1 + arch/x86/coco/sev/core.c | 8 ++++++++ 2 files changed, 9 insertions(+)