linux-kernel - Re: [PATCH v3 1/4] x86/sev: add SVSM vTPM probe/send

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2of2zhxi2c735fgvjxug2bxjfpz2zk25adf3h2ps5byau3rj3k@pgbxmpbskezi>
Date: Wed, 12 Mar 2025 11:56:06 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Jarkko Sakkinen <jarkko@...nel.org>
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 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!
Stefano