public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Gao, Zhichao" <zhichao.gao@intel.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Rothman, Michael A" <michael.a.rothman@intel.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
	"Wu, Hao A" <hao.a.wu@intel.com>, "Ni, Ray" <ray.ni@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	Sean Brogan <sean.brogan@microsoft.com>,
	Michael Turner <Michael.Turner@microsoft.com>,
	Bret Barkelew <Bret.Barkelew@microsoft.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2-devel] [PATCH V2 3/4] MdeModulePkg/RuntimeDxe: Set RuntimeServicesSupport base on Pcd
Date: Fri, 19 Jul 2019 16:01:54 +0000	[thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B9D791C9@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <20190719080921.17516-4-zhichao.gao@intel.com>

Zhichao,

I realized that MdeModulePkg/Core/RuntimeDxe may not be the
best place to set this variable.  This module has a depex of
TRUE, so there is no guarantee that the variable services are
available when this module is dispatched.

This new variable is only intended to be consumed by OS Loaders
and OS Kernels, so the latest time that this variable can be
set is Ready To Boot in BDS.

All UEFI Services are available when gBds->Entry() is called
from the DXE Core, so maybe the best place to set this new
variable is in gBds->Entry().

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 <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: [edk2-devel] [PATCH V2 3/4]
> MdeModulePkg/RuntimeDxe: Set RuntimeServicesSupport base
> on Pcd
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1907
> 
> Add a pcd named PcdRuntimeServicesSupport, RuntimeDxe
> driver would set variable L"RuntimeServicesSupported"
> base on this pcd. The varialbe would indicate whether
> the runtime services is supported or not in runtime
> phase. If the pcd's value is 0x3FFF, that means all
> runtime services is supported at runtime, then the
> variable wouldn't be set.
> Refer to UEFI spec 2.8, Section 8.1.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  MdeModulePkg/Core/RuntimeDxe/Runtime.c      | 65
> ++++++++++++++++++++-
>  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf |  8 ++-
>  2 files changed, 71 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> index c52b2b7ecf..394baf230a 100644
> --- a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> +++ b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
> @@ -35,7 +35,7 @@ Revision History:
>    Table now contains an item named CalculateCrc32.
> 
> 
> -Copyright (c) 2006 - 2015, Intel Corporation. All
> rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All
> rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -359,6 +359,65 @@ RuntimeDriverSetVirtualAddressMap (
>    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 = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS;  DataSize = sizeof
> (UINT16);  Status =
> + gRT->GetVariable (
> +                  L"RuntimeServicesSupported",
> +                  &gEfiGlobalVariableGuid,
> +                  &Attributes,
> +                  &DataSize,
> +                  &RuntimeServicesSupported
> +                  );
> +
> +  if (Status == EFI_NOT_FOUND) {
> +    //
> +    // L"RuntimeServicesSupported" isn't set yet. Then
> set it if
> +    // some of the RuntimeServices is unsupported.
> +    //
> +    RuntimeServicesSupported = PcdGet16
> (PcdRuntimeServicesSupport);
> +    if (RuntimeServicesSupported != 0x3FFF) {
> +      Status = 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 = EFI_NOT_FOUND;
> +    }
> +  }
> +
> +  return Status;
> +}
> +
>  /**
>    Entry Point for Runtime driver.
> 
> @@ -401,6 +460,10 @@ RuntimeDriverInitialize (
>    gRT->SetVirtualAddressMap =
> RuntimeDriverSetVirtualAddressMap;
>    gRT->ConvertPointer       =
> RuntimeDriverConvertPointer;
> 
> +  Status = SetRuntimeServicesSupported ();
> +
> +  ASSERT (Status == EFI_NOT_FOUND || Status ==
> EFI_SUCCESS);
> +
>    //
>    // Install the Runtime Architectural Protocol onto a
> new handle
>    //
> diff --git a/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> b/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> index 694c7690fa..48ecec9e99 100644
> --- a/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> +++ b/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> @@ -5,7 +5,7 @@
>  #  CalculateCrc32 boot services table,
> SetVirtualAddressMap & ConvertPointer  #  runtime
> services table.
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All
> rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All
> rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -
> 53,6 +53,12 @@
>    gEfiRuntimeArchProtocolGuid                   ##
> PRODUCES
>    gEfiLoadedImageProtocolGuid                   ##
> CONSUMES
> 
> +[Guids]
> +  gEfiGlobalVariableGuid                        ##
> SOMETIMES_CONSUMES   ## Variable
> L"RuntimeServicesSupported"
> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupport
> ## SOME_TIMES_CONSUMES
> +
>  [depex]
>    TRUE
> 
> --
> 2.21.0.windows.1
> 
> 
> 


  reply	other threads:[~2019-07-19 16:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-19  8:09 [PATCH V2 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder Gao, Zhichao
2019-07-19  8:09 ` [PATCH V2 1/4] MdePkg/UefiSpec.h: Add define of runtime services support Gao, Zhichao
2019-07-19  8:09 ` [PATCH V2 2/4] MdePkg: Add new pcd PcdRuntimeServicesSupport Gao, Zhichao
2019-07-19  8:09 ` [PATCH V2 3/4] MdeModulePkg/RuntimeDxe: Set RuntimeServicesSupport base on Pcd Gao, Zhichao
2019-07-19 16:01   ` Michael D Kinney [this message]
2019-07-19  8:09 ` [PATCH V2 4/4] MdeModulePkg/CapsuleRuntimeDxe: Implement RuntimeServicesSupported Gao, Zhichao
2019-07-19 16:08   ` [edk2-devel] " Michael D Kinney
2019-07-22  2:53     ` Gao, Zhichao
2019-07-22  3:39       ` Michael D Kinney
2019-07-19 14:15 ` [PATCH V2 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder Laszlo Ersek
2019-07-22  3:17   ` [edk2-devel] " Gao, Zhichao
2019-07-22 17:28     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E92EE9817A31E24EB0585FDF735412F5B9D791C9@ORSMSX113.amr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox