From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.10064.1588691746862959239 for ; Tue, 05 May 2020 08:15:48 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dpOoTbk4; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588691746; 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=BhU7Btb/KQG8VhxO64wkHl97HgAdB/d4cpG3Cpj2zqw=; b=dpOoTbk4IOOtrWpRHF6+CvIVjoEJp1iZsjnWS3pjmWpL5yhqcLDeBkDDF0B0/g/vemuZQw Ej//6145LazBrYiDyYcbmF9H973HwVUA8bw8H1BUkiuG7GKd9irSZQPJhKuf9Di7I7FZkc 9EXHLfryU9EITHiYSlMID4j1MSqP46c= 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-435-gXnZg5XyNYiQQHISOHJR-Q-1; Tue, 05 May 2020 11:15:25 -0400 X-MC-Unique: gXnZg5XyNYiQQHISOHJR-Q-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 05F591800D4A; Tue, 5 May 2020 15:15:24 +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 E90735C1B2; Tue, 5 May 2020 15:15:21 +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: <8e4574e0-ca8f-87bc-f8e8-9c7c6f696c21@redhat.com> Date: Tue, 5 May 2020 17:15:21 +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: <4a86e0f1-48d2-31bb-7e5a-faf41f3c4a3a@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 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 04/30/20 23:12, Tom Lendacky wrote: > On 4/30/20 1:58 PM, Laszlo Ersek wrote: >> Hi Tom, >=20 > Hi Laszlo, >=20 >> >> On 04/22/20 19:41, Lendacky, Thomas wrote: >>> BZ: >>> 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%7Cce256f35aa2e4748e8e008d7ed3874ae%7C3dd8961fe4884e608e1= 1a82d994e183d%7C0%7C0%7C637238699042461059&sdata=3DtXX8nkBo3fB4OVTs2ave= vW8pwL6AcqJHvFhvlshKySI%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 vec= tor code >>> can >>> =A0=A0=A0=A0=A0 communicate the SEV-ES status to SEC for use before exc= eption >>> =A0=A0=A0=A0=A0 handling can be enabled in SEC. After SEC, this field i= s no longer >>> =A0=A0=A0=A0=A0 valid and the standard way of determine if SEV-ES is ac= tive 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 hypervis= or is not >>> =A0=A0=A0=A0=A0 allowed to set the RIP to the guest requested value. In= stead 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-SIP= I-SIPI >>> =A0=A0=A0=A0=A0 sequence. >>> >>> =A0=A0=A0=A0=A0 Use this memory for reset vector code that can be progr= ammed 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|gUefiOvmfPkgTokenSpaceGui= d.PcdOvmfSecGhcbSize >>> >>> =A0 +0x00B000|0x001000 >>> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGu= id.PcdSevEsWorkAreaSize >>> >>> + >>> =A0 0x010000|0x010000 >>> =A0 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSp= aceGuid.PcdOvmfSecPeiTempRamSize >>> >>> =A0 >> >> 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? >=20 > Nice catch! Yes, I missed this one. >=20 >> >> >> 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 immediately >> 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 (at >> 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? >=20 > Yes, that is correct. So I just need to do the same thing for this area > that I did in patch #29. >=20 > I can probably shift patch #29 after #33 and have one patch for the S3 > reservation instead of having two separate patches doing S3 reservation. Sounds good, thanks! Laszlo