Re: [PATCH v3 1/4] x86/sev: add SVSM vTPM probe/send_command functions

[Jarkko Sakkinen <jarkko@kernel.org>]

On Wed, Mar 12, 2025 at 11:56:06AM +0100, Stefano Garzarella wrote:
> On Tue, Mar 11, 2025 at 11:56:23AM +0200, Jarkko Sakkinen wrote:
> > On Tue, Mar 11, 2025 at 10:42:22AM +0100, Stefano Garzarella wrote:
> > > Add two new functions to probe and send commands to the SVSM vTPM.
> > > They leverage the two calls defined by the AMD SVSM specification [1]
> > > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD.
> > > 
> > > Expose these functions to be used by other modules such as a tpm
> > > driver.
> > > 
> > > [1] "Secure VM Service Module for SEV-SNP Guests"
> > >     Publication # 58019 Revision: 1.00
> > > 
> > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> > > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > v3:
> > > - removed link to the spec because those URLs are unstable [Borislav]
> > > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch
> > >   in this one [Borislav]
> > > - slimmed down snp_svsm_vtpm_probe() [Borislav]
> > > - removed features check and any print related [Tom]
> > > ---
> > >  arch/x86/include/asm/sev.h |  7 +++++++
> > >  arch/x86/coco/sev/core.c   | 31 +++++++++++++++++++++++++++++++
> > >  2 files changed, 38 insertions(+)
> > > 
> > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> > > index ba7999f66abe..09471d058ce5 100644
> > > --- a/arch/x86/include/asm/sev.h
> > > +++ b/arch/x86/include/asm/sev.h
> > > @@ -384,6 +384,10 @@ struct svsm_call {
> > >  #define SVSM_ATTEST_SERVICES		0
> > >  #define SVSM_ATTEST_SINGLE_SERVICE	1
> > > 
> > > +#define SVSM_VTPM_CALL(x)		((2ULL << 32) | (x))
> > > +#define SVSM_VTPM_QUERY			0
> > > +#define SVSM_VTPM_CMD			1
> > > +
> > >  #ifdef CONFIG_AMD_MEM_ENCRYPT
> > > 
> > >  extern u8 snp_vmpl;
> > > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
> > >  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req,
> > >  			   struct snp_guest_request_ioctl *rio);
> > > 
> > > +bool snp_svsm_vtpm_probe(void);
> > > +int snp_svsm_vtpm_send_command(u8 *buffer);
> > > +
> > >  void __init snp_secure_tsc_prepare(void);
> > >  void __init snp_secure_tsc_init(void);
> > > 
> > > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> > > index 96c7bc698e6b..2166bdff88b7 100644
> > > --- a/arch/x86/coco/sev/core.c
> > > +++ b/arch/x86/coco/sev/core.c
> > > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat
> > >  	return ret;
> > >  }
> > > 
> > 
> > Since this is an exported symbol, it'd be a good practice document
> > snp_svsm_vtpm_probe().
> 
> Yes, you are right, since the others were not documented, I had not added
> it, but I agree with you, I'll do in v4.
> 
> > 
> > > +bool snp_svsm_vtpm_probe(void)
> > > +{
> > > +	struct svsm_call call = {};
> > > +
> > > +	/* The vTPM device is available only if a SVSM is present */
> > > +	if (!snp_vmpl)
> > > +		return false;
> > > +
> > > +	call.caa = svsm_get_caa();
> > > +	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> > 
> > I supposed CAA is some kind of shared memory area for host and VM?
> 
> Not with the host, but with SVSM, which is the firmware running in the
> guest, but at a higher privilege level (VMPL) than the kernel, where, for
> example, the vTPM is emulated.
> 
> BTW, yep it is a shared memory defined by the SVSM calling convention.
> From AMD SVSM specification:
> 
>   5 Calling Convention
> 
>     Each call to the SVSM conveys data through a combination of the
>     SVSM Calling Area (whose address was first configured through the
>     SVSM_CAA field of the secrets page) and registers. Use of the
>     Calling Area is necessary for the SVSM to detect the difference
>     between a call that was issued by the guest and a spurious
>     invocation by a poorly behaved host. Registers are used for all
>     other parameters.
>     The initially configured SVSM Calling Area is a page of memory that
>     lies outside the initial SVSM memory range and has not had its VMPL
>     permissions restricted in any way. The address is guaranteed to be
>     aligned to a 4 KB boundary, so the remainder of the page may be used
>     by the guest for memory-based parameter passing if desired.
>     The contents of the Calling Area are described in the following
>     table:
> 
>     Table 2: Calling Area
>     Byte      Size     Name                Description
>     Offset
> 
>     0x000     1 byte   SVSM_CALL_PENDING   Indicates whether a call has
>                                            been requested by the guest
>                                            (0=no call requested, 1=call
>                                            requested).
>     0x001     1 byte   SVSM_MEM_AVAILABLE  Free memory is available to
>                                            be withdrawn.
>     0x002     6 byte                       Reserved. The SVSM is not
>                                            required to verify that
>                                            these bytes are 0.
> 
> > 
> > > +
> > > +	if (svsm_perform_call_protocol(&call))
> > > +		return false;
> > > +
> > > +	/* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */
> > > +	return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8);
> > > +}
> > > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe);
> > > +
> > 
> > Ditto.
> 
> Ack.
> 
> > 
> > > +int snp_svsm_vtpm_send_command(u8 *buffer)
> > > +{
> > > +	struct svsm_call call = {};
> > > +
> > > +	call.caa = svsm_get_caa();
> > > +	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
> > > +	call.rcx = __pa(buffer);
> > > +
> > > +	return svsm_perform_call_protocol(&call);
> > > +}
> > > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command);
> > > +
> > >  static struct platform_device sev_guest_device = {
> > >  	.name		= "sev-guest",
> > >  	.id		= -1,
> > > --
> > > 2.48.1
> > > 
> > 
> > That said, these are rather self-documenting (i.e, nice and clean).
> 
> Thanks for the review!

Sure, don't worry about it! Let's just cycle this enough rounds that
it fits well...

> Stefano

BR, Jarkko