From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web12.3188.1664525180083209422 for ; Fri, 30 Sep 2022 01:06:20 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=cx62WAgq; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 125FFB82760 for ; Fri, 30 Sep 2022 08:06:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BF39EC43470 for ; Fri, 30 Sep 2022 08:06:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1664525176; bh=2C4Vll3TP+0ZyE/uT34+enP8lkxRYigY0/Se04d3ZyA=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=cx62WAgqXJ5lcMCrQBy0Yt2DSvKJBuag1zz34eHnTRh4bdUKvy3u2/mZ40EGIkmqV 20MvVrQ8iHDUGP2479hsnCfQMnDrtzo99bFhgMIhzzVE/JfiwaroXbWMwpulqjhEg/ dLyecKo7N6MvWdHB74O+4i5uCqMlnEpmIjGEB0Pv7WElxI77uugytDH4XleFHi+hHl Ur3Fr8AN850drKmqcUZFje8pgdNBVfRij2qiT5k9DmuLyqZK1cgRiAQFPwqm0wNcdf eIGrK4rQOrTt4M4GDQvQY/9G12DrGD4zqgPB3OC4+HxmN4AxqKNHlE10+MRNGhxPuP YXIT/5nYD99ZQ== Received: by mail-lf1-f49.google.com with SMTP id d42so5760224lfv.0 for ; Fri, 30 Sep 2022 01:06:16 -0700 (PDT) X-Gm-Message-State: ACrzQf0irf0akCwi5XpAo+CEXjyGfbWO0BL7YtnlAjzc5HCk3DIqzX4Y xrQF8c/o0Ln9Q9PsDis3G73e9+FhZpKGWM2bYWA= X-Google-Smtp-Source: AMsMyM4yzqbJaFZwZ2LmKaEKlovYtoZzhWAQIeEHYt4vOOgYWDlRanMTHOr8wwUXmj2zot2EjE2Gzs+yAwUGdfB7ZHc= X-Received: by 2002:a05:6512:3da0:b0:4a1:e17a:2305 with SMTP id k32-20020a0565123da000b004a1e17a2305mr2907561lfv.228.1664525174734; Fri, 30 Sep 2022 01:06:14 -0700 (PDT) MIME-Version: 1.0 References: <20220928153323.2583389-1-dionnaglaze@google.com> <20220928153323.2583389-5-dionnaglaze@google.com> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 30 Sep 2022 10:06:03 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v4 4/6] MdeModulePkg: DxeMain accepts all memory at EBS if needed To: Dionna Amalie Glaze , "Ni, Ray" Cc: "devel@edk2.groups.io" , Gerd Hoffmann , James Bottomley , "Yao, Jiewen" , Tom Lendacky Content-Type: text/plain; charset="UTF-8" On Thu, 29 Sept 2022 at 20:14, Dionna Amalie Glaze wrote: > > > > > Can you explain a bit more why this PCD is needed? > > > > I'll expand the comment further, but essentially we need a bit in the > firmware to persist from a call into a protocol to the eventual call > to ExitBootServices. If we needed offline persistence, then we'd need > to standardize a new bit in the OsIndications variable. We don't so we > make due with a Pcd. > As I mentioned elsewhere, I'd prefer to avoid a dynamic PCD here. For Ray's benefit, I'll copy/paste my proposal here that I made in one of the other thread: """ Given that dynamic PCDs are optional but protocols are not, can we just drop the PCD and (in view of some other comments below) do the following: - Define a protocol in MdeModulePkg that ExitBootServices() will invoke after disarming the timer but before finalizing the memory map. It doesn't have to be specific to memory acceptance at all, the only thing that matters is that it is invoked at the right time (and perhaps only on the first call to EBS()). E.g., gEdkiiExitBootServicesCallbackProtocol with a single method called TerminateMemoryMapPreHook(). This protocol is fundamentally internal to EDK2 and does not require inclusion in PI or UEFI - Define a protocol in OvmfPkg that will be called by the OS loader to indicate that it is capable of taking charge of the unaccepted memory, and so it does not need the firmware to do so on its behalf, by accepting all of it during ExitBootServices(). - Implement a single DXE driver in OvmfPkg (or add the functionality to an existing one) that provides an implementation of the former protocol, and implements the latter protocol by uninstalling the former again. This means the 'accept all memory' routine can be moved out of DXE core as well, and into your driver. """ > > > > > I am not following the logic here 100%. As far as I can tell, if > > accepting all memory succeeded without errors, ExitBootServices() > > returns with EFI_SUCCESS, even though it has modified the memory map. > > This means the actual memory map is out of sync with the last > > GetMemoryMap() call performed by the OS loader before it called > > ExitBootServices(), and so it will still contain unaccepted memory, > > right? > > Ah yes you're right, I missed that part. I'll change it to return > EFI_WARN_STALE_DATA if any memory region gets accepted, and force a > failure in DxeMain if the status is an error or that warning. > The only documented error code for ExitBootServices() is EFI_INVALID_PARAMETER, which indicates that the map key has changes, so this is the one you should return in this case. EFI_WARN_STALE_DATA is not an error code (it has the MSB cleared) so macros like EFI_ERROR() will not identify it as an error.