From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (NAM10-DM6-obe.outbound.protection.outlook.com [40.107.93.85]) by mx.groups.io with SMTP id smtpd.web11.988.1590529652318544795 for ; Tue, 26 May 2020 14:47:32 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=Km99X+2Q; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.93.85, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QdNKx6NPGA9S8ldLqrwfDAd47/IZYhGZ2U0o714BW9PGpPENQExYr3vELEMQFmGHUxHRlXc24aqvOoAjG+1Oh2BeKVb+/iRCkwliCQVde/JDRHm03gj10+7Cl3X+EbBJ969heprbM4Cfy8eNZwbA5dzgmyAvEQuDOsgOuj35cKkrSSHCRkuRzwV2V3EgQf3MTDKVU5CcHGT79Ika+PaUFgeAaZnwzv369/Wsn2kHKQNzFpMSMdl7bpkLa0y6VsWoZT0duZEUGLZq2JFj55ARqYJUJ8Gd94kGR08hZi+kblA7AQTpXMw2BksGOL/lJaLiktod59Ae/VNWC2V90Vms7g== 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=5/nbNyUYIsALzxTMDzfkzDLfjDWeB5PqUkKMWZXa624=; b=EVml8gTJWKHLEDLWllMnOyS2GEo8BDxKy6vmBpRr/oqBU/QPk27nbnmNmMOt/r8ic2MQl/kJ8X1CRyLfd3AYrOwHocYjDcqdZxyQl2LnVnAk+TaRpSBxIykcGRGsE5uDYDOlYN3FOYrEHcOABylHAZbT/uqRHEN1UNMhBopu5eiASaG40R3Ol+igPAknyADpW7Yku87k/ezzbmWtwHZOJ4YKdiwn9tCLY3S+X2ZEOrGrxHA4DqfC4B2p3y9xmU7h0dTcLtimzw5EfOKIlABA3BL6zRz6IOP0aO03axUwoyUy/vRqcLm5q9E7EsxoWVoCTAJUIldnJjBPyHmKQDFvjg== 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=5/nbNyUYIsALzxTMDzfkzDLfjDWeB5PqUkKMWZXa624=; b=Km99X+2QlFxCyW8eGQtxnYzqLy58q0RGi8midRYtOf5EiVtzYRNIzKEyFvgE4gxAIVJZAZG8xuQOHk9/wBKX2Kf0a6GImaC7hPLh0rBuuRflvzrgX3xVcSMw6ro6sKnzTalrQryNky3/OHFY86MAELXW2lnejUAvx+NFadviPD8= 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 DM5PR12MB1643.namprd12.prod.outlook.com (2603:10b6:4:11::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3045.17; Tue, 26 May 2020 21:47:30 +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 21:47:30 +0000 Subject: Re: [edk2-devel] [PATCH v8 34/46] OvmfPkg: Reserve a page in memory for the SEV-ES usage From: "Lendacky, Thomas" To: Laszlo Ersek , devel@edk2.groups.io CC: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <5f3a4f30804261206adde675b983f42b777dd5d8.1589925074.git.thomas.lendacky@amd.com> <0f3cdd22-189d-3980-7639-7e7f58f909cb@redhat.com> <69067fe2-093b-0699-f460-79c9b081440f@amd.com> Message-ID: Date: Tue, 26 May 2020 16:47:28 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 In-Reply-To: <69067fe2-093b-0699-f460-79c9b081440f@amd.com> X-ClientProxiedBy: SN6PR16CA0072.namprd16.prod.outlook.com (2603:10b6:805:ca::49) 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 SN6PR16CA0072.namprd16.prod.outlook.com (2603:10b6:805:ca::49) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3021.27 via Frontend Transport; Tue, 26 May 2020 21:47:29 +0000 X-Originating-IP: [67.79.209.213] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 3cb97793-cb91-495f-d0e3-08d801be63af X-MS-TrafficTypeDiagnostic: DM5PR12MB1643: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:8882; X-Forefront-PRVS: 041517DFAB X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Q3HiDhUhdLIWR/IOIx2tDmwueFHY1UQjVVFREOwJifZY8JBghlvlQFxvi5NymW3eX6LyM/gNUMZfUJOC/mpd/mq+1MBO7bRX7/lOed6KbRqgs4mcXDQ8sQXVVPEdq4ouwc13PkiaGyjUNPKFr0fbCaxP8W8hodvV4fyq5p6X6w7iJdfShebxHmTxR0JwMCUR8H9cA30WdnE0oFLYnMi5xllQy03b8jQRMwbA7EkyvTt3vf4M+EINlTov3AnJA9I6xBa9ICvwFaevbJblTAeYPKXlCewwdsz/j0R9fLsNsoLikvUmL8eghcnfWlblI378D7qgsRarj8MZkmEbS+q7TGQUJW92P9UDbbSkJW0+6av5Z9t0XU1VilR896DWOOMqZPgPiXOfKhWdnrWr2j0HD2qJVvuRX+j7Gzi+kM/V90rZ47jZHmD2HcPfXbn1L/nTMn0qYTFSsiEZub/hhN61uw== 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)(136003)(376002)(346002)(396003)(39850400004)(366004)(966005)(8676002)(6512007)(4326008)(8936002)(19627235002)(31696002)(6486002)(2906002)(45080400002)(36756003)(54906003)(316002)(478600001)(31686004)(26005)(956004)(2616005)(5660300002)(16526019)(186003)(66476007)(52116002)(66946007)(53546011)(86362001)(66556008)(6506007)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: k+ndJkAHd2jWMRsL1Fsxio8RdEKnBqTB/n+UEoX2vkX9ZHFItu7Kxb9Sg1LOozrvDfWMBaxXXGFMYeQTrf4uV8Jl+uhEvRSgTjRCcn+7Tx2bN2mxistU9UvZD1PjrSFLjl/4+01+YUO6+9VxMSBVZ7LY3YLE1frLfo4L3Dx/xzBDjpzilgSaDE34kWrSgE+k2L+d72jxgG7Sw04TRTbQmGq7HaX6MRa3ROV4homKEYQdy/vdbqfPxIi4RbT4hYvL6U0i/el32q2JKf8ooWlw1UNNjpV1L4Td+Fh6jIf6iOvnexkgPWShmRywAGPj+QuW/GsISa2JmOO7645khwyAL0uzESn22ZLa0/nufhNxatRasJ81ZkkZuPG9U64g0X+QQPKb+usjmt+zLa0VcyxyRLzV8+Koz+bB9jUidrfFZW4rKP1Y+xm91imthfH3ndE+LPvQtLv7UcUIEE2Yy1sF1MixBomL8flWt0DB2tHQWto= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3cb97793-cb91-495f-d0e3-08d801be63af X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 May 2020 21:47:30.4174 (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: XVws8uk/pnLRZLhU4mlqlVtfjVA0IPXqBXvHZ9DKPeHTVno8nn81bAaYjpxXNyj04F+OyV4f7WvRWeLzDLus0Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1643 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 5/26/20 9:28 AM, Tom Lendacky wrote: > On 5/25/20 11:00 AM, Laszlo Ersek wrote: >> On 05/19/20 23:51, Lendacky, Thomas wrote: >>> BZ:=20 >>> https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbug= zilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=3D02%7C01%7Cthomas.= lendacky%40amd.com%7C498df3e8d335449e596508d800c4c955%7C3dd8961fe4884e608e1= 1a82d994e183d%7C0%7C0%7C637260192476035384&sdata=3DUKux5gXwpNe59RKQHTyk= 577b%2B%2FBmTIdblij8JWhXBG4%3D&reserved=3D0=20 >>> >>> >>> Reserve a fixed area of memory for SEV-ES use and set a fixed PCD, >>> PcdSevEsWorkAreaBase, to this value. >>> >>> This area will be used by SEV-ES support for two purposes: >>> =C2=A0=C2=A0 1. Communicating the SEV-ES status during BSP boot to SEC: >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Using a byte of memory from the page, th= e BSP reset vector code can >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 communicate the SEV-ES status to SEC for= use before exception >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 handling can be enabled in SEC. After SE= C, this field is no longer >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 valid and the standard way of determine = if SEV-ES is active should >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 be used. >>> >>> =C2=A0=C2=A0 2. Establishing an area of memory for AP boot support: >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 A hypervisor is not allowed to update an= SEV-ES guest's register >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 state, so when booting an SEV-ES guest A= P, the hypervisor is not >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 allowed to set the RIP to the guest requ= ested value. Instead an >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SEV-ES AP must be re-directed from withi= n the guest to the actual >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 requested staring location as specified = in the INIT-SIPI-SIPI >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sequence. >>> >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Use this memory for reset vector code th= at can be programmed to have >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 the AP jump to the desired RIP location = after starting the AP. This >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 is required for only the very first AP r= eset. >>> >>> Cc: Jordan Justen >>> Cc: Laszlo Ersek >>> Cc: Ard Biesheuvel >>> Reviewed-by: Laszlo Ersek >>> Signed-off-by: Tom Lendacky >>> --- >>> =C2=A0 OvmfPkg/OvmfPkgX64.fdf=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |= =C2=A0 3 +++ >>> =C2=A0 OvmfPkg/ResetVector/ResetVector.inf=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 |=C2=A0 1 + >>> =C2=A0 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 11 +++++++++++ >>> =C2=A0 OvmfPkg/ResetVector/ResetVector.nasmb=C2=A0=C2=A0=C2=A0=C2=A0 |= =C2=A0 1 + >>> =C2=A0 4 files changed, 16 insertions(+) >>> >>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >>> index 88b1e880e603..8836b30a0cef 100644 >>> --- a/OvmfPkg/OvmfPkgX64.fdf >>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>> @@ -82,6 +82,9 @@ [FD.MEMFD] >>> =C2=A0 0x009000|0x002000 >>> =20 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGui= d.PcdOvmfSecGhcbSize=20 >>> >>> +0x00B000|0x001000 >>> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGu= id.PcdSevEsWorkAreaSize=20 >>> >>> + >>> =C2=A0 0x010000|0x010000 >>> =20 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSp= aceGuid.PcdOvmfSecPeiTempRamSize=20 >>> >>> diff --git a/OvmfPkg/ResetVector/ResetVector.inf=20 >>> b/OvmfPkg/ResetVector/ResetVector.inf >>> index 483fd90fe785..e94e1bfcce7e 100644 >>> --- a/OvmfPkg/ResetVector/ResetVector.inf >>> +++ b/OvmfPkg/ResetVector/ResetVector.inf >>> @@ -34,6 +34,7 @@ [BuildOptions] >>> =C2=A0=C2=A0=C2=A0=C2=A0 *_*_X64_NASMB_FLAGS =3D -I$(WORKSPACE)/UefiCpu= Pkg/ResetVector/Vtf0/ >>> =C2=A0 [Pcd] >>> +=C2=A0 gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase >>> =C2=A0=C2=A0=C2=A0 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase >>> =C2=A0=C2=A0=C2=A0 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >>> =C2=A0=C2=A0=C2=A0 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBa= se >>> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm=20 >>> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm >>> index c3587a1b7814..73a4eaadb1b6 100644 >>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm >>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm >>> @@ -89,6 +89,10 @@ SevExit: >>> =C2=A0 ; If SEV-ES is disabled then EAX will be zero. >>> =C2=A0 ; >>> =C2=A0 CheckSevEsFeature: >>> +=C2=A0=C2=A0=C2=A0 ; Initialize the first byte of the workarea to zero= to communicate to >>> +=C2=A0=C2=A0=C2=A0 ; the SEC phase that SEV-ES is not enabled. >>> +=C2=A0=C2=A0=C2=A0 mov=C2=A0=C2=A0=C2=A0=C2=A0 byte[SEV_ES_WORK_AREA],= 0 >>> + >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 xor=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = eax, eax >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ; SEV-ES can't be enabled if SEV isn't, = so first check the=20 >>> encryption >>> @@ -108,6 +112,13 @@ CheckSevEsFeature: >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ; Restore encryption mask >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mov=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = edx, ebx >>> +=C2=A0=C2=A0=C2=A0 test=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 eax, eax >>> +=C2=A0=C2=A0=C2=A0 jz=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 NoSevE= s >>> + >>> +=C2=A0=C2=A0=C2=A0 ; Set the first byte of the workarea to one to comm= unicate to the SEC >>> +=C2=A0=C2=A0=C2=A0 ; phase that SEV-ES is enabled. >>> +=C2=A0=C2=A0=C2=A0 mov=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 byte[SEV_ES= _WORK_AREA], 1 >>> + >>> =C2=A0 NoSevEs: >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 OneTimeCallRet CheckSevEsFeature >>> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb=20 >>> b/OvmfPkg/ResetVector/ResetVector.nasmb >>> index bfb77e439105..2967617bfaa0 100644 >>> --- a/OvmfPkg/ResetVector/ResetVector.nasmb >>> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb >>> @@ -72,6 +72,7 @@ >>> =C2=A0=C2=A0=C2=A0 %define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbP= ageTableBase)) >>> =C2=A0=C2=A0=C2=A0 %define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase= )) >>> =C2=A0=C2=A0=C2=A0 %define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize= )) >>> +=C2=A0 %define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase)) >>> =C2=A0 %include "Ia32/PageTables64.asm" >>> =C2=A0 %endif >>> >> >> The OvmfPkg/ResetVector modifications have been moved to this patch, at >> least in part, from patch "OvmfPkg/ResetVector: Add support for a 32-bit >> SEV check". >> >> And I don't understand why. >=20 > I was trying to keep everything logically grouped. The early use of this= =20 > area is to communicate the SEV-ES status to SEC and so logically I though= t=20 > that should be done when the area was introduced. >=20 >> >> I mean it's possible that setting the first byte of the work area to 1 >> does not belong in "OvmfPkg/ResetVector: Add support for a 32-bit SEV >> check". That's OK; then said manipulation of the work area should be >> split to its own patch, which I should then review afresh. >> >> What's not OK is to move code between two reviewed patches *and* keep my >> R-b on both. >=20 > Sorry about that. A bad assumption on my part about being able to do that= =20 > here and in a few other places. >=20 >> >> Please be more transparent about incremental changes. >> >> (1) Please revert this patch to its v7 state, and keep my R-b on it. >=20 > Will do. >=20 >> >> (2) Please split the ResetVector changes to a new patch. For the subject >> line, I suggest: >> >> OvmfPkg/ResetVector: communicate SEV-ES status to SEC before exceptions >> >> or something similar. >=20 > Will do. Actually, these changes can remain part of the revert to the v6 version of= =20 patch 36 ("OvmfPkg/ResetVector: Add support for a 32-bit SEV check") so=20 that no changes are seen in that patch from the original v6 that was review= ed. Thanks, Tom >=20 > Thanks, > Tom >=20 >> >> Thanks >> Laszlo >>