From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: michael.d.kinney@intel.com) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by groups.io with SMTP; Sun, 21 Jul 2019 20:39:18 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Jul 2019 20:39:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,293,1559545200"; d="scan'208";a="344291438" Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by orsmga005.jf.intel.com with ESMTP; 21 Jul 2019 20:39:17 -0700 Received: from orsmsx112.amr.corp.intel.com (10.22.240.13) by ORSMSX103.amr.corp.intel.com (10.22.225.130) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 21 Jul 2019 20:39:17 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.177]) by ORSMSX112.amr.corp.intel.com ([169.254.3.253]) with mapi id 14.03.0439.000; Sun, 21 Jul 2019 20:39:17 -0700 From: "Michael D Kinney" To: "Gao, Zhichao" , "devel@edk2.groups.io" , "Kinney, Michael D" CC: "Wang, Jian J" , "Wu, Hao A" , "Ni, Ray" , "Zeng, Star" , "Gao, Liming" , Sean Brogan , Michael Turner , Bret Barkelew , Laszlo Ersek Subject: Re: [edk2-devel] [PATCH V2 4/4] MdeModulePkg/CapsuleRuntimeDxe: Implement RuntimeServicesSupported Thread-Topic: [edk2-devel] [PATCH V2 4/4] MdeModulePkg/CapsuleRuntimeDxe: Implement RuntimeServicesSupported Thread-Index: AQHVPgnG1M+xf+YNkE6Zy0aCI8di36bSGqgggARP+wD//5ZkgA== Date: Mon, 22 Jul 2019 03:39:17 +0000 Message-ID: References: <20190719080921.17516-1-zhichao.gao@intel.com> <20190719080921.17516-5-zhichao.gao@intel.com> <3CE959C139B4C44DBEA1810E3AA6F9000B809356@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <3CE959C139B4C44DBEA1810E3AA6F9000B809356@SHSMSX101.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Return-Path: michael.d.kinney@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The UefiRuntimeLib has an EfiAtRuntime() service. Pease use that instead. The CapsuleRuntimeDxe module is already using the UefiRuntimeLib library. The patch below adds a 2nd ExitBootServices event and global that is not needed. Thanks, Mike > -----Original Message----- > From: Gao, Zhichao > Sent: Sunday, July 21, 2019 7:54 PM > To: Kinney, Michael D ; > devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A > ; Ni, Ray ; Zeng, > Star ; Gao, Liming > ; Sean Brogan > ; Michael Turner > ; Bret Barkelew > ; Laszlo Ersek > > Subject: RE: [edk2-devel] [PATCH V2 4/4] > MdeModulePkg/CapsuleRuntimeDxe: Implement > RuntimeServicesSupported >=20 > Hi Mike, >=20 > I agree to remove the GetVariable. But why for > ExitBootServices? If we remove that, the services would > be disabled at both boot phase and runtime phase. >=20 >=20 > Thanks, > Zhichao >=20 > > -----Original Message----- > > From: Kinney, Michael D > > Sent: Saturday, July 20, 2019 12:09 AM > > To: devel@edk2.groups.io; Gao, Zhichao > ; > > Kinney, Michael D > > Cc: Wang, Jian J ; Wu, Hao A > > ; Ni, Ray ; Zeng, > Star > > ; Gao, Liming > ; Sean Brogan > > ; Michael Turner > > ; Bret Barkelew > > ; Laszlo Ersek > > > Subject: RE: [edk2-devel] [PATCH V2 4/4] > MdeModulePkg/CapsuleRuntimeDxe: > > Implement RuntimeServicesSupported > > > > Zhichao, > > > > The GetVariable and ExitBootServices events should be > removed from > > this patch. I recommend RT drivers that produce RT > services consume > > the new PCD to determine if the services return > EFI_UNSUPPORTED or not > > after ExitBootServices(). This keeps RT Driver as > simple as possible > > and removes the need for the UEFI Variable to be in a > known state > > earlier in the boot and removes the need for RT Drivers > to setup extra notification events. > > > > The new UEFI Variable is intended to be used by OS > Loaders and OS > > Kernels that call ExitBootServices() and need to know > if specific RT > > services are available or not after ExitBootServices(). > > > > Thanks, > > > > Mike > > > > > -----Original Message----- > > > From: devel@edk2.groups.io > [mailto:devel@edk2.groups.io] On Behalf > > > Of Gao, Zhichao > > > Sent: Friday, July 19, 2019 1:09 AM > > > To: devel@edk2.groups.io > > > Cc: Wang, Jian J ; Wu, Hao A > > > ; Ni, Ray ; > Zeng, Star > > > ; Gao, Liming > ; Sean > > > Brogan ; Michael Turner > > > ; Bret Barkelew > > > ; Laszlo Ersek > > > > Subject: [edk2-devel] [PATCH V2 4/4] > > > MdeModulePkg/CapsuleRuntimeDxe: Implement > RuntimeServicesSupported > > > > > > REF: > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1907 > > > > > > Control the capsule services supported at runtime > base on the > > > variable L"RuntimeServicesSupported". > > > It would update a global variable > > > mRuntimeServicesSupported at ExitBootServices event. > > > > > > Cc: Jian J Wang > > > Cc: Hao A Wu > > > Cc: Ray Ni > > > Cc: Star Zeng > > > Cc: Liming gao > > > Cc: Sean Brogan > > > Cc: Michael Turner > > > Cc: Bret Barkelew > > > Cc: Laszlo Ersek > > > Signed-off-by: Zhichao Gao > > > --- > > > .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf | 2 + > > > .../CapsuleRuntimeDxe/CapsuleService.c | 68 > > > +++++++++++++++++++ > > > 2 files changed, 70 insertions(+) > > > > > > diff --git > > > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntim > > > eDxe.inf > > > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntim > > > eDxe.inf > > > index 9da450722b..15d498863a 100644 > > > --- > > > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntim > > > eDxe.inf > > > +++ > > > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntim > > > eDxe.inf > > > @@ -72,6 +72,8 @@ > > > ## SOMETIMES_PRODUCES ## > > > Variable:L"CapsuleLongModeBuffer" # The long mode > buffer used by > > > IA32 Capsule PEIM to call X64 CapsuleCoalesce code to > handle >4GB > > > capsule blocks > > > gEfiCapsuleVendorGuid > > > gEfiFmpCapsuleGuid ## > > > SOMETIMES_CONSUMES ## GUID # FMP capsule GUID > > > + gEfiGlobalVariableGuid ## > > > CONSUMES ## Variable > > > L"RuntimeServicesSupported" > > > + gEfiEventExitBootServicesGuid ## > > > CONSUMES > > > > > > [Protocols] > > > gEfiCapsuleArchProtocolGuid ## > > > PRODUCES > > > diff --git > > > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleServic > > > e.c > > > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleServic > > > e.c > > > index 77b8f00062..e4e700764b 100644 > > > --- > > > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleServic > > > e.c > > > +++ > > > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleServic > > > e.c > > > @@ -24,6 +24,13 @@ UINTN mTimes =3D 0; > > > UINT32 mMaxSizePopulateCapsule =3D 0; > > > UINT32 mMaxSizeNonPopulateCapsule =3D 0; > > > > > > +// > > > +// Runtime Services Supported Flag > > > +// Initialize it to 0x3FFF to indicate it is all > > > supported before > > > +runtime // > > > +static UINT16 mRuntimeServicesSupported =3D > 0x3FFF; > > > + > > > + > > > /** > > > Passes capsules to the firmware with both virtual > and physical > > > mapping. Depending on the intended > > > consumption, the firmware may process the capsule > immediately. If > > > the payload should persist @@ -71,6 > > > +78,10 @@ UpdateCapsule ( > > > CHAR16 CapsuleVarName[30]; > > > CHAR16 *TempVarName; > > > > > > + if (!(mRuntimeServicesSupported | > > > EFI_RT_SUPPORTED_UPDATE_CAPSULE)) { > > > + return EFI_UNSUPPORTED; > > > + } > > > + > > > // > > > // Check if platform support Capsule In RAM or > not. > > > // Platform could choose to drop > > > CapsulePei/CapsuleX64 and do not support Capsule In > RAM. > > > @@ -259,6 +270,10 @@ QueryCapsuleCapabilities ( > > > EFI_CAPSULE_HEADER *CapsuleHeader; > > > BOOLEAN NeedReset; > > > > > > + if (!(mRuntimeServicesSupported | > > > EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES)) { > > > + return EFI_UNSUPPORTED; > > > + } > > > + > > > // > > > // Capsule Count can't be less than one. > > > // > > > @@ -343,6 +358,48 @@ QueryCapsuleCapabilities ( > > > return EFI_SUCCESS; > > > } > > > > > > +/** > > > + ExitBootServices Event to update the > > > mRuntimeServicesSupported to > > > + affect the runtime services. > > > + > > > + @param[in] Event Event whose notification > > > function is being invoked > > > + @param[in] Context Pointer to the notification > > > function's context > > > +**/ > > > +static > > > +VOID > > > +EFIAPI > > > +UpdateRuntimeServicesSupported ( > > > + IN EFI_EVENT Event, > > > + IN VOID *Context > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + UINT16 RuntimeServicesSupported; > > > + UINT32 Attributes; > > > + UINTN DataSize; > > > + > > > + Attributes =3D EFI_VARIABLE_BOOTSERVICE_ACCESS | > > > + EFI_VARIABLE_RUNTIME_ACCESS; DataSize =3D sizeof > > > (UINT16); Status =3D > > > + gRT->GetVariable ( > > > + L"RuntimeServicesSupported", > > > + &gEfiGlobalVariableGuid, > > > + &Attributes, > > > + &DataSize, > > > + &RuntimeServicesSupported > > > + ); > > > + > > > + if (!EFI_ERROR (Status)) { > > > + mRuntimeServicesSupported =3D > > > RuntimeServicesSupported; > > > + } else if (Status =3D=3D EFI_NOT_FOUND) { > > > + mRuntimeServicesSupported =3D 0x3FFF; > > > + } else { > > > + // > > > + // Should never arrive here. > > > + // > > > + ASSERT_EFI_ERROR (Status); > > > + } > > > +} > > > + > > > > > > /** > > > > > > @@ -362,6 +419,7 @@ CapsuleServiceInitialize ( > > > ) > > > { > > > EFI_STATUS Status; > > > + EFI_EVENT Event; > > > > > > mMaxSizePopulateCapsule =3D > > > PcdGet32(PcdMaxSizePopulateCapsule); > > > mMaxSizeNonPopulateCapsule =3D > > > PcdGet32(PcdMaxSizeNonPopulateCapsule); > > > @@ -381,6 +439,16 @@ CapsuleServiceInitialize ( > > > gRT->UpdateCapsule =3D > > > UpdateCapsule; > > > gRT->QueryCapsuleCapabilities =3D > > > QueryCapsuleCapabilities; > > > > > > + Status =3D gBS->CreateEventEx( > > > + EVT_NOTIFY_SIGNAL, > > > + TPL_NOTIFY, > > > + UpdateRuntimeServicesSupported, > > > + NULL, > > > + &gEfiEventExitBootServicesGuid, > > > + &Event > > > + ); > > > + ASSERT_EFI_ERROR (Status); > > > + > > > // > > > // Install the Capsule Architectural Protocol on a > new handle > > > // to signify the capsule runtime services are > ready. > > > -- > > > 2.21.0.windows.1 > > > > > > > > >=20