From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (NAM04-BN3-obe.outbound.protection.outlook.com [40.107.68.54]) by mx.groups.io with SMTP id smtpd.web10.30151.1574369166079181703 for ; Thu, 21 Nov 2019 12:46:06 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=DeiPn1N7; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.68.54, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dkJKuS5hEy3m6Bd7EG6d5WQIMDVcDP+B5ixypOFxuNFVWF3w696ZAHf3YzCuRzAZhWraqkjBg35nTF1WH808n74miUS6jzrr3nLqkXhTlUYa8X/TFD0p3QM7LhPuJn2krE7H/tc3M9XQysw5J0Zm2JAJg11b/Dz1U8xsdnUWhK6C5dnqI6YZc2siLX0/Er80chDVc1ab8mZU3+7l9rHcf0H3HD2ubsJcyamzIvk+Co8yObGmVNyRg7rSKWRH8mRkysP92AXI75YvdGHlzv4Y6CYEXWJ8EXRI4nsIMuQUSRo+t6/BCP4VC2TUzImkOmZW8gHbmU3pJZZRwFiai1qkng== 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=6P0hEqX3Ad+XtHzIU8O7ZUSaQByMU7O36AqFJqeK9AY=; b=eVJ++fedJJRAxI+hFAGbXEpvsOMx4DwSCD8A8T1osAB6GUNp4hhTUbWoSErPZ9NpQPds1Mx6vJRfFDbzsLjyLDI2RVQSue7+v5+kyhAAs+N+gSxyVfhpSGbUW3PX0MWF4oW/0X/hjr4kNFlHowmIiUjm3oloGJkyUnkbJHNryvoVkQ8JFbK/MR57ZwP09CvI3y4k0ZSwcU5kHQrYk+37iLwOxiJE539AFZuicqoT+Ak7aWxxt0QiLcY9eGcrESUoIGlKBL4J/hp5vqjxrvUiC7hn0OuYDLdjAe/vbPVPMx8ChA4+0U28F/V49apGv5K8IFerHJKMvJGXEpCi+AtdyQ== 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=6P0hEqX3Ad+XtHzIU8O7ZUSaQByMU7O36AqFJqeK9AY=; b=DeiPn1N73/kEVMDoSOSJP4INVd7/lkJEj49J1uGB5UZcYDPDC0r9hwhZbTt6eqVzTP6KUYlgjxSNU/haYFa8EdIjlTaI9T//HYrPWKQQLy4teW9sGELK4l0KrwonU7ayi/xAGJtQFiJIPMhEL9X+RNMyEbNCab8RZG8+b0NrSa8= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Thomas.Lendacky@amd.com; Received: from DM6PR12MB3163.namprd12.prod.outlook.com (20.179.71.154) by DM6PR12MB4092.namprd12.prod.outlook.com (10.141.185.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2474.17; Thu, 21 Nov 2019 20:46:04 +0000 Received: from DM6PR12MB3163.namprd12.prod.outlook.com ([fe80::dd0c:8e53:4913:8ef4]) by DM6PR12MB3163.namprd12.prod.outlook.com ([fe80::dd0c:8e53:4913:8ef4%5]) with mapi id 15.20.2474.019; Thu, 21 Nov 2019 20:46:04 +0000 Subject: Re: [edk2-devel] [RFC PATCH v3 30/43] 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: <041cb6f6352c9e0f9982d316cd842c2daf57e7fa.1574280425.git.thomas.lendacky@amd.com> <12e35ff5-ff14-87d4-f4ac-604448289688@redhat.com> From: "Lendacky, Thomas" Openpgp: preference=signencrypt Message-ID: Date: Thu, 21 Nov 2019 14:46:02 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 In-Reply-To: <12e35ff5-ff14-87d4-f4ac-604448289688@redhat.com> X-ClientProxiedBy: DM6PR13CA0069.namprd13.prod.outlook.com (2603:10b6:5:134::46) To DM6PR12MB3163.namprd12.prod.outlook.com (2603:10b6:5:15e::26) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-Originating-IP: [165.204.77.1] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 925d1b9c-74dd-4d87-2cca-08d76ec3d360 X-MS-TrafficTypeDiagnostic: DM6PR12MB4092:|DM6PR12MB4092: X-MS-Exchange-PUrlCount: 1 X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-Forefront-PRVS: 0228DDDDD7 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4636009)(396003)(366004)(376002)(39860400002)(346002)(136003)(189003)(199004)(66476007)(53546011)(58126008)(6246003)(25786009)(3846002)(6116002)(26005)(6506007)(66946007)(31696002)(99286004)(386003)(186003)(316002)(23746002)(54906003)(14444005)(4326008)(65806001)(65956001)(47776003)(66066001)(36756003)(5660300002)(478600001)(66556008)(8936002)(52116002)(76176011)(6512007)(7736002)(11346002)(50466002)(81166006)(81156014)(446003)(86362001)(6486002)(14454004)(2906002)(31686004)(6436002)(6306002)(305945005)(2616005)(8676002)(229853002)(230700001)(966005);DIR:OUT;SFP:1101;SCL:1;SRVR:DM6PR12MB4092;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: DbhT8GBSoONiPL27xU3wLWHeZImNKJ8jhVmTqGt/DJP11Mf5DIFEdEZQzHI0A69ygRaR/FmOMWgTvXcJuaSne6WtSbvZaVWU+faaj8zGudI26y6nQYhlK+R57iwmaKe4JPBUhBL5lB8Kj0ZDxz0ONRhAfO1UmBzjj+bKt7+vbYLsbuVvnK8f80jqHmqfY2bIsr4JK3KjUkrD7rL7BwZh9wj48Olz6ZJuKl2HAFXYPsjhg5qr9psKSz3f8fFBUrJo1+t8DVxPd7fLW6MwkPg7CzFeaLf2Cg/dXGawLgGkeTAfk6lLsVb9R9M6ph+u3bJNwWKBjn6Mj6eut6bbJ9YrKOEvaVbtT6GTXP5tufzYRQLoInGfdNvcpnxTnyoJNXdZJ/F6oPsN3RUfxONqA8siLfWBmk4kjKsUNzNY/GICeBd/p3YQltsjoBMVgNvPigkXNlchuxwR3Vit7K+UiAzp7QBjHPc3RCHRv4e9v/LDsQM= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 925d1b9c-74dd-4d87-2cca-08d76ec3d360 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Nov 2019 20:46:04.5455 (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: sUOQrjcjf34qJnshnyjXv+e71R18qXPURtqLVn3+JHgXgZqDdIepODjgmuBzGLB7KF3vZkPoL1BEXXUl4AUNvQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4092 Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/21/19 6:06 AM, Laszlo Ersek wrote: > On 11/20/19 21:06, Lendacky, Thomas wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 >> >> 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 >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/Sec/SecMain.inf | 1 + >> OvmfPkg/Sec/SecMain.c | 29 ++++++++++++++++------------- >> 2 files changed, 17 insertions(+), 13 deletions(-) >> >> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf >> index 63ba4cb555fb..7f53845f5436 100644 >> --- a/OvmfPkg/Sec/SecMain.inf >> +++ b/OvmfPkg/Sec/SecMain.inf >> @@ -50,6 +50,7 @@ [LibraryClasses] >> PeCoffExtraActionLib >> ExtractGuidedSectionLib >> LocalApicLib >> + CpuExceptionHandlerLib >> >> [Ppis] >> gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED >> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >> index bae9764577f0..db319030ee58 100644 >> --- a/OvmfPkg/Sec/SecMain.c >> +++ b/OvmfPkg/Sec/SecMain.c >> @@ -24,6 +24,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -737,6 +738,21 @@ SecCoreStartupWithStack ( >> Table[Index] = 0; >> } >> >> + // >> + // 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); >> + >> + InitializeCpuExceptionHandlers (NULL); >> + >> ProcessLibraryConstructorList (NULL, NULL); >> >> DEBUG ((EFI_D_INFO, > > (1) The problem here is that we call multiple library APIs before > calling ProcessLibraryConstructorList() -- namely CopyMem(), > AsmWriteIdtr(), and InitializeCpuExceptionHandlers(). > > (See also the "SetMem" reference in the leading context, in the source > file -- it is not quoted in this patch.) > > Thus, would it be possible to move all the "+" lines, quoted above, just > below the ProcessLibraryConstructorList() call? Unfortunately, I can't. The invocation of ProcessLibraryConstructorList() results in #VC exceptions and so the exception handler needs to be in place before invoking ProcessLibraryConstructorList(). It looks like there are some SerialPort I/O writes to initialize the serial port and some PCI I/O reads and writes from AcpiTimerLibConstructor() in OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c. > > > (2) If possible I'd like to restrict the > InitializeCpuExceptionHandlers() call to SevEsIsEnabled(). > > (Unless you're implying that InitializeCpuExceptionHandlers() is useful > even without SEV-ES -- but then the commit message should be reworded > accordingly.) It does give you earlier exception handling and displays the exception information should an exception occur during SEC. But, it might require some tricks to somehow communicate from the ResetVector code to the SecMain code that SEV-ES is enabled. This is because you need to do a CPUID instruction to determine if SEV-ES is enabled, which will generate a #VC, which requires an exception handler... Thanks, Tom > > If you agree to that restriction, then I further suggest reordering this > patch against the next one: > > [edk2-devel] [RFC PATCH v3 31/43] > OvmfPkg/Sec: Enable cache early to speed up booting > > Namely, if you put that patch first, then in the present patch you can > extend the already existing SevEsIsEnabled()-dependent scope, with a > call to InitializeCpuExceptionHandlers(). > > The end result would be something like: > > ProcessLibraryConstructorList(); > > // > // Initialize IDT > // ... > // > > if (SevEsIsEnabled()) { > // > // non-automatic exit events (NAE) can occur during SEC ... > // > InitializeCpuExceptionHandlers (NULL); > > // > // Under SEV-ES, the hypervisor can't modify CR0 ... > // > AsmEnableCache (); > } > > What's your opinion? > > Thanks! > Laszlo > >> @@ -751,19 +767,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 >> >