From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x231.google.com (mail-wr0-x231.google.com [IPv6:2a00:1450:400c:c0c::231]) (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 7B1B921A00AC2 for ; Mon, 3 Jul 2017 07:13:45 -0700 (PDT) Received: by mail-wr0-x231.google.com with SMTP id r103so235290872wrb.0 for ; Mon, 03 Jul 2017 07:15:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=kwOa2n0e+gUmAxgGquwodBeWNF8uR/v1QH1Rekslpjk=; b=OLl8nO4peW5yXsZ8pLeZa1D2pYl/BkuimXBUWwH1km5m5w6fuLl8F9SXeZ9nasIL5T +U6WsFef3AXCX7Tk3U9zeLMcc4Qp3uSnN+tCxgPHF6aWR1IM5jIS/LOsZJv6t32sOpZ7 IaX8+iWQnq1tKe77O/n8VOCxQvWz2RfGI00VA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=kwOa2n0e+gUmAxgGquwodBeWNF8uR/v1QH1Rekslpjk=; b=tf9kd9+o0SmV8gOWsQLo1LfA2RjyeKzWPXnJWXVF3IOJfDFLJuoFW3/NphsWeT1dif pwCDA0QXRxhr9tsFTQSEAO8eVD+hjijzeZO75knDThBN8laUCH5kW3RauQUV/+bRdEOU z8JLDbkr5M7/Vml8UopP01LptxhOBaYHa5FZQVvPtq0IRnNOCiWhnGJrLhvG0CWspXIx Z/CeFLBWCRvUD0szx+7IxRgFwWAk2c9xUVrtFAYo9UGpzn455dKyKBxfKllHB5ML2Ob0 72TUg/Iq5P5yHf8/LOWv3UVRZQ3UrP9gWPh2tAaSnC07PLFO5n4h5pi/ux/Q+C1Gg2cV 0RPg== X-Gm-Message-State: AKS2vOylYuJi88is2iJ9VcvdCtamFTpWuqtSTwqgJXHCp5aTifhAvFts XtLmr0TP9u+YDZz7 X-Received: by 10.223.133.211 with SMTP id 19mr37588946wru.27.1499091321212; Mon, 03 Jul 2017 07:15:21 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 185sm18991139wmu.33.2017.07.03.07.15.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 03 Jul 2017 07:15:20 -0700 (PDT) Date: Mon, 3 Jul 2017 15:15:19 +0100 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel@lists.01.org, lersek@redhat.com Message-ID: <20170703141519.GG26676@bivouac.eciton.net> References: <20170703135345.28703-1-ard.biesheuvel@linaro.org> MIME-Version: 1.0 In-Reply-To: <20170703135345.28703-1-ard.biesheuvel@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH] ArmVirtPkg: switch to generic ResetSystemRuntimeDxe 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 14:13:46 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jul 03, 2017 at 02:53:45PM +0100, Ard Biesheuvel wrote: > For obscure reasons, ARM platforms use a different implementation of > the ResetSystem() runtime service call than other platforms. So let's > switch all ArmVirtPkg platforms to the generic version instead. > > Given that all platforms use an implementation of EfiResetSystemLib [as > consumed by the ResetRuntimeDxe in EmbeddedPkg that we are replacing] > which is unlikely to be depended upon by out of tree platforms, let's > simply modify this library into an implementation of ResetSystemLib > instead [which is what the generic driver in MdeModulePkg consumes] > > This does mean we need to update all clients at the same time, which > is why all changes are part of the same patch. > > As before, warm reset and platform specific reset are mapped onto > cold reset (which is the only thing PSCI implements, at least the > version we depend on). The new library function EnterS3WithImmediateWake() > is left unimplemented, as permitted by the ResetSystemLib library class. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel Looks good. Reviewed-by: Leif Lindholm > --- > Note: this a no-whitespace diff > > ArmVirtPkg/ArmVirt.dsc.inc | 2 +- > ArmVirtPkg/ArmVirtQemu.dsc | 2 +- > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 2 +- > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +- > ArmVirtPkg/ArmVirtXen.dsc | 2 +- > ArmVirtPkg/ArmVirtXen.fdf | 2 +- > ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.c | 125 +++++++++++++------- > ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf | 4 +- > 8 files changed, 89 insertions(+), 52 deletions(-) > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index 5bf537635488..0b9b457b5619 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -101,7 +101,7 @@ [LibraryClasses.common] > > PlatformPeiLib|ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf > MemoryInitPeiLib|ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.inf > - EfiResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf > + ResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf > > # ARM PL031 RTC Driver > RealTimeClockLib|ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > index 10de57a4149e..e23a6d17bc44 100644 > --- a/ArmVirtPkg/ArmVirtQemu.dsc > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > @@ -266,7 +266,7 @@ [Components.common] > MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > - EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf { > > NULL|ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > index ba2e7157cf2c..237b2d03a714 100644 > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > @@ -63,7 +63,7 @@ [FV.FvMain] > INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf > !endif > INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > - INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf > INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf > INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 93ccb6df37e1..aa01debfda69 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -257,7 +257,7 @@ [Components.common] > MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > - EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf { > > NULL|ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf > diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc > index f832b3fb64f4..eb37137f271e 100644 > --- a/ArmVirtPkg/ArmVirtXen.dsc > +++ b/ArmVirtPkg/ArmVirtXen.dsc > @@ -167,7 +167,7 @@ [Components.common] > MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf > > MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > - EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf > EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf > > diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf > index b591818f0b42..67fde73e696d 100644 > --- a/ArmVirtPkg/ArmVirtXen.fdf > +++ b/ArmVirtPkg/ArmVirtXen.fdf > @@ -140,7 +140,7 @@ [FV.FvMain] > INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf > > INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > - INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf > + INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf > INF EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf > INF EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf > INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf > diff --git a/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.c b/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.c > index b94d1b0090c6..4b0db206e920 100644 > --- a/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.c > +++ b/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.c > @@ -23,7 +23,7 @@ > > #include > #include > -#include > +#include > #include > #include > #include > @@ -67,49 +67,75 @@ ArmPsciResetSystemLibConstructor ( > } > > /** > - 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 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. > **/ > -EFI_STATUS > +VOID > EFIAPI > -LibResetSystem ( > - IN EFI_RESET_TYPE ResetType, > - IN EFI_STATUS ResetStatus, > - IN UINTN DataSize, > - IN CHAR16 *ResetData OPTIONAL > +ResetCold ( > + VOID > ) > { > ARM_SMC_ARGS ArmSmcArgs; > ARM_HVC_ARGS ArmHvcArgs; > > - switch (ResetType) { > - > - case EfiResetPlatformSpecific: > - // Map the platform specific reset as reboot > - case EfiResetWarm: > - // Map a warm reset into a cold reset > - case EfiResetCold: > // Send a PSCI 0.2 SYSTEM_RESET command > ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_RESET; > ArmHvcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_RESET; > + > + switch (mArmPsciMethod) { > + case 1: > + ArmCallHvc (&ArmHvcArgs); > break; > - case EfiResetShutdown: > - // Send a PSCI 0.2 SYSTEM_OFF command > - ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF; > - ArmHvcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF; > + > + case 2: > + ArmCallSmc (&ArmSmcArgs); > break; > + > default: > - ASSERT (FALSE); > - return EFI_UNSUPPORTED; > + DEBUG ((EFI_D_ERROR, "%a: no PSCI method defined\n", __FUNCTION__)); > + } > +} > + > +/** > + 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 > +ResetWarm ( > + VOID > + ) > +{ > + // Map a warm reset into a cold reset > + ResetCold (); > } > > +/** > + 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 shutdown reset. > +**/ > +VOID > +EFIAPI > +ResetShutdown ( > + VOID > + ) > +{ > + ARM_SMC_ARGS ArmSmcArgs; > + ARM_HVC_ARGS ArmHvcArgs; > + > + // Send a PSCI 0.2 SYSTEM_OFF command > + ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF; > + ArmHvcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF; > + > switch (mArmPsciMethod) { > case 1: > ArmCallHvc (&ArmHvcArgs); > @@ -121,30 +147,41 @@ LibResetSystem ( > > default: > DEBUG ((EFI_D_ERROR, "%a: no PSCI method defined\n", __FUNCTION__)); > - return EFI_UNSUPPORTED; > } > - > - // We should never be here > - DEBUG ((EFI_D_ERROR, "%a: PSCI Reset failed\n", __FUNCTION__)); > - CpuDeadLoop (); > - return EFI_UNSUPPORTED; > } > > /** > - Initialize any infrastructure required for LibResetSystem () to function. > + This function causes the system to enter S3 and then wake up immediately. > > - @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. > + If this function returns, it means that the system does not support S3 feature. > +**/ > +VOID > +EFIAPI > +EnterS3WithImmediateWake ( > + VOID > + ) > +{ > + // 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. > **/ > -EFI_STATUS > +VOID > EFIAPI > -LibInitializeResetSystem ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > +ResetPlatformSpecific ( > + IN UINTN DataSize, > + IN VOID *ResetData > ) > { > - return EFI_SUCCESS; > + // Map the platform specific reset as reboot > + ResetCold (); > } > diff --git a/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf b/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf > index 9dc515aee52b..f28aed3fb1c2 100644 > --- a/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf > +++ b/ArmVirtPkg/Library/ArmVirtPsciResetSystemLib/ArmVirtPsciResetSystemLib.inf > @@ -20,7 +20,7 @@ [Defines] > FILE_GUID = c81d76ed-66fa-44a3-ac4a-f163120187a9 > MODULE_TYPE = BASE > VERSION_STRING = 1.0 > - LIBRARY_CLASS = EfiResetSystemLib|DXE_DRIVER DXE_RUNTIME_DRIVER > + LIBRARY_CLASS = ResetSystemLib|DXE_DRIVER DXE_RUNTIME_DRIVER > CONSTRUCTOR = ArmPsciResetSystemLibConstructor > > [Sources] > @@ -29,8 +29,8 @@ [Sources] > [Packages] > ArmPkg/ArmPkg.dec > ArmVirtPkg/ArmVirtPkg.dec > + MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > - EmbeddedPkg/EmbeddedPkg.dec > > [LibraryClasses] > ArmSmcLib > -- > 2.9.3 >