lkml.org 
[lkml]   [2023]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH RFC v7 25/64] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP
From
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

\
 
 \ /
  Last update: 2023-03-26 23:29    [W:0.569 / U:0.496 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site