Re: [PATCH v1 1/8] x86/sev: Make SVSM calls preemption-safe

[Stefano Garzarella <sgarzare@redhat.com>]

On Mon, Aug 31, 2026 at 06:19:34PM -0700, Borislav Petkov wrote:
>+ folks who added the vtpm stuff.

Thanks.

>
>On Sat, Aug 29, 2026 at 03:39:39AM +0000, Melody Wang wrote:
>> Two functions in the the SVSM vTPM guest implementation do not disable
>> preemption when fetching CAA.
>
>"	... when fetching the SVSM Calling Area Address (CAA).
>
>The SVSM CAA..."
>
>This way, you introduce what CAA means and then use the abbreviation.
>
>> The SVSM calling area is a per-CPU structure. When a thread is preempted
>> and migrated to a different CPU after fetching the per-CPU Calling Area
>> Address (CAA), the SVSM call will execute on the new CPU with the
>
>and here you can use "CAA" directly because you've explained it above.
>
>> original CPU's CAA. Which is wrong.
>>
>> Move the CAA fetching operation inside svsm_perform_call_protocol()
>> which disables interrupts around the SVSM call and thus runs
>> preemption-safe.
>>
>> Signed-off-by: Melody Wang <huibo.wang@amd.com>
>
>Should this be CC:stable so that it gets backported?
>
>I think we want to do that and mark it with
>
>Fixes: 770de678bc28 ("x86/sev: Add SVSM vTPM probe/send_command functions")
>
>> ---
>>  arch/x86/coco/sev/svsm.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/coco/sev/svsm.c b/arch/x86/coco/sev/svsm.c
>> index 916d62cd17dc..92ea93f506c0 100644
>> --- a/arch/x86/coco/sev/svsm.c
>> +++ b/arch/x86/coco/sev/svsm.c
>> @@ -74,6 +74,15 @@ int svsm_perform_call_protocol(struct svsm_call *call)
>>
>>  	flags = native_local_irq_save();
>>
>> +	/*
>> +	 * 'caa' is a per-CPU variable. To avoid using a stale or incorrect
>> +	 * 'caa' if the task is preempted or migrates to another CPU after it
>
>"... or migrated to another CPU after the CAA has been fetched..."
>
>> +	 * is fetched, always fetch 'caa' and then issue the SVSM call with
>> +	 * interrupts disabled. This ensures the correct 'caa' is used even
>> +	 * under preemption or CPU migration.
>
>s/even under preemption or CPU migration//
>
>That's kinda clear.
>
>Leaving in the rest for the newly CCed.
>
>> +	 */
>> +	call->caa = svsm_get_caa();

svsm_perform_call_protocol() is called also by svsm_pval_pages() and 
snp_issue_svsm_attest_req(). After this change call->caa is set 
unconditionally inside svsm_perform_call_protocol(), so the call->caa = 
svsm_get_caa() there becomes somewhat redundant. That said we can't just 
drop it though, so at most it could become a local variable. Probably 
not worth the churn, just wanted to point it out.

That said, I agree that in those functions we don't have the issue since 
both of them are disabling irqs.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano