On 10/02/19 17:15, Laszlo Ersek wrote: > Adding Phil. > > I'm looking at this patch only because one thing caught my attention in > the previous one, "OvmfPkg: Add support for SEV-ES AP reset vector > re-directing": > > On 09/19/19 21:53, Lendacky, Thomas wrote: >> From: Tom Lendacky <thomas.lenda...@amd.com> >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 >> >> Typically, an AP is booted using the INIT-SIPI-SIPI sequence. This >> sequence is intercepted by the hypervisor, which sets the AP's registers >> to the values requested by the sequence. At that point, the hypervisor can >> start the AP, which will then begin execution at the appropriate location. >> >> Under SEV-ES, AP booting presents some challenges since the hypervisor is >> not allowed to alter the AP's register state. In this situation, we have >> to distinguish between the AP's first boot and AP's subsequent boots. >> >> First boot: >> Once the AP's register state has been defined (which is before the guest >> is first booted) it cannot be altered. Should the hypervisor attempt to >> alter the register state, the change would be detected by the hardware >> and the VMRUN instruction would fail. Given this, the first boot for the >> AP is required to begin execution with this initial register state, which >> is typically the reset vector. This prevents the BSP from directing the >> AP startup location through the INIT-SIPI-SIPI sequence. >> >> To work around this, provide a four-byte field at offset 0xffffffd0 that >> can contain an IP / CS register combination, that if non-zero, causes >> the AP to perform a far jump to that location instead of a near jump to >> EarlyBspInitReal16. Before booting the AP for the first time, the BSP >> should set the IP / CS value for the AP based on the value that would be >> derived from the INIT-SIPI-SIPI sequence. > > I don't understand how this can work: the guest-phys address 0xffffffd0 > is backed by read-only pflash in most OVMF deployments. > > In addition: > > [...] > >> @@ -1002,6 +1204,7 @@ WakeUpAP ( >> CpuMpData->InitFlag != ApInitDone) { >> ResetVectorRequired = TRUE; >> AllocateResetVector (CpuMpData); >> + AllocateSevEsAPMemory (CpuMpData); >> FillExchangeInfoData (CpuMpData); >> SaveLocalApicTimerSetting (CpuMpData); >> } >> @@ -1038,6 +1241,15 @@ WakeUpAP ( >> } >> } >> if (ResetVectorRequired) { >> + // >> + // For SEV-ES, set the jump address for initial AP boot >> + // >> + if (CpuMpData->SevEsActive) { >> + SEV_ES_AP_JMP_FAR *JmpFar = (SEV_ES_AP_JMP_FAR *)0xFFFFFFD0; >> + >> + JmpFar->ApStart.Rip = 0; >> + JmpFar->ApStart.Segment = (UINT16) (ExchangeInfo->BufferStart >> 4); >> + } > > Even if the address is backed by a single "unified" pflash, mapped r/w > -- which we can call a "non-standard OVMF deployment" nowadays --, a > normal store doesn't appear sufficient to me. The first write to pflash > will flip it to "programming mode", and the values stored are supposed > to be pflash commands (not just the raw data we intend to put in place). > > See for example the QemuFlashWrite() function in > "OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c". (But, again, that > is used with the pflash chip that hosts the variable store, and is > therefore mapped r/w.) > > > Taking a step back... I don't think APs execute any code from pflash, > when MpInitLib boots them. > > In OVMF, PcdCpuApLoopMode is ApInHltLoop (value 1), therefore > "CpuMpData->WakeUpByInitSipiSipi" should be TRUE, and > "ResetVectorRequired" too should be TRUE, at first AP boot. > Consequently, the reset vector seems to be allocated with > AllocateResetVector(). > > AllocateResetVector() has separate implementations for PEI and DXE, but > in both cases, it returns RAM. So I don't see where the AP accesses (or > executes) pflash.
... I believe I understand that this is precisely what cannot work under SEV-ES -- because we cannot launch an AP at an address that's dynamically chosen by the firmware (and passed to the hypervisor), like with INIT-SIPI-SIPI. And so firmware and hypervisor have to agree upon a *constant* AP reset vector address, in advance. We have two options: - pick the reset vector address *constant* such that it falls into RAM, or - let the AP reset vector reside in pflash, but then the code in pflash has to look for a parameter block at a fixed address in RAM. So in the end, both options require the same -- we need a RAM address constant that is determined at firmware build time. Either for the reset vector itself, or for the reset vector's parameter block. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48377): https://edk2.groups.io/g/devel/message/48377 Mute This Topic: https://groups.io/mt/34203585/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
