From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (NAM12-DM6-obe.outbound.protection.outlook.com [40.107.243.67]) by mx.groups.io with SMTP id smtpd.web12.54179.1590510719040680129 for ; Tue, 26 May 2020 09:31:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=VMvDnIsA; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.243.67, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZFp32+PLwR/aaP73K32c5JR4i8dr4oZO9qFMuwyQm3KwSfsJB/oXuzH3QzcH9sl+lMIrrjKbhOzN0HfRvdmNcJTTe2+GoBZEl+BmwE+9i7bF+rwpEGtac3ICoc8+3ZdH6UMz79MTsIReLd27/+V0ZmX/DiJqoQaTuOj9vo/pMecIz7F+WBjBmmVl9NaxhTRYrgnETOQh1SATev/6gYR2g9oHSPZGXt2mwrQSBKgWqMnZBKRc5C3ggV3iovRYY0rNxOUXBLNN/xGaU4vDtLui27ggDU/L+FeL/09tmQaOkzgzniIiSHlHcYG04muZfJKeSurB8o2dDmGoueQscDcHzQ== 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=Hgo6zdvgUhG8SFwXgOU2p+J2GRC5PZ9arlwGaKCjf+A=; b=Eirk3D0h4M62FTAFmVdidBwRpFIK9yA3Yi/GLUzbZKnTX0NjX0238gAQyFfI8wj3TcZVUphFqA6Vbsj8TCXaPmi+ZO1voDrQm0vOh0tBJP+qpHtwNzm2fEeCLPwxR1hiyUIcnIrvoXm64CsefaXP4atrKMOmVS5eladxTDWS5FEt3hw9Cx2VAODPizW/8mhlD7cPDVHw9zOvhEgmyEbeGL5rrk3+9TwJ2uNvkW+X3D++HK2yJv/pmmX25K7+wKfYu7HZz9+SXZQiVkXjgzMylGTcNa865keO5dgu9RhRPqFTWur3vc/ssCbrH0MZHAB+bXv53fiy44AwSj12gEQFdw== 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=Hgo6zdvgUhG8SFwXgOU2p+J2GRC5PZ9arlwGaKCjf+A=; b=VMvDnIsA6uySGO89Wxqg+FK3KFAuIV3UQPmvoBMxGnWNxaiRphRSMKrOBVyeu4vsTa/4a8WIc2pXo7mRQJX176MnkapYhZYuEX++2/f4FosYZ4P6zBRyAfYY8GsChKXKJjK4QHcbEoYalbnhqOpim69H6pPyZMBKJQ6h+7tfxRc= Authentication-Results: amd.com; dkim=none (message not signed) header.d=none;amd.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM5PR12MB1516.namprd12.prod.outlook.com (2603:10b6:4:5::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3021.27; Tue, 26 May 2020 16:31:57 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4ce1:9947:9681:c8b1]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4ce1:9947:9681:c8b1%10]) with mapi id 15.20.3021.029; Tue, 26 May 2020 16:31:57 +0000 Subject: Re: [edk2-devel] [PATCH v8 36/46] OvmfPkg/ResetVector: Add support for a 32-bit SEV check To: Laszlo Ersek , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <337fcec3-28bc-6498-1b73-0b4c7409d01d@redhat.com> From: "Lendacky, Thomas" Message-ID: <349ca934-2fcf-020c-ddb9-998a3de5e2ad@amd.com> Date: Tue, 26 May 2020 11:31:55 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 In-Reply-To: <337fcec3-28bc-6498-1b73-0b4c7409d01d@redhat.com> X-ClientProxiedBy: SN2PR01CA0071.prod.exchangelabs.com (2603:10b6:800::39) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN2PR01CA0071.prod.exchangelabs.com (2603:10b6:800::39) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3021.26 via Frontend Transport; Tue, 26 May 2020 16:31:56 +0000 X-Originating-IP: [67.79.209.213] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: e3a50986-292e-4a0d-89a2-08d801924ed0 X-MS-TrafficTypeDiagnostic: DM5PR12MB1516: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:5797; X-Forefront-PRVS: 041517DFAB X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: LQoS5W16ajAnhs0xL1GVFri+CYL9/7LoKjMEYnR2kfsEl4bTuTFtyhL3pbrBEAgiOLqIuaXSOTB8pXRgNjMXB+/af3KysDjPEHz4vgbylYnK0F98uuoqY+vzHXT/FZdc8W4dq44IkZ1gmT00cRmwojyJvxb8vNpewnUyNHR59hCs0obc7AltFmzua0eiL98nDn2EgkpMbKXgBi20WKR+kKChuEoEWbyNyJlDYL1L7lPhAmRAd+wOUJukcalDqYw3eVVtdZ24ex3D2whGn+3y0pf48NDIRObkNIiGk1SuCn75CnSawgZAHi/XBj5LzxmrtFOO4uZwo9mssW9wPTDpHkGPMiyi3wKjGxlb2wqOhDFPAAXApxF4uEcs0Hu3o4+ipBIfz8jYcB7VFqGFfzptWieT9kjFDuI00SFbV38ExnzeTGjJHNpzl5JkkU1rLjxcYTXDKx61B1App/53mqd0Uw== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(39860400002)(396003)(136003)(376002)(346002)(366004)(186003)(26005)(31696002)(16526019)(66476007)(53546011)(6506007)(52116002)(66556008)(956004)(2906002)(66946007)(31686004)(45080400002)(478600001)(2616005)(54906003)(316002)(6512007)(6486002)(966005)(8936002)(36756003)(4326008)(86362001)(5660300002)(8676002)(19627235002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: yjVbF2wgQusKr70oq4Rl9AQUcazuDKVyT+DPlPtS4kJsEfoCFk/S38b/4mQMOdgB+uHDHPUa5aQ3+vzIzajf6vkO1Qso2EnibaaiYj4ZnMFGc/zH7Tm1ihvkx+8lXTinm+l9ofy7eUwgKO/uFeMpMejXD79j25eGwmoy+5OoCJYuqeO71q28QEFEldVDU/ito1VJMZ3u7B4X3/pbGbADS7YQN+Vq/pPFZjgE2buLfF0o0UpNS+5TQxriQnryaPYpE65YbQe1mCHsrJM1kIz5L4I4t2RVbDnXwL2tfMYdV70fpO0+5X0E3Xt78Ie3XwlIpDei/jPIZAheebG7makBSx3I+bS8/9USnXvNWgaPF/+ZqrmniIifZxpZN+6NSX0jy57fZDlVHU5sxlSGvGtDtNtfWJ/fOTupY1aevcyRxqrEnTyBiywqofN52I578I8mAWkPrOqeE4LSLsPqJDt7yju00XX2r8INFIzr9t++6/Q= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: e3a50986-292e-4a0d-89a2-08d801924ed0 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 May 2020 16:31:57.4526 (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: wCz+28lGpHKBSMe+gzTSZzucqsWxWxov5WK7GuEQfWSYFootIDUggYX80TisneG2pZWA00SUaYjp8JjOTwtX9g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1516 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 5/25/20 11:50 AM, Laszlo Ersek wrote: > Tom, > > On 05/19/20 23:51, 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%7C4fe7191ef9fe43793eb408d800cbbb44%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637260222303712873&sdata=DJP9%2Fbe1ttZ%2FGEwqZ2Flv5z4dTV0T8QkYdeSnaIGWcY%3D&reserved=0 >> >> During BSP startup, the reset vector code will issue a CPUID instruction >> while in 32-bit mode. When running as an SEV-ES guest, this will trigger >> a #VC exception. >> >> Add exception handling support to the early reset vector code to catch >> these exceptions. Also, since the guest is in 32-bit mode at this point, >> writes to the GHCB will be encrypted and thus not able to be read by the >> hypervisor, so use the GHCB CPUID request/response protocol to obtain the >> requested CPUID function values and provide these to the guest. >> >> The exception handling support is active during the SEV check and uses the >> OVMF temporary RAM space for a stack. After the SEV check is complete, the >> exception handling support is removed and the stack pointer cleared. >> >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Reviewed-by: Laszlo Ersek >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/ResetVector/ResetVector.inf | 2 + >> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 329 +++++++++++++++++++--- >> OvmfPkg/ResetVector/ResetVector.nasmb | 1 + >> 3 files changed, 294 insertions(+), 38 deletions(-) > > this doesn't work for me. > > Under your v5 posting, I reviewed those OvmfPkg patches that still > needed my review. > > The v6 posting carried all my R-b's; all OvmfPkg patches had been > reviewed. I trusted you and I only verified the commit messages for my > R-b's. I thought the OvmfPkg state was final. > > The v7 posting again carried my R-b's; I briefly checked the v6->v7 > changes in the blurb, and re-checked my R-b's on the OvmfPkg patches. > This was in the v7 blurb: > >> Changes since v6: >> - Add function comments to all functions, including local functions >> - Add function parameter direction to all functions (in/out) >> - Add support for MMIO MOVZX/MOVSX instructions >> - Ensure the per-CPU variable page remains encrypted >> - Coding-style fixes as identified by Ecc > > This summary didn't indicate I'd have to go through the OvmfPkg patches > again -- and the presence of my R-b's on all the OvmfPkg patches > supported that impression. > > I commented on v7 only later, independently; namely on two topics: > > - on one of the S3 reservation aspects, > - on the upcoming / requested movement of VmgExitLib to OvmfPkg. > > These were the two updates I was going to expect in v8. > > So, in order to "page in" your work again, in preparation for reviewing > v8, I decided to review the v5->v6 changes in more detail -- the code > too (incrementally), not just the picking up of my R-b's, like I had > originally done under v6. I was happy with v6, after performing this > review; see . > > Now I'm reviewing the differences (incrementally from v6 to v8), and I'm > shocked how many changes you incorporated into preexistent patches, > while keeping my R-b's. My apologies for this. I was experimenting with cleaning things up and making the code more readable and I guess I forgot to either remove it or note it as a change to be re-reviewed - thinking I had done one or the other. > > On this patch, you significantly changed the logic from v6 to v7, and I > don't have the slightest clue why. I don't feel inclined to > reverse-engineer the logic change from the v6->v7 interdiff. The right > way to present a significant change is to (a) drop the existent R-b's > from the patch, and (b) spell out the news in the blurb and/or in the > "notes" section of the individual patch. If you had dropped the R-b in > v7, then I would have known to review the changes in v7 at once (rather > than let it accumulate to v8). And if you had explained the updates, I > may have started with a re-review of the patch from scratch (and > wouldn't be stuck with an incremental one / interdiff now, between v6 > and v8). > > Then, the patch changed *again*, from v7 to v8; and my R-b (which only > applied to v6) got carried forward again. > > Consider the v7->v8 changes noted in the blurb: > >> Changes since v7: >> - Reserve the SEV-ES workarea when S3 is enabled >> - Fix warnings issued by the Visual Studio compiler >> - Create a NULL VmgExitLib instance that is used for VMGEXIT >> related operations as well as #VC handling. Then create the full >> VmgExitLib support only in OvmfPkg - where it will be used. This >> removes a bunch of implementation code from platforms that will >> not be using the functionality. >> - Remove single use interfaces from the VmgExitLib (VmgMmioWrite >> and VmgSetApJumpTable) > > Not a word on this patch, as far as I can see. > > I don't even know what to do about this patch now. I'd be really unhappy > to review it from zero; it's a difficult one. The reset vector is also > shared with non-SEV X64, so it's not like I can just slap an Acked-by on > it. > > (1) Unless there was an actual bug in the v6 version of this patch, > please let's go back to that. IOW, if the v6->v8 changes are only > cleanups or optimizations, let's please postpone them. Yes, I'll revert it back to the original version. Again, sorry for all of this churn. Thanks, Tom > > I'm going to take a walk now. > > Laszlo >