| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z9RFYCAGEZTVfqjb@kernel.org>
Date: Fri, 14 Mar 2025 17:04:00 +0200
From: Jarkko Sakkinen <jarkko@...nel.org>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: Peter Huewe <peterhuewe@....de>, Tom Lendacky <thomas.lendacky@....com>,
Jason Gunthorpe <jgg@...pe.ca>, x86@...nel.org,
linux-kernel@...r.kernel.org, Borislav Petkov <bp@...en8.de>,
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 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@...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>
> > > ---
> > > 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
Powered by blists - more mailing lists