Messages in this thread | | | Date | Thu, 5 Jan 2023 16:40:29 -0600 | Subject | Re: [PATCH RFC v7 25/64] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP | From | "Kalra, Ashish" <> |
| |
Hello Jarkko,
On 12/31/2022 9:32 AM, Jarkko Sakkinen wrote: > On Wed, Dec 14, 2022 at 01:40:17PM -0600, Michael Roth wrote: >> From: Brijesh Singh <brijesh.singh@amd.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@amd.com> >> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> Signed-off-by: Michael Roth <michael.roth@amd.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.
But, how will SEV_CMD_INIT_EX happen before as sev_pci_init() which is invoked during CCP module load/initialization, will first try to do sev_snp_init() if SNP is supported, before it invokes sev_platform_init() to do SEV firmware initialization ?
Thanks, Ashish
| |