On 12/5/2024 5:25 PM, Borislav Petkov wrote:
> On Tue, Dec 03, 2024 at 02:30:35PM +0530, Nikunj A Dadhania wrote:
>> Add support for Secure TSC in SNP-enabled guests. Secure TSC allows guests
>> to securely use RDTSC/RDTSCP instructions, ensuring that the parameters
>> used cannot be altered by the hypervisor once the guest is launched.
>>
>> Secure TSC-enabled guests need to query TSC information from the AMD
>> Security Processor. This communication channel is encrypted between the AMD
>> Security Processor and the guest, with the hypervisor acting merely as a
>> conduit to deliver the guest messages to the AMD Security Processor. Each
>> message is protected with AEAD (AES-256 GCM).
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
>> Tested-by: Peter Gonda <pgonda@xxxxxxxxxx>
>
>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>
> This patch changed somewhat from last time.
Yes, most of the change was dynamic allocation in snp_get_tsc_info().
> When did Peter test it again and
> Tom review it again?
It makes sense to drop both.
>
>> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
>> index a61898c7f114..39683101b526 100644
>> --- a/arch/x86/coco/sev/core.c
>> +++ b/arch/x86/coco/sev/core.c
>> @@ -96,6 +96,14 @@ static u64 sev_hv_features __ro_after_init;
>> /* Secrets page physical address from the CC blob */
>> static u64 secrets_pa __ro_after_init;
>>
>> +/*
>> + * For Secure TSC guests, the BP fetches TSC_INFO using SNP guest messaging and
>
> s/BP/BSP/
>
>> + * initializes snp_tsc_scale and snp_tsc_offset. These values are replicated
>> + * across the APs VMSA fields (TSC_SCALE and TSC_OFFSET).
>> + */
>> +static u64 snp_tsc_scale __ro_after_init;
>> +static u64 snp_tsc_offset __ro_after_init;
>> +
>> /* #VC handler runtime per-CPU data */
>> struct sev_es_runtime_data {
>> struct ghcb ghcb_page;
>
> ...
>
>> + memcpy(tsc_resp, buf, sizeof(*tsc_resp));
>> + pr_debug("%s: response status 0x%x scale 0x%llx offset 0x%llx factor 0x%x\n",
>> + __func__, tsc_resp->status, tsc_resp->tsc_scale, tsc_resp->tsc_offset,
>> + tsc_resp->tsc_factor);
>> +
>> + if (tsc_resp->status == 0) {
>
> Like the last time:
>
> if (!tsc_resp->status)
Ack.
Regards
Nikunj
