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

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Mar 25, 2025 at 06:20:48PM +0100, Stefano Garzarella wrote:
> On Tue, Mar 25, 2025 at 09:56:53AM -0700, Dionna Amalie Glaze wrote:
> > On Mon, Mar 24, 2025 at 3:47 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
> > > 
> > > From: Stefano Garzarella <sgarzare@redhat.com>
> > > 
> > > 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>
> > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > v4:
> > > - added Tom's R-b
> > > - added functions documentation [Jarkko]
> > > - simplified TPM_SEND_COMMAND check [Tom/Jarkko]
> > > 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   | 59 ++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 66 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);
> > > +
> > 
> > These should be defined static inline with trivial definitions in the
> > #else case, yes?
> 
> For now the only user of those is the tpm_svsm driver which is build only if
> CONFIG_AMD_MEM_ENCRYPT is defined, so there should be no problem, but you
> are right, better to follow the other functions and define the stubs
> 
> I'll fix in v5, good catch!

Only denoting here that you can keep my reviewed-by in the v5 for
this despite the minor change.

> 
> Thanks,
> Stefano
> 

BR, Jarkko