Re: [edk2-devel] edk2 master: AMD SEV-ES guest boot with OvmfPkgX64 fails

["Ard Biesheuvel via groups.io" <ardb=kernel.org@groups.io>]

On Tue, 28 Jan 2025 at 23:38, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 1/28/25 14:57, Lendacky, Thomas via groups.io wrote:
> > On 1/28/25 10:26, Ard Biesheuvel via groups.io wrote:
> >> Please retry with a build created from the latest HEAD. There was a
> >> bug in that change that got fixed today.
> >
> > I tried the latest HEAD and the issue is still there.
> >
> > On a whim, I deleted the added DEBUG () calls from the patch and the
> > issue went away. At some point during efi_set_virtual_address_map(),
> > RelocBase and RelocBaseEnd are NULL and "DEBUG ((DEBUG_ERROR,
> > "Relocation block is not valid\n"));" is executed, which crashes the
> > boot. I'm guessing there shouldn't be any output generated during
> > SetVirtualAddressMap ?
>
> After some digging, the reason for the failure is the attempted DEBUG()
> print.
>
> The DEBUG() ends up generating an IO instruction. The IO instruction
> generates a #VC which is handled by the Linux kernel. To validate that
> the IOIO error code truly came from an IO instruction, the #VC handler
> attempts to read the instruction bytes. To read the bytes, the #VC
> handler first determines if the context is kernel or userspace. In this
> case, the context is kernel, so the instruction bytes are accessed using
> copy_from_kernel_nofault(). However, the RIP that is used is the EFI
> identity-mapped value, 0x7f6e1331, which appears as a userspace address,
> so it fails the check in copy_from_kernel_nofault_allowed().
>
> At that point the #VC handler simulates a #PF by using information from
> the #VC exception and forwarding it to the #PF handler.
>
> The easiest and most backward compatible fix would be to remove the
> newly added DEBUG() invocations from: aedcaa3df8a2 ("MdePkg: Fix
> overflow issue in PeCoffLoaderRelocateImageForRuntime").
>

Thanks for the analysis.

This will break on other platforms too (e.g., ARM) where serial output
is emitted using MMIO regions that are not in the EFI_MEMORY_RUNTIME
set. So I agree that this code should not use DEBUG() prints at all.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#121060): https://edk2.groups.io/g/devel/message/121060
Mute This Topic: https://groups.io/mt/110856145/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-