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.

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Tested-by: Peter Gonda <pgonda@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/coco/sev/shared.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/coco/sev/shared.c b/arch/x86/coco/sev/shared.c
index 71de53194089..c2a9e2ada659 100644
--- a/arch/x86/coco/sev/shared.c
+++ b/arch/x86/coco/sev/shared.c
@@ -1140,6 +1140,16 @@ 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.
+	 * This file is included from kernel/sev.c and boot/compressed/sev.c,
+	 * use sev_status here as cc_platform_has() is not available when
+	 * compiling boot/compressed/sev.c.
+	 */
+	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;
-- 
2.34.1

On 10/21/2024 1:51 PM, Nikunj A Dadhania wrote:
> 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.

There is another option to ignore the interception and just return back 
to guest execution. I think it better to add some justification on why 
make it fatal and terminate the guest is better than ignoring the 
interception.

> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> Tested-by: Peter Gonda <pgonda@google.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>   arch/x86/coco/sev/shared.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/coco/sev/shared.c b/arch/x86/coco/sev/shared.c
> index 71de53194089..c2a9e2ada659 100644
> --- a/arch/x86/coco/sev/shared.c
> +++ b/arch/x86/coco/sev/shared.c
> @@ -1140,6 +1140,16 @@ 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.
> +	 * This file is included from kernel/sev.c and boot/compressed/sev.c,
> +	 * use sev_status here as cc_platform_has() is not available when
> +	 * compiling boot/compressed/sev.c.
> +	 */
> +	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;

On 10/24/2024 1:26 PM, Xiaoyao Li wrote:
> On 10/21/2024 1:51 PM, Nikunj A Dadhania wrote:
>> 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.
> 
> There is another option to ignore the interception and just return back to
> guest execution. 

That is not correct, RDTSC/RDTSCP should return the timestamp counter value
computed using the GUEST_TSC_SCALE and GUEST_TSC_OFFSET part of VMSA.
 
> I think it better to add some justification on why make it> fatal and terminate the guest is better than ignoring the interception.

How about the below updated commit message:

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 as the guest cannot rely on the TSC value provided 
by the hypervisor.

Regards
Nikunj

On 10/24/2024 4:44 PM, Nikunj A. Dadhania wrote:
> 
> 
> On 10/24/2024 1:26 PM, Xiaoyao Li wrote:
>> On 10/21/2024 1:51 PM, Nikunj A Dadhania wrote:
>>> 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.
>>
>> There is another option to ignore the interception and just return back to
>> guest execution.
> 
> That is not correct, RDTSC/RDTSCP should return the timestamp counter value
> computed using the GUEST_TSC_SCALE and GUEST_TSC_OFFSET part of VMSA.

Ah, I missed this. Yes, if ignore the interception, guest needs to do 
TSC scale itself with GUEST_TSC_SCALE and GUEST_TSC_OFFSET to get the 
correct TSC. It's complicating things while making not intercepting 
RDTSC/RDTSP a hard requirement is much simple.

I think it's worth adding it as the justification.

>> I think it better to add some justification on why make it> fatal and terminate the guest is better than ignoring the interception.
> 
> How about the below updated commit message:
> 
> 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 as the guest cannot rely on the TSC value provided
> by the hypervisor.
> 
> Regards
> Nikunj
>