From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web11.10215.1588692320350802020 for ; Tue, 05 May 2020 08:25:20 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WwzaRoHR; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588692319; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=f05jTgwW7lQW9eet4DiCA+skmgxi07ePVOUQdxPQaxQ=; b=WwzaRoHRC3HQ188FRuMgpkcc/rNSxRJqX6s0dQErYhm1w/HDUut9CMDlX/vuoc4vJxTIWW BORJsWp+JXJ8iQxjxAIeJJ4Tg4lXqcR+/jQfbNPGBoP78porE0GXMr17Nl3MNxaLEa7CUO f5ZodJ0HR//3gzJa/Qu0qXy2PwConj4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-261-Kcs-BLq2OR-XJXbuZqLu8g-1; Tue, 05 May 2020 11:25:15 -0400 X-MC-Unique: Kcs-BLq2OR-XJXbuZqLu8g-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 27A35800687; Tue, 5 May 2020 15:25:14 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-134.ams2.redhat.com [10.36.114.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id F25B262963; Tue, 5 May 2020 15:25:11 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v7 33/43] OvmfPkg: Reserve a page in memory for the SEV-ES usage To: Tom Lendacky , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <458aea8874eaecec248c69a3ef809392226ad4e4.1587577317.git.thomas.lendacky@amd.com> <93f7386f-6e9e-52e1-4a81-d8b599687677@redhat.com> <4a86e0f1-48d2-31bb-7e5a-faf41f3c4a3a@amd.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 5 May 2020 17:25:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 05/01/20 00:09, Tom Lendacky wrote: > On 4/30/20 4:12 PM, Tom Lendacky wrote: >> On 4/30/20 1:58 PM, Laszlo Ersek wrote: >>> Hi Tom, >> >> Hi Laszlo, >> >>> >>> On 04/22/20 19:41, Lendacky, Thomas wrote: >>>> BZ: >>>> https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbu= gzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=3D02%7C01%7Cthomas= .lendacky%40amd.com%7Cce256f35aa2e4748e8e008d7ed3874ae%7C3dd8961fe4884e608e= 11a82d994e183d%7C0%7C0%7C637238699042461059&sdata=3DtXX8nkBo3fB4OVTs2av= evW8pwL6AcqJHvFhvlshKySI%3D&reserved=3D0 >>>> >>>> >>>> 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: >>>> =A0=A0 1. Communicating the SEV-ES status during BSP boot to SEC: >>>> =A0=A0=A0=A0=A0 Using a byte of memory from the page, the BSP reset ve= ctor >>>> code can >>>> =A0=A0=A0=A0=A0 communicate the SEV-ES status to SEC for use before ex= ception >>>> =A0=A0=A0=A0=A0 handling can be enabled in SEC. After SEC, this field = is no >>>> longer >>>> =A0=A0=A0=A0=A0 valid and the standard way of determine if SEV-ES is a= ctive >>>> should >>>> =A0=A0=A0=A0=A0 be used. >>>> >>>> =A0=A0 2. Establishing an area of memory for AP boot support: >>>> =A0=A0=A0=A0=A0 A hypervisor is not allowed to update an SEV-ES guest'= s register >>>> =A0=A0=A0=A0=A0 state, so when booting an SEV-ES guest AP, the hypervi= sor is not >>>> =A0=A0=A0=A0=A0 allowed to set the RIP to the guest requested value. I= nstead an >>>> =A0=A0=A0=A0=A0 SEV-ES AP must be re-directed from within the guest to= the actual >>>> =A0=A0=A0=A0=A0 requested staring location as specified in the INIT-SI= PI-SIPI >>>> =A0=A0=A0=A0=A0 sequence. >>>> >>>> =A0=A0=A0=A0=A0 Use this memory for reset vector code that can be prog= rammed >>>> to have >>>> =A0=A0=A0=A0=A0 the AP jump to the desired RIP location after starting= the AP. >>>> This >>>> =A0=A0=A0=A0=A0 is required for only the very first AP reset. >>>> >>>> Cc: Jordan Justen >>>> Cc: Laszlo Ersek >>>> Cc: Ard Biesheuvel >>>> Reviewed-by: Laszlo Ersek >>>> Signed-off-by: Tom Lendacky >>>> --- >>>> =A0 OvmfPkg/OvmfPkgX64.fdf | 3 +++ >>>> =A0 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >>>> index 36414c1f8b49..a0bea86f9875 100644 >>>> --- a/OvmfPkg/OvmfPkgX64.fdf >>>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>>> @@ -82,6 +82,9 @@ [FD.MEMFD] >>>> =A0 0x009000|0x002000 >>>> =A0 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGu= id.PcdOvmfSecGhcbSize >>>> >>>> +0x00B000|0x001000 >>>> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceG= uid.PcdSevEsWorkAreaSize >>>> >>>> + >>>> =A0 0x010000|0x010000 >>>> =A0 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenS= paceGuid.PcdOvmfSecPeiTempRamSize >>>> >>>> >>> >>> in patch #28 ("OvmfPkg: Create a GHCB page for use during Sec phase"), >>> we carve out two ranges in FD.MEMFD, and introduce a matching set of >>> 4 PCDs. >>> >>> Then in patch #29 ("OvmfPkg/PlatformPei: Reserve GHCB-related areas if >>> S3 is supported"), we reserve those ranges from the OS, as AcpiNVS, if >>> S3 is supported. The reason we only reserve those ranges if S3 is >>> enabled because the ranges are only needed in SEC. (See the details in >>> the commit mesage of patch #29.) >>> >>> In this patch (patch #33), we carve out a third region in FD.MEMFD. We >>> don't seem to ever reserve it. I think that's minimally a problem for >>> S3; the same argument should apply as to the other two areas. Do you >>> agree? >> >> Nice catch! Yes, I missed this one. >> >>> >>> >>> Furthermore, I wonder if we should reserve this "work area" from the OS= , >>> and even from the DXE phase (!), *regardless* of S3. I can't immediatel= y >>> tell when it's the last time (with S3 disabled) when this area is used. >>> >>> As I understand it, it is only used the first time the APs are booted >>> up. And that should happen still in the PEI phase, because CpuMpPei >>> boots up all the APs and counts them. Afterwards (still in the PEI >>> phase), the APs should be sleeping in ApWakeupFunction(), namely in the >>> code added by patch #40 ("UefiCpuPkg: Allow AP booting under SEV-ES"). >>> If the AP is woken again, it is actually only "released" by the >>> hypervisor, and it goes through the special 64bit->16bit transition, >>> again implemented in patch#40. >>> >>> So ultimately it shouldn't be necessary to reserve this third region (a= t >>> PcdSevEsWorkAreaBase), if S3 is disabled, because it is never used past >>> the very first AP boot (which happens when CpuMpPei counts the APs). >>> >>> Do I understand right? >> >> Yes, that is correct. So I just need to do the same thing for this >> area that I did in patch #29. >=20 > I think I might want to protect the area from DXE allocations, too. OK. In such cases, the area in question is always covered with a memory allocation HOB; what depends on S3 is only whether the memory type is "boot services data" (S3 disabled on the QEMU cmdline) or AcpiNVS (S3 enabled). The region is then always kept away from DXE, and S3 enablement determines only whether the OS is told to stay away as well. > Is > there an easy way to detect that PEI is active vs DXE? Even so, will it > *always* be the case that PEI will start the APs first? I'd hate to see > a change down the road where PEI doesn't start the APs and then things > break. We'll have to continue including CpuMpPei, otherwise we couldn't re-set the feature control MSR to the desired value at S3 resume. IOW, removing CpuMpPei from OVMF would regress at least that feature: https://bugzilla.tianocore.org/show_bug.cgi?id=3D86 Including CpuMpPei in OVMF was surprisingly quirky; I don't see it going away anytime soon. Thanks Laszlo