On Tue, Mar 18, 2025 at 11:07:57AM +0100, Stefano Garzarella wrote:
> On Mon, Mar 17, 2025 at 03:36:26PM +0200, Jarkko Sakkinen wrote:
> > On Fri, Mar 14, 2025 at 10:27:07AM -0500, Tom Lendacky wrote:
> > > On 3/11/25 04:42, 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@xxxxxxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> > > > Co-developed-by: Claudio Carvalho <cclaudio@xxxxxxxxxxxxx>
> > > > Signed-off-by: Claudio Carvalho <cclaudio@xxxxxxxxxxxxx>
> > > > Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
> > >
> > > One minor nit below, otherwise:
> > >
> > > Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>
> Thanks!
>
> > >
> > > > ---
> > > > 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;
> > > > }
> > > >
> > > > +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);
> > > > +
> > > > + 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);
> > >
> > > It's a bool function, so this could simplified to just:
> > >
> > > return call.rcx_out & BIT_ULL(8);
>
> Sure.
>
> >
> > Or perhaps even just "call.rcx_out & 0x100". I don't think BIT_ULL()
> > here brings much additional clarity or anything useful...
>
> I can do that, I slightly prefer BIT_ULL() macro, but I don't have a strong
> opinion on my side.
> @Borislav since you suggested it, WDYT?
Either goes for me. Sorry for nitpicking that :-) The first comment
stil applies.
>
> Thanks,
> Stefano
>
BR, Jarkko
