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.web11.366.1574438770951237482 for ; Fri, 22 Nov 2019 08:06:11 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=FxmmG4Da; 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=1574438769; 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=69RiCgsQjkTigjztyD2AYdDrmex+sAQRHjV73dNSvMA=; b=FxmmG4DatY+WKJr8ZgmCcxRTVt+v1J/jOdgkFN78DOqlkaczFWeS4o6aQ7OhFqb1oMO0ld huf2txvRgzb0oMBP9Sp58NV3byllG+NWuD95d5Mk+328K5axsc8+8pYpq/BM3g9hnsaolM 7a5i392oql5CjxjvGzjG0VrMbTToywo= 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-179-bUujj9K7PgW96K7cpvClhQ-1; Fri, 22 Nov 2019 11:06:06 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3037A801E58; Fri, 22 Nov 2019 16:06:04 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.23]) by smtp.corp.redhat.com (Postfix) with ESMTP id D585E5DDB2; Fri, 22 Nov 2019 16:06:01 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH v3 37/43] OvmfPkg: Reserve a page in memory for the SEV-ES AP reset vector To: Tom Lendacky , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <706f17aa0ea3ed3f57c1e933e93164a94ba1a0cf.1574280425.git.thomas.lendacky@amd.com> <85a5bcf3-25e2-7298-bca6-cef09cdb5d47@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 22 Nov 2019 17:06:00 +0100 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.14 X-MC-Unique: bUujj9K7PgW96K7cpvClhQ-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 11/21/19 23:49, Tom Lendacky wrote: > On 11/21/19 1:27 PM, Laszlo Ersek wrote: >> On 11/20/19 21:06, Lendacky, Thomas wrote: >>> +; sevEsResetBlock: >>> +; For the initial boot of an AP under SEV-ES, the "reset" RIP must be >>> +; programmed to the RAM area defined by SEV_ES_RESET_IP. A known offset >>> +; and GUID will be used to locate this block in the firmware and extract >>> +; the build time RIP value. The GUID must always be 48 bytes from the >>> +; end of the firmware. >>> +; >>> +; 0xffffffca (-0x36) - IP value >>> +; 0xffffffcc (-0x34) - CS selector value >> >> (5) I think the documentation of these four bytes is incorrect. >> (Similarly in the previous patch, in the commit message.) We populate >> these bytes with the *linear address* of the "reset trampoline" where >> the host will have to boot the AP for the first time. It's not >> expressed in real mode CS:IP notation / meaning. > > This is correct. The code will be in "big real mode" as you note below > (the reset vector is actually 0xfffffff0, just below 4GB and running > in 16-bit mode). > > If you look in Qemu, target/i386/cpu.c, x86_cpu_reset(), you'll see > where the CS segment is being loaded with a base of 0xffff0000 and the > eip gets loaded with 0xfff0. That still doesn't explain the above field names to me ("IP value" and "CS selector value"). Once the firmware image is built, we have: - CS: 0x0080 - IP: 0xB000 I don't understand the CS=0x80 part as - either a genuine segment selector - or the linear segment base address 0x800 (formed per real-address mode). > So the change to Qemu takes the "CS selector value" and effectively > left shifts it 16-bits to set the base This logic seems correct, as it translates 0x80 to 0x80_0000, i.e. 8 MB. However, I still think it's wrong to call the 16-bit value 0x0080 that we store at 0xffff_ffcc a "CS selector value". It is not a CS selector value, because it neither points into a segment descriptor table, nor is it left-shifted by 4-bits to form a linear base address, as in real-address mode. I would call this field most significant 16 bits of the CS base address [1] > and sets the eip to the "IP value" (in actuality, Qemu reads this as a > 32-bit value starting at the IP value and ANDs it with 0xffff0000 to > get the base and 0x0000ffff to get the eip. That seems correct to me. So... to summarize my point (5), *all* I'm asking for is to change the field description from "CS selector value" to [1] (or something to the same effect). > >> >> >> (6) Which brings me to my main point... Value 0x00B000 (for the >> "base" PCD) is relative to the base address of FD.MEMFD, namely 8MB >> (see MEMFD_BASE_ADDRESS). >> >> IOW, the ultimate value of SEV_ES_RESET_IP will be 0x0x0080_B000. >> That address is larger than 1MB. > > Which is ok since Qemu will set up CS appropriately. > >> >> Therefore, the host will have to launch the AP (for the first time) >> above 1MB (in guest-phys address space). > > Which is also OK, just like the BSP was launched at 0xFFFF_FFF0. > >> >> How can that work, if the AP is supposed to start in real mode? The >> linear address 0x0x0080_B000 cannot be expressed in 16-bit real mode >> CS:IP notation at all. >> >> Does the hypervisor start the AP in "big real mode" (16-bit mode with >> 4GB segment limits, and CS containing a segment selector, not the >> actual segment base)? Actually, I have to retract my question (6), because here I got lost in the weeds. I keep forgetting the following quirk in the Intel SDM: 9.1.4 First Instruction Executed The first instruction that is fetched and executed following a hardware reset is located at physical address FFFFFFF0H. This address is 16 bytes below the processors uppermost physical address. The EPROM containing the software- initialization code must be located at this address. The address FFFFFFF0H is beyond the 1-MByte addressable range of the processor while in real-address mode. The processor is initialized to this starting address as follows. The CS register has two parts: the visible segment selector part and the hidden base address part. In real-address mode, the base address is normally formed by shifting the 16-bit segment selector value 4 bits to the left to produce a 20-bit base address. However, during a hardware reset, the segment selector in the CS register is loaded with F000H and the base address is loaded with FFFF0000H. The starting address is thus formed by adding the base address to the value in the EIP register (that is, FFFF0000 + FFF0H = FFFFFFF0H). The first time the CS register is loaded with a new value after a hardware reset, the processor will follow the normal rule for address translation in real-address mode (that is, [CS base address = CS segment selector * 16]). To insure that the base address in the CS register remains unchanged until the EPROM based software- initialization code is completed, the code must not contain a far jump or far call or allow an interrupt to occur (which would cause the CS selector value to be changed). I need to be reminded of this every once in a while! While reviewing your patch, I thought that in real mode, the CS base address played no part, only the CS selector did -- which would be left-shifted by 4 bits directly, to form the segment base, in real-address mode. However, the fact is that the CS base address *always* plays a part (even in real mode). The "left shifting by 4 bits" does not occur when the CS:IP *reference* is made, but when the CS register is *loaded* explicitly, in real-address mode. And, at reset, CS is *pre-loaded* - with selector=0xF000, and - (as a quirk!) with base=0xFFFF_0000 (and *not* with base=0xF_0000, as the selector would otherwise dictate in real-address mode). And the QEMU change is that, at first AP boot, the CS will be pre-loaded - with selector=0x80 (which is a complete happenstance -- it's irrelevant!), - and, importantly, with base=0x80_0000 (and *not* with base=0x800, as the selector would otherwise dictate in real-address mode). So, it's all clear now (as long as you can please confirm that my understanding is finally correct). Thus, question (6) falls away. >> ... I guess that would answer my question (6) -- and the last few >> patches in this series, before this one, *do* suggest "big real mode" >> to me --, but the documentation around (5), and in the commit message >> of the previous patch, still doesn't match that. > > I'll expand on the commit message to indicate that Qemu, or others, > must do something similar relative to the CS register setup. To summarize, my request would be: - in the previous patch, please replace, in the commit message, Reset CS with most significant 16 bits of the reset CS base address - in this patch, please replace, in the code comment, CS selector value with most significant 16 bits of the CS base address (I mean... you *could* keep the current language in both places, but then you would have to describe this *entirely new* quirk for forming the CS base address from the CS selector value. This new SEV-ES / QEMU method does not match *any* { CS selector --> CS base address } transformation from the SDM, so you'd have to be quite verbose in the documenation. Given that the CS selector value 0x80 is completely irrelevant here, and we only care about the ultimate CS base address 0x80_0000, I'd suggest defining the 16-bit field at 0xffffffcc in terms of the desired CS base address [1], and not once mentioning "selector".) Thanks! Laszlo