From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web08.773.1606331294000045466 for ; Wed, 25 Nov 2020 11:08:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Bh1VmU7e; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1606331293; 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=axk/UcBi3f9JJJt3RuTp24vRQtulIHGoRitReW41ekc=; b=Bh1VmU7ehXePjutXcSSD6RcqQcW3m8d05CYvAWX5uZ6XgzEfCh8/LEsQgebEV0/RaUeFsC rzOPu1N5QBox3ZVMhH9f4m/SfR9po1/1GC3RosUmfVJLsdcwW62qNkFj0TXWP2bE0nRfJ1 R+dcQ4iK/V5F+jVMEhzAwsUSeV8DO8M= 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-320-I6glqvKlOxO_TQs4Lcr2Tw-1; Wed, 25 Nov 2020 14:08:09 -0500 X-MC-Unique: I6glqvKlOxO_TQs4Lcr2Tw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 608831005D55; Wed, 25 Nov 2020 19:08:06 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-239.ams2.redhat.com [10.36.112.239]) by smtp.corp.redhat.com (Postfix) with ESMTP id 063109CA0; Wed, 25 Nov 2020 19:08:02 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 2/6] OvmfPkg/AmdSev: add Grub Firmware Volume Package From: "Laszlo Ersek" To: jejb@linux.ibm.com, devel@edk2.groups.io, Bret Barkelew , "Liming Gao (Byosoft address)" Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, thomas.lendacky@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" , "Ard Biesheuvel (ARM address)" References: <20201120184521.19437-1-jejb@linux.ibm.com> <20201120184521.19437-3-jejb@linux.ibm.com> <28e99174-79b3-e805-b977-5fed0071a702@redhat.com> <06b9425507ab8c1b35d377cf9bba155b0cc44147.camel@linux.ibm.com> <3b7899fa-fa52-7652-2d2a-d4ec67ece34d@redhat.com> <1c871b56-f459-5ac4-3b8d-a55d978eac06@redhat.com> <93fdaca88b53d400670b338a06fd1410c1445a39.camel@linux.ibm.com> <082a97c2-9a49-acf6-fd7c-70ee6b61c000@redhat.com> <5b9b21c3eb37ba7024c1cb85ead267867b323c7d.camel@linux.ibm.com> <1064db1d53315987bf8bb478894a07bda8d90a96.camel@linux.ibm.com> <53957e99-3f81-0121-b8fd-db28fdb01a73@redhat.com> Message-ID: Date: Wed, 25 Nov 2020 20:08:02 +0100 MIME-Version: 1.0 In-Reply-To: <53957e99-3f81-0121-b8fd-db28fdb01a73@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/25/20 19:35, Laszlo Ersek wrote: > On 11/25/20 18:09, James Bottomley wrote: >> On Wed, 2020-11-25 at 08:02 -0800, James Bottomley wrote: >>> On Wed, 2020-11-25 at 15:01 +0100, Laszlo Ersek wrote: >>>> This upgrade gave me kernel 5.8.18-100.fc31.x86_64 in the guest -- >>>> and this one does *not* crash. From your boot log below, I see your >>>> guest kernel is 5.5.0; I suggest upgrading it. >>> >>> Heh, that's easier said than done ... I always make my encrypted >>> images too small to upgrade a kernel easily. Anyway, after doing the >>> remove and add stuff dance, I finally got it upgraded to the latest >>> debian testing linux-image-5.8.0-3 it's still crashing although with >>> a slightly different traceback. It looks like there might be >>> something additional in the fedora 5.8 kernel that fixes this. I'm >>> going to try out upstream kernels next. >> >> I've got the upstream kernel booting through OVMF with a qemu -kernel >> command line. I also have a fix: it's not to delete the dummy variable >> which was part of the ancient x86 anti bricking code (which is also why >> arm64 doesn't have the problem). >> >> If you remove the set variable in arch/x86/platform/efi/quirks.c: >> >> /* >> * Deleting the dummy variable which kicks off garbage collection >> */ >> void efi_delete_dummy_variable(void) >> { >> efi.set_variable_nonblocking((efi_char16_t *)efi_dummy_name, >> &EFI_DUMMY_GUID, >> EFI_VARIABLE_NON_VOLATILE | >> EFI_VARIABLE_BOOTSERVICE_ACCESS | >> EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL); >> } >> >> The kernel will boot. I'm not sure why we have this deletion >> unconditionally in efi_enter_virtual_mode, but removing the call with >> the patch below allows the kernel to boot. > > I think commit 2ecb7402cfc7 ("efi/x86: Do not clean dummy variable in > kexec path", 2019-10-07) is related (part of v5.4), but it's not > sufficient to prevent the boot crash. (That removal only covered the > kexec path, and not the normal boot path.) > >> >> However, once the kernel has booted, any attempt to write to an EFI >> variable results in this: >> >> [ 975.440240] [Firmware Bug]: Page fault caused by firmware at PA: 0x7e450020 >> >> And then the efi runtime gets disabled. > > Blech, that doesn't look good. We still get a page fault somewhere in > the firmware, it just doesn't kill the kernel outright. That kind of > suggests the crash on the boot path *is* firmware-originated, it's just > that the kernel is unable to mask the problem that early. Actually, I do think we have a bug in the firmware. I need to verify it (and if verification confirms it, to send a patch for it). Consider the following snippet in file "MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c", function RegisterVariablePolicy(): // Reallocate and copy the table. NewTable = AllocatePool( NewSize ); if (NewTable == NULL) { return EFI_OUT_OF_RESOURCES; } CopyMem( NewTable, mPolicyTable, mCurrentTableUsage ); mCurrentTableSize = NewSize; if (mPolicyTable != NULL) { FreePool( mPolicyTable ); } mPolicyTable = NewTable; * In the SMM build of OVMF, this code is linked (via "MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf") into the following SMM driver: MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf For that module (well, for SMM drivers in general), the AllocatePool() function comes from the following MemoryAllocationLib instance: MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf and so the AllocatePool() call ultimately boils down to allocating *SMRAM*: gSmst->SmmAllocatePool() Which is fine. * However, in the non-SMM build, the same code snippet is linked (via "MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf") into the following driver: MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf In this module (well, in runtime DXE drivers in general), the AllocatePool() call function comes from the following MemoryAllocationLib instance: MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf And that is *wrong*. Because there, AllocatePool() allocates Boot Services Data type memory, via gBS->AllocatePool() *plus* "EfiBootServicesData" Therefore, even though VariablePolicyLibVirtualAddressCallback() in file "MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c" attempts to convert "mPolicyTable" to a runtime virtual address, in the SMM-less build, that conversion fails -- the original memory that the pointer points at, pre-conversion, is not runtime, but boot time memory. The fix *should be* to call AllocateRuntimePool() from the MemoryAllocationLib class. In the SMM instance, that will make no difference; compare, in "MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c": VOID * EFIAPI AllocatePool ( IN UINTN AllocationSize ) { return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize); } VOID * EFIAPI AllocateRuntimePool ( IN UINTN AllocationSize ) { return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize); } The InternalAllocatePool() calls are identical, in the SMM instance. Whereas in the UEFI instance, in "MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c", it does make a difference; compare: VOID * EFIAPI AllocatePool ( IN UINTN AllocationSize ) { return InternalAllocatePool (EfiBootServicesData, AllocationSize); } VOID * EFIAPI AllocateRuntimePool ( IN UINTN AllocationSize ) { return InternalAllocatePool (EfiRuntimeServicesData, AllocationSize); } The "MemoryType" parameter of InternalAllocatePool(), which gets forwarded to gBS->AllocatePool(), changes from "EfiBootServicesData" to "EfiRuntimeServicesData". Let me test this. (BTW I see some more pool allocations in "MdeModulePkg/Library/VariablePolicyHelperLib" to... I guess I'll have to check those as well.) Thanks Laszlo