linux-kernel - Re: [PATCH 3/3] x86/sev: add a SVSM vTPM platform device

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: <f8c6c1e0-a42d-6fa6-a10e-925592d7992f@amd.com>
Date: Wed, 11 Dec 2024 10:30:50 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Stefano Garzarella <sgarzare@...hat.com>, linux-coco@...ts.linux.dev
Cc: Borislav Petkov <bp@...en8.de>, Dave Hansen
 <dave.hansen@...ux.intel.com>, Peter Huewe <peterhuewe@....de>,
 "H. Peter Anvin" <hpa@...or.com>, linux-integrity@...r.kernel.org,
 James Bottomley <James.Bottomley@...senPartnership.com>, x86@...nel.org,
 Joerg Roedel <jroedel@...e.de>, Jason Gunthorpe <jgg@...pe.ca>,
 Jarkko Sakkinen <jarkko@...nel.org>, linux-kernel@...r.kernel.org,
 Ingo Molnar <mingo@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
 Claudio Carvalho <cclaudio@...ux.ibm.com>, Dov Murik <dovmurik@...ux.ibm.com>
Subject: Re: [PATCH 3/3] x86/sev: add a SVSM vTPM platform device

On 12/10/24 08:34, Stefano Garzarella wrote:
> From: James Bottomley <James.Bottomley@...senPartnership.com>
> 
> If the SNP boot has a SVSM, probe for the vTPM device by sending a
> SVSM_VTPM_QUERY call (function 8). The SVSM will return a bitmap with
> the TPM_SEND_COMMAND bit set only if the vTPM is present and it is able
> to handle TPM commands at runtime.
> 
> If a vTPM is found, register a platform device as "platform:tpm" so it
> can be attached to the tpm_platform.c driver.
> 
> Signed-off-by: James Bottomley <James.Bottomley@...senPartnership.com>
> [CC] Used SVSM_VTPM_QUERY to probe the TPM
> Signed-off-by: Claudio Carvalho <cclaudio@...ux.ibm.com>
> [SG] Code adjusted with some changes introduced in 6.11
> [SG] Used macro for SVSM_VTPM_CALL
> Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
> ---
>  arch/x86/coco/sev/core.c | 64 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index c5b0148b8c0a..ec0153fddc9e 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -21,6 +21,7 @@
>  #include <linux/cpumask.h>
>  #include <linux/efi.h>
>  #include <linux/platform_device.h>
> +#include <linux/tpm_platform.h>
>  #include <linux/io.h>
>  #include <linux/psp-sev.h>
>  #include <linux/dmi.h>
> @@ -2578,6 +2579,51 @@ static struct platform_device sev_guest_device = {
>  	.id		= -1,
>  };
>  
> +static struct platform_device tpm_device = {
> +	.name		= "tpm",
> +	.id		= -1,
> +};
> +
> +static int snp_issue_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);
> +}
> +
> +static bool is_svsm_vtpm_send_command_supported(void)
> +{
> +	struct svsm_call call = {};
> +	u64 send_cmd_mask = 0;
> +	u64 platform_cmds;
> +	u64 features;
> +	int ret;
> +
> +	call.caa = svsm_get_caa();
> +	call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> +
> +	ret = svsm_perform_call_protocol(&call);
> +
> +	if (ret != SVSM_SUCCESS)
> +		return false;
> +
> +	features = call.rdx_out;
> +	platform_cmds = call.rcx_out;
> +
> +	/* No feature supported, it must be zero */
> +	if (features)
> +		return false;

I think this check should be removed. The SVSM currently returns all
zeroes for the features to allow for future support. If a new feature is
added in the future, this then allows a driver that supports that
feature to operate with a version of an SVSM that doesn't have that
feature implemented. It also allows a version of the driver that doesn't
know about that feature to work with an SVSM that has that feature.

A feature added to the vTPM shouldn't alter the behavior of something
that isn't using or understands that feature.

> +
> +	/* TPM_SEND_COMMAND - platform command 8 */
> +	send_cmd_mask = 1 << 8;
> +
> +	return (platform_cmds & send_cmd_mask) == send_cmd_mask;
> +}
> +
>  static int __init snp_init_platform_device(void)
>  {
>  	struct sev_guest_platform_data data;
> @@ -2593,6 +2639,24 @@ static int __init snp_init_platform_device(void)
>  		return -ENODEV;
>  
>  	pr_info("SNP guest platform device initialized.\n");
> +
> +	/*
> +	 * The VTPM device is available only if we have a SVSM and
> +	 * its VTPM supports the TPM_SEND_COMMAND platform command

s/VTPM/vTPM/g

Thanks,
Tom

> +	 */
> +	if (IS_ENABLED(CONFIG_TCG_PLATFORM) && snp_vmpl &&
> +	    is_svsm_vtpm_send_command_supported()) {
> +		struct tpm_platform_ops pops = {
> +			.sendrcv = snp_issue_svsm_vtpm_send_command,
> +		};
> +
> +		if (platform_device_add_data(&tpm_device, &pops, sizeof(pops)))
> +			return -ENODEV;
> +		if (platform_device_register(&tpm_device))
> +			return -ENODEV;
> +		pr_info("SNP SVSM VTPM platform device initialized\n");
> +	}
> +
>  	return 0;
>  }
>  device_initcall(snp_init_platform_device);