From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D9B8B1A1E05 for ; Sun, 23 Oct 2016 21:46:15 -0700 (PDT) Received: from nwb-ext-pat.microfocus.com ([10.120.13.103]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Mon, 24 Oct 2016 06:46:14 +0200 Received: from GaryWorkstation (nwb-a10-snat.microfocus.com [10.120.13.201]) by nwb-ext-pat.microfocus.com with ESMTP (TLS encrypted); Mon, 24 Oct 2016 05:45:58 +0100 Date: Mon, 24 Oct 2016 12:45:51 +0800 From: Gary Lin To: Laszlo Ersek Cc: edk2-devel-01 , Anthony PERARD , Jordan Justen Message-ID: <20161024044551.dg7sceb7olp3axsz@GaryWorkstation> References: <20161021212737.15974-1-lersek@redhat.com> <20161021212737.15974-9-lersek@redhat.com> MIME-Version: 1.0 In-Reply-To: <20161021212737.15974-9-lersek@redhat.com> User-Agent: Mutt/1.6.2 (2016-07-01) Subject: Re: [PATCH 08/19] OvmfPkg/PlatformPei: eliminate unchecked PcdSetXX() calls X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Oct 2016 04:46:16 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Oct 21, 2016 at 11:27:26PM +0200, Laszlo Ersek wrote: > These are deprecated / disabled under the > DISABLE_NEW_DEPRECATED_INTERFACES feature test macro. > > Introduce a variable called PcdStatus, and use it to assert the success of > these operations (there is no reason for them to fail here). > Reviewed-by: Gary Lin and Tested-by: Gary Lin > Cc: Anthony PERARD > Cc: Gary Lin > Cc: Jordan Justen > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek > --- > > Notes: > The ReserveEmuVariableNvStore() and InitializeXen() hunks make no > difference in my setup, so testing for those would be appreciated. > > OvmfPkg/PlatformPei/MemDetect.c | 11 +++-- > OvmfPkg/PlatformPei/Platform.c | 44 +++++++++++++------- > OvmfPkg/PlatformPei/Xen.c | 5 ++- > 3 files changed, 40 insertions(+), 20 deletions(-) > > diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c > index 4863eb101067..d00a570d4381 100644 > --- a/OvmfPkg/PlatformPei/MemDetect.c > +++ b/OvmfPkg/PlatformPei/MemDetect.c > @@ -107,6 +107,7 @@ GetFirstNonAddress ( > FIRMWARE_CONFIG_ITEM FwCfgItem; > UINTN FwCfgSize; > UINT64 HotPlugMemoryEnd; > + RETURN_STATUS PcdStatus; > > FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb (); > > @@ -154,7 +155,8 @@ GetFirstNonAddress ( > if (mBootMode != BOOT_ON_S3_RESUME) { > DEBUG ((EFI_D_INFO, "%a: disabling 64-bit PCI host aperture\n", > __FUNCTION__)); > - PcdSet64 (PcdPciMmio64Size, 0); > + PcdStatus = PcdSet64S (PcdPciMmio64Size, 0); > + ASSERT_RETURN_ERROR (PcdStatus); > } > > // > @@ -202,8 +204,11 @@ GetFirstNonAddress ( > // the GCD memory space map through our PciHostBridgeLib instance; here we > // only need to set the PCDs. > // > - PcdSet64 (PcdPciMmio64Base, Pci64Base); > - PcdSet64 (PcdPciMmio64Size, Pci64Size); > + PcdStatus = PcdSet64S (PcdPciMmio64Base, Pci64Base); > + ASSERT_RETURN_ERROR (PcdStatus); > + PcdStatus = PcdSet64S (PcdPciMmio64Size, Pci64Size); > + ASSERT_RETURN_ERROR (PcdStatus); > + > DEBUG ((EFI_D_INFO, "%a: Pci64Base=0x%Lx Pci64Size=0x%Lx\n", > __FUNCTION__, Pci64Base, Pci64Size)); > } > diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c > index ca1e6dc7e320..c6e1106c9ed0 100644 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -156,8 +156,9 @@ MemMapInitialization ( > VOID > ) > { > - UINT64 PciIoBase; > - UINT64 PciIoSize; > + UINT64 PciIoBase; > + UINT64 PciIoSize; > + RETURN_STATUS PcdStatus; > > PciIoBase = 0xC000; > PciIoSize = 0x4000; > @@ -212,8 +213,11 @@ MemMapInitialization ( > // > PciSize = 0xFC000000 - PciBase; > AddIoMemoryBaseSizeHob (PciBase, PciSize); > - PcdSet64 (PcdPciMmio32Base, PciBase); > - PcdSet64 (PcdPciMmio32Size, PciSize); > + PcdStatus = PcdSet64S (PcdPciMmio32Base, PciBase); > + ASSERT_RETURN_ERROR (PcdStatus); > + PcdStatus = PcdSet64S (PcdPciMmio32Size, PciSize); > + ASSERT_RETURN_ERROR (PcdStatus); > + > AddIoMemoryBaseSizeHob (0xFEC00000, SIZE_4KB); > AddIoMemoryBaseSizeHob (0xFED00000, SIZE_1KB); > if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) { > @@ -266,8 +270,10 @@ MemMapInitialization ( > PciIoBase, > PciIoSize > ); > - PcdSet64 (PcdPciIoBase, PciIoBase); > - PcdSet64 (PcdPciIoSize, PciIoSize); > + PcdStatus = PcdSet64S (PcdPciIoBase, PciIoBase); > + ASSERT_RETURN_ERROR (PcdStatus); > + PcdStatus = PcdSet64S (PcdPciIoSize, PciIoSize); > + ASSERT_RETURN_ERROR (PcdStatus); > } > > EFI_STATUS > @@ -316,11 +322,13 @@ GetNamedFwCfgBoolean ( > > #define UPDATE_BOOLEAN_PCD_FROM_FW_CFG(TokenName) \ > do { \ > - BOOLEAN Setting; \ > + BOOLEAN Setting; \ > + RETURN_STATUS PcdStatus; \ > \ > if (!EFI_ERROR (GetNamedFwCfgBoolean ( \ > "opt/ovmf/" #TokenName, &Setting))) { \ > - PcdSetBool (TokenName, Setting); \ > + PcdStatus = PcdSetBoolS (TokenName, Setting); \ > + ASSERT_RETURN_ERROR (PcdStatus); \ > } \ > } while (0) > > @@ -379,12 +387,13 @@ MiscInitialization ( > VOID > ) > { > - UINTN PmCmd; > - UINTN Pmba; > - UINT32 PmbaAndVal; > - UINT32 PmbaOrVal; > - UINTN AcpiCtlReg; > - UINT8 AcpiEnBit; > + UINTN PmCmd; > + UINTN Pmba; > + UINT32 PmbaAndVal; > + UINT32 PmbaOrVal; > + UINTN AcpiCtlReg; > + UINT8 AcpiEnBit; > + RETURN_STATUS PcdStatus; > > // > // Disable A20 Mask > @@ -424,7 +433,8 @@ MiscInitialization ( > ASSERT (FALSE); > return; > } > - PcdSet16 (PcdOvmfHostBridgePciDevId, mHostBridgeDevId); > + PcdStatus = PcdSet16S (PcdOvmfHostBridgePciDevId, mHostBridgeDevId); > + ASSERT_RETURN_ERROR (PcdStatus); > > // > // If the appropriate IOspace enable bit is set, assume the ACPI PMBA > @@ -491,6 +501,7 @@ ReserveEmuVariableNvStore ( > ) > { > EFI_PHYSICAL_ADDRESS VariableStore; > + RETURN_STATUS PcdStatus; > > // > // Allocate storage for NV variables early on so it will be > @@ -509,7 +520,8 @@ ReserveEmuVariableNvStore ( > VariableStore, > (2 * PcdGet32 (PcdFlashNvStorageFtwSpareSize)) / 1024 > )); > - PcdSet64 (PcdEmuVariableNvStoreReserved, VariableStore); > + PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore); > + ASSERT_RETURN_ERROR (PcdStatus); > } > > > diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c > index 223908a4f529..ab38f97a67aa 100644 > --- a/OvmfPkg/PlatformPei/Xen.c > +++ b/OvmfPkg/PlatformPei/Xen.c > @@ -210,6 +210,8 @@ InitializeXen ( > VOID > ) > { > + RETURN_STATUS PcdStatus; > + > if (mXenLeaf == 0) { > return EFI_NOT_FOUND; > } > @@ -222,7 +224,8 @@ InitializeXen ( > // > AddReservedMemoryBaseSizeHob (0xFC000000, 0x1000000, FALSE); > > - PcdSetBool (PcdPciDisableBusEnumeration, TRUE); > + PcdStatus = PcdSetBoolS (PcdPciDisableBusEnumeration, TRUE); > + ASSERT_RETURN_ERROR (PcdStatus); > > return EFI_SUCCESS; > } > -- > 2.9.2 > > >