| 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
| ||
|
Date: Sat, 31 Dec 2022 15:32:42 +0000
From: Jarkko Sakkinen <jarkko@...nel.org>
To: Michael Roth <michael.roth@....com>
Cc: kvm@...r.kernel.org, linux-coco@...ts.linux.dev,
linux-mm@...ck.org, linux-crypto@...r.kernel.org, x86@...nel.org,
linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
jroedel@...e.de, thomas.lendacky@....com, hpa@...or.com,
ardb@...nel.org, pbonzini@...hat.com, seanjc@...gle.com,
vkuznets@...hat.com, wanpengli@...cent.com, jmattson@...gle.com,
luto@...nel.org, dave.hansen@...ux.intel.com, slp@...hat.com,
pgonda@...gle.com, peterz@...radead.org,
srinivas.pandruvada@...ux.intel.com, rientjes@...gle.com,
dovmurik@...ux.ibm.com, tobin@....com, bp@...en8.de,
vbabka@...e.cz, kirill@...temov.name, ak@...ux.intel.com,
tony.luck@...el.com, marcorr@...gle.com,
sathyanarayanan.kuppuswamy@...ux.intel.com, alpergun@...gle.com,
dgilbert@...hat.com, ashish.kalra@....com, harald@...fian.com,
Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH RFC v7 25/64] crypto: ccp: Add support to initialize the
AMD-SP for SEV-SNP
On Wed, Dec 14, 2022 at 01:40:17PM -0600, Michael Roth wrote:
> From: Brijesh Singh <brijesh.singh@....com>
>
> Before SNP VMs can be launched, the platform must be appropriately
> configured and initialized. Platform initialization is accomplished via
> the SNP_INIT command. Make sure to do a WBINVD and issue DF_FLUSH
> command to prepare for the first SNP guest launch after INIT.
>
> During the execution of SNP_INIT command, the firmware configures
> and enables SNP security policy enforcement in many system components.
> Some system components write to regions of memory reserved by early
> x86 firmware (e.g. UEFI). Other system components write to regions
> provided by the operation system, hypervisor, or x86 firmware.
> Such system components can only write to HV-fixed pages or Default
> pages. They will error when attempting to write to other page states
> after SNP_INIT enables their SNP enforcement.
>
> Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list of
> system physical address ranges to convert into the HV-fixed page states
> during the RMP initialization. If INIT_RMP is 1, hypervisors should
> provide all system physical address ranges that the hypervisor will
> never assign to a guest until the next RMP re-initialization.
> For instance, the memory that UEFI reserves should be included in the
> range list. This allows system components that occasionally write to
> memory (e.g. logging to UEFI reserved regions) to not fail due to
> RMP initialization and SNP enablement.
>
> Co-developed-by: Ashish Kalra <ashish.kalra@....com>
> Signed-off-by: Ashish Kalra <ashish.kalra@....com>
> Signed-off-by: Brijesh Singh <brijesh.singh@....com>
> Signed-off-by: Michael Roth <michael.roth@....com>
> ---
> drivers/crypto/ccp/sev-dev.c | 225 +++++++++++++++++++++++++++++++++++
> drivers/crypto/ccp/sev-dev.h | 2 +
> include/linux/psp-sev.h | 17 +++
> 3 files changed, 244 insertions(+)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 9d84720a41d7..af20420bd6c2 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -26,6 +26,7 @@
> #include <linux/fs_struct.h>
>
> #include <asm/smp.h>
> +#include <asm/e820/types.h>
>
> #include "psp-dev.h"
> #include "sev-dev.h"
> @@ -34,6 +35,10 @@
> #define SEV_FW_FILE "amd/sev.fw"
> #define SEV_FW_NAME_SIZE 64
>
> +/* Minimum firmware version required for the SEV-SNP support */
> +#define SNP_MIN_API_MAJOR 1
> +#define SNP_MIN_API_MINOR 51
> +
> static DEFINE_MUTEX(sev_cmd_mutex);
> static struct sev_misc_dev *misc_dev;
>
> @@ -76,6 +81,13 @@ static void *sev_es_tmr;
> #define NV_LENGTH (32 * 1024)
> static void *sev_init_ex_buffer;
>
> +/*
> + * SEV_DATA_RANGE_LIST:
> + * Array containing range of pages that firmware transitions to HV-fixed
> + * page state.
> + */
> +struct sev_data_range_list *snp_range_list;
> +
> static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
> {
> struct sev_device *sev = psp_master->sev_data;
> @@ -830,6 +842,186 @@ static int sev_update_firmware(struct device *dev)
> return ret;
> }
>
> +static void snp_set_hsave_pa(void *arg)
> +{
> + wrmsrl(MSR_VM_HSAVE_PA, 0);
> +}
> +
> +static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
> +{
> + struct sev_data_range_list *range_list = arg;
> + struct sev_data_range *range = &range_list->ranges[range_list->num_elements];
> + size_t size;
> +
> + if ((range_list->num_elements * sizeof(struct sev_data_range) +
> + sizeof(struct sev_data_range_list)) > PAGE_SIZE)
> + return -E2BIG;
> +
> + switch (rs->desc) {
> + case E820_TYPE_RESERVED:
> + case E820_TYPE_PMEM:
> + case E820_TYPE_ACPI:
> + range->base = rs->start & PAGE_MASK;
> + size = (rs->end + 1) - rs->start;
> + range->page_count = size >> PAGE_SHIFT;
> + range_list->num_elements++;
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int __sev_snp_init_locked(int *error)
> +{
> + struct psp_device *psp = psp_master;
> + struct sev_data_snp_init_ex data;
> + struct sev_device *sev;
> + int rc = 0;
> +
> + if (!psp || !psp->sev_data)
> + return -ENODEV;
> +
> + sev = psp->sev_data;
> +
> + if (sev->snp_initialized)
> + return 0;
Shouldn't this follow this check:
if (sev->state == SEV_STATE_INIT) {
/* debug printk about possible incorrect call order */
return -ENODEV;
}
It is game over for SNP, if SEV_CMD_INIT{_EX} got first, which means that
this should not proceed.
BR, Jarkko
Powered by blists - more mailing lists