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.115, mailfrom: zhichao.gao@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Wed, 17 Jul 2019 18:56:31 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Jul 2019 18:56:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,276,1559545200"; d="scan'208";a="173037879" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga006.jf.intel.com with ESMTP; 17 Jul 2019 18:56:30 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 17 Jul 2019 18:56:29 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 17 Jul 2019 18:56:29 -0700 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 17 Jul 2019 18:56:29 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.134]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.60]) with mapi id 14.03.0439.000; Thu, 18 Jul 2019 09:56:27 +0800 From: "Gao, Zhichao" 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 Subject: Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported Thread-Topic: [edk2-devel] [PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported Thread-Index: AQHVPHKGJyg74mh6wkWxt4SAw2rHpqbO8h0AgACn1IA= Date: Thu, 18 Jul 2019 01:56:27 +0000 Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B80879F@SHSMSX101.ccr.corp.intel.com> References: <20190717073725.30304-1-zhichao.gao@intel.com> <20190717073725.30304-4-zhichao.gao@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: zhichao.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Kinney, Michael D > Sent: Wednesday, July 17, 2019 11:41 PM > To: devel@edk2.groups.io; Gao, Zhichao ; Kinney, > Michael D > Cc: Wang, Jian J ; Wu, Hao A = ; > Ni, Ray ; Zeng, Star ; Gao, Limin= g > ; Sean Brogan ; > Michael Turner ; Bret Barkelew > > Subject: RE: [edk2-devel] [PATCH 3/3] MdeModulePkg/CapsuleRuntimeDxe: > Control runtime services supported >=20 > Zhichao, >=20 > Why is CapsuleRuntimeDxe the component that is responsible for setting t= he > UEFI Variable for supported RT services? >=20 > I would think this should be in: >=20 > =09MdeModulePkg\Core\RuntimeDxe Sorry. I didn't think of RuntimeDxe when I work on the patch. I assume the= variable may be set in some dxe driver (may be a platform drive not in the= edk repo) and capsule driver would check that and then set it if it isn't = set yet. Agree with you, RuntimeDxe is a better place to set the variable. Once the= setting variable code is added to edk repo, there would be no responsibili= ty for others to set the variable. Thanks, Zhichao >=20 > I agree that each RT driver that populates the RT Services Table with a = RT > services can consume the new bitmask PCD and use the PCD to determine if > the RT Service should return EFI_UNSUPPORTED after ExitBootServices(). >=20 > Thanks, >=20 > Mike >=20 > > -----Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > Gao, Zhichao > > Sent: Wednesday, July 17, 2019 12:37 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 > > > > Subject: [edk2-devel] [PATCH 3/3] > > MdeModulePkg/CapsuleRuntimeDxe: Control runtime services supported > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1907 > > > > Add PcdRuntimeServicesSupport to control whether the capsule services > > is supported during runtime phase. If the L"RuntimeServicesSupported" > > variable is not set yet, it would set the variable base on the pcd. > > If the pcd value is 0x3FFF that means all runtime services is > > supported at runtime phase, L"RuntimeServicesSupported" would not be > > set. > > > > 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 > > Signed-off-by: Zhichao Gao > > --- > > .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf | 3 + > > .../CapsuleRuntimeDxe/CapsuleService.c | 129 > > ++++++++++++++++++ > > 2 files changed, 132 insertions(+) > > > > diff --git > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime > > Dxe.inf > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime > > Dxe.inf > > index 9da450722b..95b760d94e 100644 > > --- > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime > > Dxe.inf > > +++ > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntime > > Dxe.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 ## > > SOMETIMES_CONSUMES ## Variable > > L"RuntimeServicesSupported" > > + gEfiEventExitBootServicesGuid ## > > CONSUMES > > > > [Protocols] > > gEfiCapsuleArchProtocolGuid ## > > PRODUCES > > @@ -91,6 +93,7 @@ > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsu > > le ## SOMETIMES_CONSUMES > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule > > ## SOMETIMES_CONSUMES # Populate Image requires reset support. > > gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleInRamSupport > > ## CONSUMES > > + gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupport > > ## SOME_TIMES_CONSUMES > > > > [Pcd.X64] > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdCapsulePeiLongModeStack > > Size ## SOMETIMES_CONSUMES > > diff --git > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService > > .c > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService > > .c > > index 77b8f00062..8ab77361d8 100644 > > --- > > a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService > > .c > > +++ > > b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService > > .c > > @@ -24,6 +24,12 @@ 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 > > +77,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 +269,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 +357,106 @@ QueryCapsuleCapabilities ( > > return EFI_SUCCESS; > > } > > > > +/** > > + Set the L"RuntimeServicesSupported" variable depend on > > the pcd PcdRuntimeServicesSupport. > > + > > + Firstly try to get the variable, it may be set in > > other dxe driver. > > + If it is set, then return EFI_SUCCESS. If it isn't > > present, try to set it. > > + > > + @retval EFI_SUCCESS The variable is already set. > > + EFI_NOT_FOUND All runtime services are > > supported at runtime. No variable is set. > > + Others Error to get variable or set > > variable. Unexpected. > > +**/ > > +static > > +EFI_STATUS > > +SetRuntimeServicesSupported ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + UINT16 RuntimeServicesSupported; > > + UINT32 Attributes; > > + UINTN DataSize; > > + > > + // > > + // Firstly try to get L"RuntimeServicesSupported" > > variable // > > + 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 (Status =3D=3D EFI_NOT_FOUND) { > > + // > > + // L"RuntimeServicesSupported" isn't set yet. Then > > set it if > > + // some of the RuntimeServices is unsupported. > > + // > > + RuntimeServicesSupported =3D PcdGet16 > > (PcdRuntimeServicesSupport); > > + if (RuntimeServicesSupported !=3D 0x3FFF) { > > + Status =3D gRT->SetVariable ( > > + L"RuntimeServicesSupported", > > + &gEfiGlobalVariableGuid, > > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > > EFI_VARIABLE_RUNTIME_ACCESS, > > + sizeof (UINT16), > > + &RuntimeServicesSupported > > + ); > > + } else { > > + // > > + // Set Status to EFI_NOT_FOUND to indicate not > > such variable > > + // > > + Status =3D EFI_NOT_FOUND; > > + } > > + } > > + > > + return Status; > > +} > > + > > +/** > > + 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 > > +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 +476,7 @@ CapsuleServiceInitialize ( > > ) > > { > > EFI_STATUS Status; > > + EFI_EVENT Event; > > > > mMaxSizePopulateCapsule =3D > > PcdGet32(PcdMaxSizePopulateCapsule); > > mMaxSizeNonPopulateCapsule =3D > > PcdGet32(PcdMaxSizeNonPopulateCapsule); > > @@ -393,5 +508,19 @@ CapsuleServiceInitialize ( > > ); > > ASSERT_EFI_ERROR (Status); > > > > + Status =3D SetRuntimeServicesSupported (); > > + > > + ASSERT (Status =3D=3D EFI_NOT_FOUND || Status =3D=3D > > EFI_SUCCESS); > > + > > + Status =3D gBS->CreateEventEx( > > + EVT_NOTIFY_SIGNAL, > > + TPL_NOTIFY, > > + UpdateRuntimeServicesSupported, > > + NULL, > > + &gEfiEventExitBootServicesGuid, > > + &Event > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > return Status; > > } > > -- > > 2.21.0.windows.1 > > > > > >=20