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

[Stefano Garzarella]

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@xxxxxxx>
>
> 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@xxxxxxxxxx>

Thanks,
Stefano