From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-SN1-obe.outbound.protection.outlook.com (NAM04-SN1-obe.outbound.protection.outlook.com [40.107.70.58]) by mx.groups.io with SMTP id smtpd.web11.4382.1583245233444748872 for ; Tue, 03 Mar 2020 06:20:33 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=y9aDVGxy; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.70.58, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A/pJzqvBvUfGaVtdo6r/CsKS+3rtltRNsNjkDllm0SFHoVCrnmlSmLQ9bFORjUHugMpPeBnvb62noObwOXUIvUB2vSWAmP3fsO/O1433ozBcOsSYSCBx5FTWFZ7jcETfmOjyJPPCsdPwpSaq32icCqny6FeLmdxOAh+6ijvjDpSBrvq91iBjP24RsxrB0aWvGDs9HbdFtbIWX+mdqTGXHWecV5x7uIqoMeY0bgGQYIj6hlJLDJOjgvHtkKrW/hJp8aBVoPpbpR1AtvGImktfwlv8KaTht3a/p0vIuLUj/qXg18t3cdkt+V1Hm38A7OlierHnpVFY35ZBW+2RMVL4Jg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=gOsNacS0o3m8Zku4DQFghI3yyfIoFgOQjXkXgLL2Peo=; b=Wi308mIHNSl1GtFeeZd4ixIlWxeNtlSOUy8vU0cXAOkqUfrHXgMbdWXWdSFKgkbAkefzZLWvmYgS0eSSh0fwFYcrcy1jiysrVBy9Smb71X0wtdzqs6Y6eLzmIiWF7PN8YdK8e/ElBVNbVlNLNnwOBzEbRBIqDNxX47eEilShn1bQbcDb3CQWj6CU2InnDdepAbBpu0Q1QmLf6VjXwcIakFp6sAD8lDQKOOcJkUDSf7PCH/hg4cB5wExbhqvWSbRuqe8wBUCEVkcyxzjq6chVYq3qTBz1+RfcloD5YQbC5rO9DvMxR0SoYdn7zK8Rlc4ZZ0uIN3oEM1zZgNFRJB+Jkg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=gOsNacS0o3m8Zku4DQFghI3yyfIoFgOQjXkXgLL2Peo=; b=y9aDVGxyYbdgvv5k0esVhCpOqI2P5+/MulGnMrY9h8vZoVwby8jAtj7GS/E8599kzaO/3a6GRDWvrzgvN5SGEG2lh/OpbxvV/gV9184LlV4DECMa4R3X04oV9IiHedj94EqFpa9XZlrw1C1IcGI8a025pdJdGPKONuVYKlbub4c= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Thomas.Lendacky@amd.com; Received: from DM6PR12MB3163.namprd12.prod.outlook.com (2603:10b6:5:15e::26) by DM6PR12MB3722.namprd12.prod.outlook.com (2603:10b6:5:1c3::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2772.18; Tue, 3 Mar 2020 14:20:31 +0000 Received: from DM6PR12MB3163.namprd12.prod.outlook.com ([fe80::f0f9:a88f:f840:2733]) by DM6PR12MB3163.namprd12.prod.outlook.com ([fe80::f0f9:a88f:f840:2733%7]) with mapi id 15.20.2772.019; Tue, 3 Mar 2020 14:20:31 +0000 Subject: Re: [edk2-devel] [PATCH v5 34/42] OvmfPkg/Sec: Add #VC exception handling for Sec phase To: Laszlo Ersek , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <555d15ee95133cce0617edbd5500c05785847a4c.1583190432.git.thomas.lendacky@amd.com> From: "Lendacky, Thomas" Message-ID: <621263d6-3714-eeee-7287-0a5f5d408641@amd.com> Date: Tue, 3 Mar 2020 08:20:29 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 In-Reply-To: X-ClientProxiedBy: SN6PR05CA0021.namprd05.prod.outlook.com (2603:10b6:805:de::34) To DM6PR12MB3163.namprd12.prod.outlook.com (2603:10b6:5:15e::26) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [10.236.30.74] (165.204.77.1) by SN6PR05CA0021.namprd05.prod.outlook.com (2603:10b6:805:de::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.5 via Frontend Transport; Tue, 3 Mar 2020 14:20:30 +0000 X-Originating-IP: [165.204.77.1] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 03b1eda5-aa9c-43d7-d53b-08d7bf7e077b X-MS-TrafficTypeDiagnostic: DM6PR12MB3722:|DM6PR12MB3722: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-Forefront-PRVS: 03319F6FEF X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4636009)(376002)(39860400002)(366004)(346002)(396003)(136003)(189003)(199004)(66946007)(66556008)(2616005)(54906003)(81156014)(8676002)(8936002)(31696002)(16576012)(956004)(86362001)(81166006)(36756003)(316002)(52116002)(2906002)(31686004)(53546011)(5660300002)(6486002)(4326008)(66476007)(26005)(16526019)(45080400002)(478600001)(186003)(966005);DIR:OUT;SFP:1101;SCL:1;SRVR:DM6PR12MB3722;H:DM6PR12MB3163.namprd12.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; Received-SPF: None (protection.outlook.com: amd.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 0eNSHWHcW1oKNoH6GDilEFPOD/sZnYxjR42+fivC+8erRne12/K5sF7yJNgsgqoBqNeHtcmPZUVR23lrugm/y2NcU/wfKqvAolmqppQ8j4V5SFSjI2tusGnqbbRgFSHi8lcwq23YljGEqvSIldhSPLMRTi3r0hXYiq1nUH+0U3voxWqC+d3aEVFDZP8xtZGBcSTuy2xz2N5Jl7u02u0c5ZnrKmqJI2dEJnQJAIQkBSj4uJh7YjgYgXQFcaIzbUtluwzSY5jinP6wfcOtobgrHIzFVEWzSv3YAJVmzVJtjsS7grcHEo8dpmvRldYyj/8evPH5mrz8pX3G/AJaf2eqXE6SCCny3e2v2N/xMtCZ4n65DYZeInDNt57g90Hf27Mqgl72RnVwM2VaqgZbgnnrrbKzstmpdl4b5ygnIG3gz6b7Nxy9gqmRDTunXFoqfxj7Fi6WSE1IkoZPhF16qQdNFJkXx6hZKzDFkDWNYTViCL4C4uDR1o/9vKJLr8ZotVrYc6kL4X6uvJluKfUY7ienMQ== X-MS-Exchange-AntiSpam-MessageData: 8bUi0MyL6GxOR+VQJtzGZ07is7V1nRAUZuRk1FcwtOXlQBGGK1dd/pkI8sFkxZ7URCXGGWV0KrReOCQe1iIYCIAYrnMPtcZfbrCwiPCnldSln5284hR5yGK9pUBOFwYo0R3DyFiG19fSkB3CW5o4Mg== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 03b1eda5-aa9c-43d7-d53b-08d7bf7e077b X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Mar 2020 14:20:31.0214 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: z5Qo6AnyrXueT2173rQd4HYHa0DxTjKJm6bqoMxKogXbq0QEQsoDBBF3f12gHnJhPxm1X6D5SS31sz9ANM9zsw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3722 Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit On 3/3/20 7:29 AM, Laszlo Ersek wrote: > Hi Tom, > > On 03/03/20 00:07, Lendacky, Thomas wrote: >> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7C3fd18aafb8934669131408d7bf76e70a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637188389716613512&sdata=uGe95zCVx0egVqiWYDqpAeS3k5aO5ZU6eHiyx8fpKxo%3D&reserved=0 >> >> An SEV-ES guest will generate a #VC exception when it encounters a >> non-automatic exit (NAE) event. It is expected that the #VC exception >> handler will communicate with the hypervisor using the GHCB to handle >> the NAE event. >> >> NAE events can occur during the Sec phase, so initialize exception >> handling early in the OVMF Sec support. >> >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Reviewed-by: Laszlo Ersek > > this patch has changed significantly since the last version (v4); I > think my R-b given for v4 should not have been picked up in v5. Sorry about that, missed that when I made this change. > > AFAICS the new stuff is SevEsProtocolFailure() and SevEsProtocolCheck(), > which -- per v5 blurb -- have moved from UefiCpuPkg to OvmfPkg: Correct. > > >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/Sec/SecMain.inf | 4 ++ >> OvmfPkg/Sec/SecMain.c | 151 ++++++++++++++++++++++++++++++++++++---- >> 2 files changed, 142 insertions(+), 13 deletions(-) >> >> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf >> index 63ba4cb555fb..7f78dcee2772 100644 >> --- a/OvmfPkg/Sec/SecMain.inf >> +++ b/OvmfPkg/Sec/SecMain.inf >> @@ -50,15 +50,19 @@ [LibraryClasses] >> PeCoffExtraActionLib >> ExtractGuidedSectionLib >> LocalApicLib >> + CpuExceptionHandlerLib >> >> [Ppis] >> gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED >> >> [Pcd] >> + gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >> index bae9764577f0..577596a949f9 100644 >> --- a/OvmfPkg/Sec/SecMain.c >> +++ b/OvmfPkg/Sec/SecMain.c >> @@ -24,6 +24,9 @@ >> #include >> #include >> #include >> +#include >> +#include >> +#include >> >> #include >> >> @@ -34,6 +37,10 @@ typedef struct _SEC_IDT_TABLE { >> IA32_IDT_GATE_DESCRIPTOR IdtTable[SEC_IDT_ENTRY_COUNT]; >> } SEC_IDT_TABLE; >> >> +typedef struct _SEC_SEV_ES_WORK_AREA { >> + UINT8 SevEsEnabled; >> +} SEC_SEV_ES_WORK_AREA; >> + >> VOID >> EFIAPI >> SecStartupPhase2 ( >> @@ -712,6 +719,90 @@ FindAndReportEntryPoints ( >> return; >> } >> >> +STATIC >> +VOID >> +SevEsProtocolFailure ( >> + IN UINT8 ReasonCode >> + ) >> +{ >> + MSR_SEV_ES_GHCB_REGISTER Msr; >> + >> + // >> + // Use the GHCB MSR Protocol to request termination by the hypervisor >> + // >> + Msr.GhcbPhysicalAddress = 0; >> + Msr.GhcbTerminate.Function = GHCB_INFO_TERMINATE_REQUEST; >> + Msr.GhcbTerminate.ReasonCodeSet = GHCB_TERMINATE_GHCB; >> + Msr.GhcbTerminate.ReasonCode = ReasonCode; >> + AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress); >> + >> + AsmVmgExit (); >> + >> + ASSERT (0); > > (1) The argument should be FALSE, not 0. Will fix. > >> + CpuDeadLoop (); >> +} >> + >> +STATIC >> +VOID >> +SevEsProtocolCheck ( >> + VOID >> + ) >> +{ >> + MSR_SEV_ES_GHCB_REGISTER Msr; >> + GHCB *Ghcb; >> + >> + // >> + // Use the GHCB MSR Protocol to obtain the GHCB SEV-ES Information for >> + // protocol checking >> + // >> + Msr.GhcbPhysicalAddress = 0; >> + Msr.GhcbInfo.Function = GHCB_INFO_SEV_INFO_GET; >> + AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress); >> + >> + AsmVmgExit (); >> + >> + Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB); >> + >> + if (Msr.GhcbInfo.Function != GHCB_INFO_SEV_INFO) { >> + SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL); >> + } >> + >> + if (Msr.GhcbProtocol.SevEsProtocolMin > Msr.GhcbProtocol.SevEsProtocolMax) { >> + SevEsProtocolFailure (GHCB_TERMINATE_GHCB_PROTOCOL); >> + } >> + >> + if ((Msr.GhcbProtocol.SevEsProtocolMin > GHCB_VERSION_MAX) || >> + (Msr.GhcbProtocol.SevEsProtocolMax < GHCB_VERSION_MIN)) { >> + SevEsProtocolFailure (GHCB_TERMINATE_GHCB_PROTOCOL); >> + } >> + >> + // >> + // SEV-ES protocol checking succeeded, set the initial GHCB address >> + // >> + Msr.GhcbPhysicalAddress = FixedPcdGet32 (PcdOvmfSecGhcbBase); >> + AsmWriteMsr64(MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress); >> + >> + Ghcb = Msr.Ghcb; >> + SetMem (Ghcb, sizeof (*Ghcb), 0); >> + >> + /* Set the version to the maximum that can be supported */ > > (2) The comment style is not consistent with the rest. Will change this. > > With the style warts (1) and (2) fixed: > > Reviewed-by: Laszlo Ersek Thanks, Tom > > Thanks > Laszlo > >> + Ghcb->ProtocolVersion = MIN (Msr.GhcbProtocol.SevEsProtocolMax, GHCB_VERSION_MAX); >> + Ghcb->GhcbUsage = GHCB_STANDARD_USAGE; >> +} >> + >> +STATIC >> +BOOLEAN >> +SevEsIsEnabled ( >> + VOID >> + ) >> +{ >> + SEC_SEV_ES_WORK_AREA *SevEsWorkArea; >> + >> + SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase); >> + >> + return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0)); >> +} >> + >> VOID >> EFIAPI >> SecCoreStartupWithStack ( >> @@ -737,8 +828,55 @@ SecCoreStartupWithStack ( >> Table[Index] = 0; >> } >> >> + // >> + // Initialize IDT - Since this is before library constructors are called, >> + // we use a loop rather than CopyMem. >> + // >> + IdtTableInStack.PeiService = NULL; >> + for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) { >> + UINT8 *Src, *Dst; >> + UINTN Byte; >> + >> + Src = (UINT8 *) &mIdtEntryTemplate; >> + Dst = (UINT8 *) &IdtTableInStack.IdtTable[Index]; >> + for (Byte = 0; Byte < sizeof (mIdtEntryTemplate); Byte++) { >> + Dst[Byte] = Src[Byte]; >> + } >> + } >> + >> + IdtDescriptor.Base = (UINTN)&IdtTableInStack.IdtTable; >> + IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1); >> + >> + if (SevEsIsEnabled ()) { >> + SevEsProtocolCheck (); >> + >> + // >> + // For SEV-ES guests, the exception handler is needed before calling >> + // ProcessLibraryConstructorList() because some of the library constructors >> + // perform some functions that result in #VC exceptions being generated. >> + // >> + // Due to this code executing before library constructors, *all* library >> + // API calls are theoretically interface contract violations. However, >> + // because this is SEC (executing in flash), those constructors cannot >> + // write variables with static storage duration anyway. Furthermore, only >> + // a small, restricted set of APIs, such as AsmWriteIdtr() and >> + // InitializeCpuExceptionHandlers(), are called, where we require that the >> + // underlying library not require constructors to have been invoked and >> + // that the library instance not trigger any #VC exceptions. >> + // >> + AsmWriteIdtr (&IdtDescriptor); >> + InitializeCpuExceptionHandlers (NULL); >> + } >> + >> ProcessLibraryConstructorList (NULL, NULL); >> >> + if (!SevEsIsEnabled ()) { >> + // >> + // For non SEV-ES guests, just load the IDTR. >> + // >> + AsmWriteIdtr (&IdtDescriptor); >> + } >> + >> DEBUG ((EFI_D_INFO, >> "SecCoreStartupWithStack(0x%x, 0x%x)\n", >> (UINT32)(UINTN)BootFv, >> @@ -751,19 +889,6 @@ SecCoreStartupWithStack ( >> // >> InitializeFloatingPointUnits (); >> >> - // >> - // Initialize IDT >> - // >> - IdtTableInStack.PeiService = NULL; >> - for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) { >> - CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate)); >> - } >> - >> - IdtDescriptor.Base = (UINTN)&IdtTableInStack.IdtTable; >> - IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1); >> - >> - AsmWriteIdtr (&IdtDescriptor); >> - >> #if defined (MDE_CPU_X64) >> // >> // ASSERT that the Page Tables were set by the reset vector code to >> >