Hi Sami, Thanks for your comments. Please find my reply inline.
Regards, Pranav > -----Original Message----- > From: Sami Mujawar <sami.muja...@arm.com> > Sent: Thursday, March 10, 2022 9:01 PM > To: Pranav Madhu <pranav.ma...@arm.com>; devel@edk2.groups.io > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>; nd <n...@arm.com> > Subject: Re: [edk2][PATCH V1 1/1] ArmPkg: Handle warm reboot request > correctly > > Hi Pranav, > > Thank you for this patch. > > Please find my response inline marked [SAMI]. > > Regards, > > Sami Mujawar > > > On 10/03/2022 01:10 PM, Pranav Madhu wrote: > > The warm reboot requests are mapped to cold reboot as the power > > control module was not capable of handling the warm reboot requests in > > the legacy implementation. The support for warm reboot support is > > added into the power control module. To support warm reset, update > > ArmPsciResetSystemLib, and there by invoke the PSCI call with > > parameters for warm reboot. > > > > Signed-off-by: Pranav Madhu <pranav.ma...@arm.com> > > --- > > ArmPkg/Include/IndustryStandard/ArmStdSmc.h | 1 + > > ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.c | 7 > +++++-- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > Link to github branch for this patch - > > https://github.com/Pranav-Madhu/edk2/tree/topics/warm_reboot > > > > diff --git a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h > > b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h > > index 655edc21b205..c9059dead6e9 100644 > > --- a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h > > +++ b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h > > @@ -93,6 +93,7 @@ > > #define ARM_SMC_ID_PSCI_MIGRATE_AARCH32 0x84000005 > > #define ARM_SMC_ID_PSCI_SYSTEM_OFF 0x84000008 > > #define ARM_SMC_ID_PSCI_SYSTEM_RESET 0x84000009 > > +#define ARM_SMC_ID_PSCI_SYSTEM_RESET2_AARCH64 0xc4000012 > > > > /* The current PSCI version is: 0.2 */ > > #define ARM_SMC_PSCI_VERSION_MAJOR 0 diff --git > > a/ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.c > > b/ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.c > > index 7bcd34849507..27e048ba0f7a 100644 > > --- a/ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.c > > +++ b/ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.c > > @@ -45,10 +45,13 @@ LibResetSystem ( > > ARM_SMC_ARGS ArmSmcArgs; > > > > switch (ResetType) { > > + case EfiResetWarm: > > + ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_RESET2_AARCH64; > > + ArmSmcArgs.Arg1 = 0; > > + ArmSmcArgs.Arg2 = 0; > > + break; > [SAMI] SYSTEM_RESET2 is an optional feature and if not supported would > return NOT_SUPPORTED. So, if a platform does not support SYSTEM_RESET2, > should the code here fall back to SYSTEM_RESET? > According to the PSCI specification, it is the responsibility of the OS to > check > that SYSTEM_RESET2 is supported before calling SYSTEM_RESET2 (I believe this > is applicable for the case where UEFI is not used to boot the OS). However, if > the runtime service ResetSystem() is invoked by the OS requesting a warm > reset, is it not the firmware's responsibility to ensure that SYSTEM_RESET2 is > supported? Any thoughts? Right, from PSCI specification, what I understood is before invoking SYSTEM_RESET2, the OS should query the PSCI capabilities using PSCI_FEATURES for SYSTEM_RESET2. The OS should invoke RESET2 only if PSCI_FEATURES returns 0. From spec, what I understood is it is not the responsibility of firmware. If OS issue RESET2 without querying FEATURES, the only option for firmware is to return NOT_SUPPORTED. > [/SAMI] > > 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; -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87452): https://edk2.groups.io/g/devel/message/87452 Mute This Topic: https://groups.io/mt/89685482/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-