From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x232.google.com (mail-it0-x232.google.com [IPv6:2607:f8b0:4001:c0b::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1B4A721CF3B7E for ; Mon, 3 Jul 2017 10:32:14 -0700 (PDT) Received: by mail-it0-x232.google.com with SMTP id m84so59970441ita.0 for ; Mon, 03 Jul 2017 10:33:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=rJ4ydmgIKD4bFvDoahxbYh4r+W+PRsfcNfv4moz3PaQ=; b=MmTJnOZAuKqDtsgx0Hfc/sRZYn1ecmioy7itJd9Rtg0Yaj5gC7lhSad/q0Cz3cN5Oe q42TLqvzTN+JaO2rajq/fK4UfIgZocAArvYyg50WpDUnQwH5fgjHfAPjIU2PFpT43pwH rXQjwytWIje8R081RG3iLe05UZS8KNrznia6c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=rJ4ydmgIKD4bFvDoahxbYh4r+W+PRsfcNfv4moz3PaQ=; b=R5E/4tGcz+WXJ3/sTUcNZqUOzCeUocMvqCJnuMytrW2GpInMImGApP4whErmYYGfp5 hHp26CmAgOwB04euWQlTdhCYfpl5Uvuk1XrWDev+sZfNXavqO+FNl+mf9ho0npz8Hz5W RCIvmdNA7Hz6kazwO2NSlwb9s9e7BBMw2l+ja5VwuNZ+M7+vec5JLHs1h9jc9wQ6555Z tL8Fx5F5npDDkA2CpXtThUDS5AFF+zJdMSSxx0YUGzbYDVvV0C6a8ih8wQwV0USwiqcs ooyTJvZLE2IJMErF6OPOrNjYmPDxwDjZWvhHoitiL02SvOGlnvEshOe/+QJ2pA6HCWPA aYfw== X-Gm-Message-State: AIVw112A8JXPnmMIsAODdpt/ylaj4xQfm5eOCIdSjylvH4s5rl6y5SNe e3Otoyln/I11lyx1CXcaSrD75hK3XiEBvhs= X-Received: by 10.36.219.132 with SMTP id c126mr10283295itg.73.1499103230636; Mon, 03 Jul 2017 10:33:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.134.134 with HTTP; Mon, 3 Jul 2017 10:33:50 -0700 (PDT) In-Reply-To: <20170703162531.27349-1-leif.lindholm@linaro.org> References: <20170703162531.27349-1-leif.lindholm@linaro.org> From: Ard Biesheuvel Date: Mon, 3 Jul 2017 18:33:50 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" Subject: Re: [PATCH] BeagleBoardPkg: switch to use MdeModulePkg ResetSystemLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Jul 2017 17:32:14 -0000 Content-Type: text/plain; charset="UTF-8" On 3 July 2017 at 17:25, Leif Lindholm wrote: > The BeagleBoard port used EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > for its reset handling. With the arrival > MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > > As part of this, change BeagleBoardPkg/Library/ResetSystemLib to be an > implementation of ResetSystemLib instead of the previous > EfiResetSystemLib. > > Wire all reset variants to ResetCold, except for ResetShutdown and > EnterS3WithImmediateWake, which return immediately. > > Note: this ResetSystemLib never supported being called after > ExitBootservices, and this shortcoming is not addressed here. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Leif Lindholm > --- > > This is only build tested for now. > Once the edk2-platforms master branch goes live, I intend > to move this platform there, and bring back in the > BeagleBoard Black port. Which I do have a test platform > handy for. > > BeagleBoardPkg/BeagleBoardPkg.dsc | 4 +- > BeagleBoardPkg/BeagleBoardPkg.fdf | 2 +- > .../Library/ResetSystemLib/ResetSystemLib.c | 180 ++++++++------------- > .../Library/ResetSystemLib/ResetSystemLib.inf | 16 +- > 4 files changed, 70 insertions(+), 132 deletions(-) > > diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc > index 90d6af444d..b22f814a28 100644 > --- a/BeagleBoardPkg/BeagleBoardPkg.dsc > +++ b/BeagleBoardPkg/BeagleBoardPkg.dsc > @@ -56,7 +56,7 @@ > BaseLib|MdePkg/Library/BaseLib/BaseLib.inf > BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > > - EfiResetSystemLib|BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf > + ResetSystemLib|BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf > > PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf > PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf > @@ -421,7 +421,7 @@ > # SerialPortLib|ArmPkg/Library/SemiHostingSerialPortLib/SemiHostingSerialPortLib.inf > # } > > - EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf > EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf > > diff --git a/BeagleBoardPkg/BeagleBoardPkg.fdf b/BeagleBoardPkg/BeagleBoardPkg.fdf > index f140d9019c..c9c6afd714 100644 > --- a/BeagleBoardPkg/BeagleBoardPkg.fdf > +++ b/BeagleBoardPkg/BeagleBoardPkg.fdf > @@ -115,7 +115,7 @@ FvNameGuid = d0dd3e90-343d-4cb3-8f69-772214989282 > INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf > INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > > - INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf > INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf > > diff --git a/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.c b/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.c > index 6b7879b02b..7bc6c6c329 100644 > --- a/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.c > +++ b/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.c > @@ -2,6 +2,7 @@ > Do a generic Cold Reset for OMAP3550 and BeagleBoard specific Warm reset > > Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
> + Copyright (c) 2017, Linaro Ltd. All rights reserved.
> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > @@ -16,150 +17,95 @@ > > #include > > -#include > -#include > -#include > #include > -#include > -#include > -#include > +#include > > #include > > +/** > + This function causes a system-wide reset (cold reset), in which > + all circuitry within the system returns to its initial state. This type of > + reset is asynchronous to system operation and operates without regard to > + cycle boundaries. > > + If this function returns, it means that the system does not support cold > + reset. > +**/ > VOID > -ShutdownEfi ( > +EFIAPI > +ResetCold ( > VOID > ) > { > - EFI_STATUS Status; > - UINTN MemoryMapSize; > - EFI_MEMORY_DESCRIPTOR *MemoryMap; > - UINTN MapKey; > - UINTN DescriptorSize; > - UINTN DescriptorVersion; > - UINTN Pages; > - > - MemoryMap = NULL; > - MemoryMapSize = 0; > - do { > - Status = gBS->GetMemoryMap ( > - &MemoryMapSize, > - MemoryMap, > - &MapKey, > - &DescriptorSize, > - &DescriptorVersion > - ); > - if (Status == EFI_BUFFER_TOO_SMALL) { > - > - Pages = EFI_SIZE_TO_PAGES (MemoryMapSize) + 1; > - MemoryMap = AllocatePages (Pages); > - > - // > - // Get System MemoryMap > - // > - Status = gBS->GetMemoryMap ( > - &MemoryMapSize, > - MemoryMap, > - &MapKey, > - &DescriptorSize, > - &DescriptorVersion > - ); > - // Don't do anything between the GetMemoryMap() and ExitBootServices() > - if (!EFI_ERROR (Status)) { > - Status = gBS->ExitBootServices (gImageHandle, MapKey); > - if (EFI_ERROR (Status)) { > - FreePages (MemoryMap, Pages); > - MemoryMap = NULL; > - MemoryMapSize = 0; > - } > - } > - } > - } while (EFI_ERROR (Status)); > - > - //Clean and invalidate caches. > - WriteBackInvalidateDataCache(); > - InvalidateInstructionCache(); > - > - //Turning off Caches and MMU > - ArmDisableDataCache (); > - ArmDisableInstructionCache (); > - ArmDisableMmu (); > + //Perform cold reset of the system. > + MmioOr32 (PRM_RSTCTRL, RST_DPLL3); > + while ((MmioRead32(PRM_RSTST) & GLOBAL_COLD_RST) != 0x1); > } > > -typedef > +/** > + This function causes a system-wide initialization (warm reset), in which all > + processors are set to their initial state. Pending cycles are not corrupted. > + > + If this function returns, it means that the system does not support warm > + reset. > +**/ > VOID > -(EFIAPI *CALL_STUB)( > +EFIAPI > +ResetWarm ( > VOID > -); > - > + ) > +{ > + ResetCold (); > +} > > /** > - Resets the entire platform. > - > - @param ResetType The type of reset to perform. > - @param ResetStatus The status code for the reset. > - @param DataSize The size, in bytes, of WatchdogData. > - @param ResetData For a ResetType of EfiResetCold, EfiResetWarm, or > - EfiResetShutdown the data buffer starts with a Null-terminated > - Unicode string, optionally followed by additional binary data. > + This function causes the system to enter a power state equivalent > + to the ACPI G2/S5 or G3 states. > > + If this function returns, it means that the system does not support shut down > + reset. > **/ > -EFI_STATUS > +VOID > EFIAPI > -LibResetSystem ( > - IN EFI_RESET_TYPE ResetType, > - IN EFI_STATUS ResetStatus, > - IN UINTN DataSize, > - IN CHAR16 *ResetData OPTIONAL > +ResetShutdown ( > + VOID > ) > { > - CALL_STUB StartOfFv; > - > - if (ResetData != NULL) { > - DEBUG((EFI_D_ERROR, "%s", ResetData)); > - } > - > - ShutdownEfi (); > - > - switch (ResetType) { > - case EfiResetWarm: > - //Perform warm reset of the system by jumping to the begining of the FV > - StartOfFv = (CALL_STUB)(UINTN)PcdGet64 (PcdFvBaseAddress); > - StartOfFv (); > - break; > - case EfiResetCold: > - case EfiResetShutdown: > - default: > - //Perform cold reset of the system. > - MmioOr32 (PRM_RSTCTRL, RST_DPLL3); > - while ((MmioRead32(PRM_RSTST) & GLOBAL_COLD_RST) != 0x1); > - break; > - } > - > - // If the reset didn't work, return an error. > - ASSERT (FALSE); > - return EFI_DEVICE_ERROR; > + // not implemented > } > > - > - > /** > - Initialize any infrastructure required for LibResetSystem () to function. > - > - @param ImageHandle The firmware allocated handle for the EFI image. > - @param SystemTable A pointer to the EFI System Table. > - > - @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. > + This function causes the system to enter S3 and then wake up immediately. > > + If this function returns, it means that the system does not support S3 > + feature. > **/ > -EFI_STATUS > +VOID > EFIAPI > -LibInitializeResetSystem ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > +EnterS3WithImmediateWake ( > + VOID > ) > { > - return EFI_SUCCESS; > + // not implemented > } > > +/** > + This function causes a systemwide reset. The exact type of the reset is > + defined by the EFI_GUID that follows the Null-terminated Unicode string passed > + into ResetData. If the platform does not recognize the EFI_GUID in ResetData > + the platform must pick a supported reset type to perform.The platform may > + optionally log the parameters from any non-normal reset that occurs. > + > + @param[in] DataSize The size, in bytes, of ResetData. > + @param[in] ResetData The data buffer starts with a Null-terminated string, > + followed by the EFI_GUID. > +**/ > +VOID > +EFIAPI > +ResetPlatformSpecific ( > + IN UINTN DataSize, > + IN VOID *ResetData > + ) > +{ > + ResetCold (); > +} > diff --git a/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf b/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf > index 64638cb62c..150c4bb900 100644 > --- a/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf > +++ b/BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.inf > @@ -2,6 +2,7 @@ > # Reset System lib to make it easy to port new platforms > # > # Copyright (c) 2008, Apple Inc. All rights reserved.
> +# Copyright (c) 2017, Linaro Ltd. All rights reserved.
> # > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD License > @@ -19,31 +20,22 @@ > FILE_GUID = 781371a2-3fdd-41d4-96a1-7b34cbc9e895 > MODULE_TYPE = BASE > VERSION_STRING = 1.0 > - LIBRARY_CLASS = EfiResetSystemLib > + LIBRARY_CLASS = ResetSystemLib > > > [Sources.common] > ResetSystemLib.c > > [Packages] > - MdePkg/MdePkg.dec > ArmPkg/ArmPkg.dec > - EmbeddedPkg/EmbeddedPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > Omap35xxPkg/Omap35xxPkg.dec > > [Pcd.common] > gArmTokenSpaceGuid.PcdCpuResetAddress > - gEmbeddedTokenSpaceGuid.PcdEmbeddedFdBaseAddress > > [LibraryClasses] > - DebugLib > - ArmLib > - CacheMaintenanceLib > - MemoryAllocationLib > - UefiRuntimeServicesTableLib > - TimerLib > - UefiLib > - UefiBootServicesTableLib > Don't you need IoLib here? Other than that: Reviewed-by: Ard Biesheuvel > [Pcd] > gArmTokenSpaceGuid.PcdFvBaseAddress > -- > 2.11.0 >