Re: [edk2-devel] [RFC PATCH v3 37/43] OvmfPkg: Reserve a page in memory for the SEV-ES AP reset vector

["Laszlo Ersek" <lersek@redhat.com>]

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