| 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: <d598415b-0854-46a2-5262-7e204033c31c@amd.com>
Date: Wed, 23 Apr 2025 16:21:58 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Ashish Kalra <Ashish.Kalra@....com>, seanjc@...gle.com,
pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, hpa@...or.com, herbert@...dor.apana.org.au
Cc: x86@...nel.org, john.allen@....com, davem@...emloft.net,
michael.roth@....com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-crypto@...r.kernel.org
Subject: Re: [PATCH v3 2/4] crypto: ccp: Add support for SNP_FEATURE_INFO
command
On 4/21/25 19:24, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@....com>
>
> The FEATURE_INFO command provides host and guests a programmatic means
> to learn about the supported features of the currently loaded firmware.
> FEATURE_INFO command leverages the same mechanism as the CPUID instruction.
> Instead of using the CPUID instruction to retrieve Fn8000_0024,
> software can use FEATURE_INFO.
>
> Host/Hypervisor would use the FEATURE_INFO command, while guests would
> actually issue the CPUID instruction.
You probably want to word this better in combination with the next
paragraph. The actual CPUID leaf doesn't exist. The hypervisor must
populate the entry in the CPUID page during LAUNCH_UPDATE in order for
the CPUID instruction in the guest to return a value.
>
> The hypervisor can provide Fn8000_0024 values to the guest via the CPUID
> page in SNP_LAUNCH_UPDATE. As with all CPUID output recorded in that page,
> the hypervisor can filter Fn8000_0024. The firmware will examine
> Fn8000_0024 and apply its CPUID policy.
>
> During CCP module initialization, after firmware update, the SNP
> platform status and feature information from CPUID 0x8000_0024,
> sub-function 0, are cached in the sev_device structure.
>
> Signed-off-by: Ashish Kalra <ashish.kalra@....com>
> ---
> drivers/crypto/ccp/sev-dev.c | 47 ++++++++++++++++++++++++++++++++++++
> drivers/crypto/ccp/sev-dev.h | 3 +++
> include/linux/psp-sev.h | 29 ++++++++++++++++++++++
> 3 files changed, 79 insertions(+)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index b08db412f752..f4f8a8905115 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -232,6 +232,7 @@ static int sev_cmd_buffer_len(int cmd)
> case SEV_CMD_SNP_GUEST_REQUEST: return sizeof(struct sev_data_snp_guest_request);
> case SEV_CMD_SNP_CONFIG: return sizeof(struct sev_user_data_snp_config);
> case SEV_CMD_SNP_COMMIT: return sizeof(struct sev_data_snp_commit);
> + case SEV_CMD_SNP_FEATURE_INFO: return sizeof(struct snp_feature_info);
> default: return 0;
> }
>
> @@ -1072,6 +1073,50 @@ static void snp_set_hsave_pa(void *arg)
> wrmsrq(MSR_VM_HSAVE_PA, 0);
> }
>
> +static void snp_get_platform_data(void)
> +{
> + struct sev_device *sev = psp_master->sev_data;
> + struct sev_data_snp_feature_info snp_feat_info;
> + struct snp_feature_info *feat_info;
> + struct sev_data_snp_addr buf;
> + int error = 0, rc;
> +
> + if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
> + return;
> +
> + /*
> + * The output buffer must be firmware page if SEV-SNP is
> + * initialized.
> + */
> + if (sev->snp_initialized)
> + return;
> +
> + buf.address = __psp_pa(&sev->snp_plat_status);
> + rc = __sev_do_cmd_locked(SEV_CMD_SNP_PLATFORM_STATUS, &buf, &error);
See comment below...
> +
> + /*
> + * Do feature discovery of the currently loaded firmware,
> + * and cache feature information from CPUID 0x8000_0024,
> + * sub-function 0.
> + */
> + if (!rc && sev->snp_plat_status.feature_info) {
> + /*
> + * Use dynamically allocated structure for the SNP_FEATURE_INFO
> + * command to handle any alignment and page boundary check
> + * requirements.
> + */
> + feat_info = kzalloc(sizeof(*feat_info), GFP_KERNEL);
Need to check for NULL.
> + snp_feat_info.length = sizeof(snp_feat_info);
> + snp_feat_info.ecx_in = 0;
> + snp_feat_info.feature_info_paddr = __psp_pa(feat_info);
> +
> + rc = __sev_do_cmd_locked(SEV_CMD_SNP_FEATURE_INFO, &snp_feat_info, &error);
> + if (!rc)
> + sev->feat_info = *feat_info;
Should probably issue a message if the command fails.
> + kfree(feat_info);
> + }
> +}
> +
> static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
> {
> struct sev_data_range_list *range_list = arg;
> @@ -2543,6 +2588,8 @@ void sev_pci_init(void)
> api_major, api_minor, build,
> sev->api_major, sev->api_minor, sev->build);
>
> + snp_get_platform_data();
We should switch from using SEV platform status to SNP platform status
(when SNP is available) at the beginning of sev_pci_init() and cache the
results. Then you won't have to issue another platform status command in
snp_get_platform_data().
Thanks,
Tom
> +
> return;
>
> err:
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index 3e4e5574e88a..1c1a51e52d2b 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -57,6 +57,9 @@ struct sev_device {
> bool cmd_buf_backup_active;
>
> bool snp_initialized;
> +
> + struct sev_user_data_snp_status snp_plat_status;
> + struct snp_feature_info feat_info;
> };
>
> int sev_dev_init(struct psp_device *psp);
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 0b3a36bdaa90..0149d4a6aceb 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -107,6 +107,7 @@ enum sev_cmd {
> SEV_CMD_SNP_DOWNLOAD_FIRMWARE_EX = 0x0CA,
> SEV_CMD_SNP_COMMIT = 0x0CB,
> SEV_CMD_SNP_VLEK_LOAD = 0x0CD,
> + SEV_CMD_SNP_FEATURE_INFO = 0x0CE,
>
> SEV_CMD_MAX,
> };
> @@ -812,6 +813,34 @@ struct sev_data_snp_commit {
> u32 len;
> } __packed;
>
> +/**
> + * struct sev_data_snp_feature_info - SEV_SNP_FEATURE_INFO structure
> + *
> + * @length: len of the command buffer read by the PSP
> + * @ecx_in: subfunction index
> + * @feature_info_paddr : SPA of the FEATURE_INFO structure
> + */
> +struct sev_data_snp_feature_info {
> + u32 length;
> + u32 ecx_in;
> + u64 feature_info_paddr;
> +} __packed;
> +
> +/**
> + * struct feature_info - FEATURE_INFO structure
> + *
> + * @eax: output of SNP_FEATURE_INFO command
> + * @ebx: output of SNP_FEATURE_INFO command
> + * @ecx: output of SNP_FEATURE_INFO command
> + * #edx: output of SNP_FEATURE_INFO command
> + */
> +struct snp_feature_info {
> + u32 eax;
> + u32 ebx;
> + u32 ecx;
> + u32 edx;
> +} __packed;
> +
> #ifdef CONFIG_CRYPTO_DEV_SP_PSP
>
> /**
Powered by blists - more mailing lists