Re: [PATCH v15 06/13] x86/sev: Prevent GUEST_TSC_FREQ MSR interception for Secure TSC enabled guests

[Tom Lendacky <thomas.lendacky@amd.com>]

On 1/2/25 03:30, Nikunj A. Dadhania wrote:
> On 1/2/2025 2:37 PM, Borislav Petkov wrote:
>> On Thu, Jan 02, 2025 at 10:33:26AM +0530, Nikunj A. Dadhania wrote:
> 
>> As in: I will handle the TSC MSRs for STSC guests and the other flow for
>> non-STSC guests should remain. For now.
>>
>> And make that goddamn explicit.
>>
>> One possible way to do that is this:
> 
> I agree, if renaming helps to make it explicit, this is perfect. Thanks.
> 
>>
>> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
>> index 6235286a0eda..61100532c259 100644
>> --- a/arch/x86/coco/sev/core.c
>> +++ b/arch/x86/coco/sev/core.c
>> @@ -1439,7 +1439,7 @@ static enum es_result __vc_handle_msr_caa(struct pt_regs *regs, bool write)
>>   * Reads:  Reads of MSR_IA32_TSC should return the current TSC
>>   *         value, use the value returned by RDTSC.
>>   */
>> -static enum es_result __vc_handle_msr_tsc(struct pt_regs *regs, bool write)
>> +static enum es_result __vc_handle_secure_tsc_msrs(struct pt_regs *regs, bool write)
>>  {
>>  	u64 tsc;
>>  
>> @@ -1477,7 +1477,9 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
>>  	case MSR_IA32_TSC:
>>  	case MSR_AMD64_GUEST_TSC_FREQ:
>>  		if (sev_status & MSR_AMD64_SNP_SECURE_TSC)
>> -			return __vc_handle_msr_tsc(regs, write);
>> +			return __vc_handle_secure_tsc_msrs(regs, write);
>> +		else
>> +			break;

There's a return as part of the if, so no reason for the else. Just put
the break in the normal place and it reads much clearer.

Thanks,
Tom

>>  	default:
>>  		break;
>>  	}
>> ---
> 
> Regards,
> Nikunj