[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7kuhiyy7gj4py323g5n2vy3ddlg666zwhtx3mjcklebgtlstdc@xgdyeecifwei>
Date: Tue, 18 Mar 2025 11:07:57 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Jarkko Sakkinen <jarkko@...nel.org>,
Tom Lendacky <thomas.lendacky@....com>, Borislav Petkov <bp@...en8.de>
Cc: Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>,
x86@...nel.org, linux-kernel@...r.kernel.org, linux-integrity@...r.kernel.org,
Dov Murik <dovmurik@...ux.ibm.com>, Dionna Glaze <dionnaglaze@...gle.com>,
linux-coco@...ts.linux.dev, James Bottomley <James.Bottomley@...senpartnership.com>,
Claudio Carvalho <cclaudio@...ux.ibm.com>, Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
Joerg Roedel <jroedel@...e.de>
Subject: Re: [PATCH v3 1/4] x86/sev: add SVSM vTPM probe/send_command
functions
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@...senPartnership.com>
>> > Signed-off-by: James Bottomley <James.Bottomley@...senPartnership.com>
>> > Co-developed-by: Claudio Carvalho <cclaudio@...ux.ibm.com>
>> > Signed-off-by: Claudio Carvalho <cclaudio@...ux.ibm.com>
>> > Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
>>
>> One minor nit below, otherwise:
>>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@....com>
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?
Thanks,
Stefano
Powered by blists - more mailing lists