public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Support for RuntimeServicesSupported global variable
@ 2019-11-27 23:25 Jeff Brasen
  2019-11-27 23:25 ` [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations Jeff Brasen
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Jeff Brasen @ 2019-11-27 23:25 UTC (permalink / raw)
  To: devel; +Cc: liming.gao, michael.d.kinney, hao.a.wu, ray.ni, zhichao.gao,
	jbrasen

 Add support for the new UEFI 2.8 runtime services supported variable
 that is used to indicate which runtime services a platform supports.
 Also, add support for initializing this variable based on a PCD.

Change Log:
v1 - Initial version
v2 - Move pcd from MdeModulePkg to MdePkg and update uni file


Jeff Brasen (3):
  MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations
  MdePkg/MdeModule: Add support for RuntimeServicesSupported variable
  MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable

 .../VarCheckUefiLib/VarCheckUefiLibNullClass.c     | 11 +++++++
 MdeModulePkg/Universal/BdsDxe/BdsDxe.inf           |  1 +
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c           | 35 +++++++++++++++++++++-
 MdePkg/Include/Guid/GlobalVariable.h               |  7 +++++
 MdePkg/Include/Uefi/UefiSpec.h                     | 18 +++++++++++
 MdePkg/MdePkg.dec                                  | 18 +++++++++++
 MdePkg/MdePkg.uni                                  | 17 +++++++++++
 7 files changed, 106 insertions(+), 1 deletion(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations
  2019-11-27 23:25 [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Jeff Brasen
@ 2019-11-27 23:25 ` Jeff Brasen
  2019-11-28  0:33   ` [edk2-devel] " Liming Gao
  2019-11-27 23:25 ` [PATCH v2 2/3] MdePkg/MdeModule: Add support for RuntimeServicesSupported variable Jeff Brasen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Jeff Brasen @ 2019-11-27 23:25 UTC (permalink / raw)
  To: devel; +Cc: liming.gao, michael.d.kinney, hao.a.wu, ray.ni, zhichao.gao,
	jbrasen

Add bitmask values for the value of the RuntimeServicesSupported
variable defined in the UEFI 2.8 specification. This is used to describe
what services the platform supports while in runtime.

Change-Id: I7ff0fcdb856b4d2e4ba90a08291f8980e2f66375
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 MdePkg/Include/Uefi/UefiSpec.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
index 444aa35..7e2b719 100644
--- a/MdePkg/Include/Uefi/UefiSpec.h
+++ b/MdePkg/Include/Uefi/UefiSpec.h
@@ -1783,6 +1783,24 @@ EFI_STATUS
 #define EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY          0x0000000000000040
 
 //
+// Bitmasks of supported runtime functions for RuntimeServicesSupported variable
+//
+#define EFI_RT_SUPPORTED_GET_TIME                           0x0001
+#define EFI_RT_SUPPORTED_SET_TIME                           0x0002
+#define EFI_RT_SUPPORTED_GET_WAKEUP_TIME                    0x0004
+#define EFI_RT_SUPPORTED_SET_WAKEUP_TIME                    0x0008
+#define EFI_RT_SUPPORTED_GET_VARIABLE                       0x0010
+#define EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME             0x0020
+#define EFI_RT_SUPPORTED_SET_VARIABLE                       0x0040
+#define EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP            0x0080
+#define EFI_RT_SUPPORTED_CONVERT_POINTER                    0x0100
+#define EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT      0x0200
+#define EFI_RT_SUPPORTED_RESET_SYSTEM                       0x0400
+#define EFI_RT_SUPPORTED_UPDATE_CAPSULE                     0x0800
+#define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES         0x1000
+#define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO                0x2000
+
+//
 // EFI Runtime Services Table
 //
 #define EFI_SYSTEM_TABLE_SIGNATURE      SIGNATURE_64 ('I','B','I',' ','S','Y','S','T')
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] MdePkg/MdeModule: Add support for RuntimeServicesSupported variable
  2019-11-27 23:25 [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Jeff Brasen
  2019-11-27 23:25 ` [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations Jeff Brasen
@ 2019-11-27 23:25 ` Jeff Brasen
  2019-11-28  0:34   ` [edk2-devel] " Liming Gao
  2019-11-27 23:25 ` [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set " Jeff Brasen
  2019-12-17  8:17 ` [edk2-devel] [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Ard Biesheuvel
  3 siblings, 1 reply; 10+ messages in thread
From: Jeff Brasen @ 2019-11-27 23:25 UTC (permalink / raw)
  To: devel; +Cc: liming.gao, michael.d.kinney, hao.a.wu, ray.ni, zhichao.gao,
	jbrasen

Add support for new global variable defined in the UEFI 2.8
specification. This provides a bitmask of which calls are
implemented by the firmware during runtime services.

Change-Id: If871e16052ecd871fd03a0eef2e3ed5fa5beb93c
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c        | 11 +++++++++++
 MdePkg/Include/Guid/GlobalVariable.h                          |  7 +++++++
 2 files changed, 18 insertions(+)

diff --git a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
index e3bf04a..4264892 100644
--- a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
+++ b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
@@ -553,6 +553,17 @@ UEFI_DEFINED_VARIABLE_ENTRY mGlobalVariableList[] = {
     },
     NULL
   },
+  {
+    EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME,
+    {
+      VAR_CHECK_VARIABLE_PROPERTY_REVISION,
+      VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY,
+      VARIABLE_ATTRIBUTE_BS_RT,
+      sizeof (UINT16),
+      sizeof (UINT16)
+    },
+    NULL
+  },
 };
 
 UEFI_DEFINED_VARIABLE_ENTRY mGlobalVariableList2[] = {
diff --git a/MdePkg/Include/Guid/GlobalVariable.h b/MdePkg/Include/Guid/GlobalVariable.h
index 7abc103..06a8a12 100644
--- a/MdePkg/Include/Guid/GlobalVariable.h
+++ b/MdePkg/Include/Guid/GlobalVariable.h
@@ -182,5 +182,12 @@ extern EFI_GUID gEfiGlobalVariableGuid;
 /// Its attribute is BS+RT.
 ///
 #define EFI_VENDOR_KEYS_VARIABLE_NAME               L"VendorKeys"
+///
+/// Bitmask of which calls are implemented by the firmware during runtime services.
+/// RT access is required only if GetVariable() is implemented by runtime services.
+/// Should be treated as read-only.
+/// Its attribute is BS+RT.
+///
+#define EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME  L"RuntimeServicesSupported"
 
 #endif
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable
  2019-11-27 23:25 [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Jeff Brasen
  2019-11-27 23:25 ` [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations Jeff Brasen
  2019-11-27 23:25 ` [PATCH v2 2/3] MdePkg/MdeModule: Add support for RuntimeServicesSupported variable Jeff Brasen
@ 2019-11-27 23:25 ` Jeff Brasen
  2019-11-28  5:02   ` Ni, Ray
  2019-12-17  8:17 ` [edk2-devel] [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Ard Biesheuvel
  3 siblings, 1 reply; 10+ messages in thread
From: Jeff Brasen @ 2019-11-27 23:25 UTC (permalink / raw)
  To: devel; +Cc: liming.gao, michael.d.kinney, hao.a.wu, ray.ni, zhichao.gao,
	jbrasen

Add support for initializing and setting the UEFI 2.8 global variable
RuntimeServicesSupported based on the value of a PCD.

Change-Id: I8fbd404d492ff8278466edde8aa37d203537318c
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  1 +
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 +++++++++++++++++++++++++++++++-
 MdePkg/MdePkg.dec                        | 18 ++++++++++++++++
 MdePkg/MdePkg.uni                        | 17 ++++++++++++++++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
index 9310b4d..52ec04f 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
@@ -90,6 +90,7 @@
   gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultPlatformLang         ## CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdHardwareErrorRecordLevel                ## CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut                     ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported                ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor                    ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision                  ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand              ## CONSUMES
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index d387dbe..16bc593 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -40,7 +40,8 @@ CHAR16  *mReadOnlyVariables[] = {
   EFI_LANG_CODES_VARIABLE_NAME,
   EFI_BOOT_OPTION_SUPPORT_VARIABLE_NAME,
   EFI_HW_ERR_REC_SUPPORT_VARIABLE_NAME,
-  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME
+  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
+  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME
   };
 
 CHAR16 *mBdsLoadOptionName[] = {
@@ -626,6 +627,33 @@ BdsFormalizeOSIndicationVariable (
 
 /**
 
+  Formalize RuntimeServicesSupported variable.
+
+**/
+VOID
+BdsFormalizeRuntimeServicesSupportedVariable (
+  VOID
+  )
+{
+  EFI_STATUS                      Status;
+  UINT16                          RuntimeServicesSupported;
+
+  RuntimeServicesSupported = PcdGet16 (PcdRuntimeServicesSupported);
+  Status = gRT->SetVariable (
+                  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME,
+                  &gEfiGlobalVariableGuid,
+                  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+                  sizeof(RuntimeServicesSupported),
+                  &RuntimeServicesSupported
+                  );
+  //
+  // Platform needs to make sure setting volatile variable before calling 3rd party code shouldn't fail.
+  //
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+
   Validate variables.
 
 **/
@@ -645,6 +673,11 @@ BdsFormalizeEfiGlobalVariable (
   // Validate OSIndication related variable.
   //
   BdsFormalizeOSIndicationVariable ();
+
+  //
+  // Validate Runtime Services Supported variable.
+  //
+  BdsFormalizeRuntimeServicesSupportedVariable ();
 }
 
 /**
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index d022cc5..cdcb2f9 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2297,5 +2297,23 @@
   # @Prompt Boot Timeout (s)
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000002c
 
+  ## Bitmask of supported runtime services<BR>
+  #  BIT0  - GetTime
+  #  BIT1  - SetTime
+  #  BIT2  - GetWakeupTime
+  #  BIT3  - SetWakeupTime
+  #  BIT4  - GetVariable
+  #  BIT5  - GetNextVariableName
+  #  BIT6  - SetVariable
+  #  BIT7  - SetVirtualAddressMap
+  #  BIT8  - ConvertPointer
+  #  BIT9  - GetNextHighMonotonicCount
+  #  BIT10 - ResetSystem
+  #  BIT11 - UpdateCapsule
+  #  BIT12 - QueryCapsuleCapabilites
+  #  BIT13 - QueryVariableInfo
+  # @Prompt Supported Runtime services bitmask.
+  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported|0x3FFF|UINT16|0x0000002e
+
 [UserExtensions.TianoCore."ExtraFiles"]
   MdePkgExtra.uni
diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
index 5c1fa24..1edf681 100644
--- a/MdePkg/MdePkg.uni
+++ b/MdePkg/MdePkg.uni
@@ -413,3 +413,20 @@
 
 #string STR_gEfiMdePkgTokenSpaceGuid_PcdUartDefaultReceiveFifoDepth_HELP  #language en-US "Indicates the receive FIFO depth of UART controller.<BR><BR>"
 
+#string STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_PROMPT #language en-US "Supported Runtime Services bitmask"
+
+#string STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_HELP #language en-US "Bitmask of supported runtime services<BR><BR>\n"
+                                                                                      "BIT0  - GetTime<BR>\n"
+                                                                                      "BIT1  - SetTime<BR>\n"
+                                                                                      "BIT2  - GetWakeupTime<BR>\n"
+                                                                                      "BIT3  - SetWakeupTime<BR>\n"
+                                                                                      "BIT4  - GetVariable<BR>\n"
+                                                                                      "BIT5  - GetNextVariableName<BR>\n"
+                                                                                      "BIT6  - SetVariable<BR>\n"
+                                                                                      "BIT7  - SetVirtualAddressMap<BR>\n"
+                                                                                      "BIT8  - ConvertPointer<BR>\n"
+                                                                                      "BIT9  - GetNextHighMonotonicCount<BR>\n"
+                                                                                      "BIT10 - ResetSystem<BR>\n"
+                                                                                      "BIT11 - UpdateCapsule<BR>\n"
+                                                                                      "BIT12 - QueryCapsuleCapabilities<BR>\n"
+                                                                                      "BIT13 - QueryVariableInfo<BR>"
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations
  2019-11-27 23:25 ` [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations Jeff Brasen
@ 2019-11-28  0:33   ` Liming Gao
  0 siblings, 0 replies; 10+ messages in thread
From: Liming Gao @ 2019-11-28  0:33 UTC (permalink / raw)
  To: devel@edk2.groups.io, jbrasen@nvidia.com
  Cc: Kinney, Michael D, Wu, Hao A, Ni, Ray, Gao, Zhichao

The change is good. And, please remove Change-Id: in the commit message. 
Last, have you submitted BZ for it? 

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Jeff Brasen
>Sent: Thursday, November 28, 2019 7:25 AM
>To: devel@edk2.groups.io
>Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
><ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
>jbrasen@nvidia.com
>Subject: [edk2-devel] [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8
>RuntimeServicesSuppported definations
>
>Add bitmask values for the value of the RuntimeServicesSupported
>variable defined in the UEFI 2.8 specification. This is used to describe
>what services the platform supports while in runtime.
>
>Change-Id: I7ff0fcdb856b4d2e4ba90a08291f8980e2f66375
>Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>---
> MdePkg/Include/Uefi/UefiSpec.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/MdePkg/Include/Uefi/UefiSpec.h
>b/MdePkg/Include/Uefi/UefiSpec.h
>index 444aa35..7e2b719 100644
>--- a/MdePkg/Include/Uefi/UefiSpec.h
>+++ b/MdePkg/Include/Uefi/UefiSpec.h
>@@ -1783,6 +1783,24 @@ EFI_STATUS
> #define EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY
>0x0000000000000040
>
> //
>+// Bitmasks of supported runtime functions for RuntimeServicesSupported
>variable
>+//
>+#define EFI_RT_SUPPORTED_GET_TIME                           0x0001
>+#define EFI_RT_SUPPORTED_SET_TIME                           0x0002
>+#define EFI_RT_SUPPORTED_GET_WAKEUP_TIME                    0x0004
>+#define EFI_RT_SUPPORTED_SET_WAKEUP_TIME                    0x0008
>+#define EFI_RT_SUPPORTED_GET_VARIABLE                       0x0010
>+#define EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME             0x0020
>+#define EFI_RT_SUPPORTED_SET_VARIABLE                       0x0040
>+#define EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP            0x0080
>+#define EFI_RT_SUPPORTED_CONVERT_POINTER                    0x0100
>+#define EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT
>0x0200
>+#define EFI_RT_SUPPORTED_RESET_SYSTEM                       0x0400
>+#define EFI_RT_SUPPORTED_UPDATE_CAPSULE                     0x0800
>+#define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES         0x1000
>+#define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO                0x2000
>+
>+//
> // EFI Runtime Services Table
> //
> #define EFI_SYSTEM_TABLE_SIGNATURE      SIGNATURE_64 ('I','B','I','
>','S','Y','S','T')
>--
>2.7.4
>
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v2 2/3] MdePkg/MdeModule: Add support for RuntimeServicesSupported variable
  2019-11-27 23:25 ` [PATCH v2 2/3] MdePkg/MdeModule: Add support for RuntimeServicesSupported variable Jeff Brasen
@ 2019-11-28  0:34   ` Liming Gao
  0 siblings, 0 replies; 10+ messages in thread
From: Liming Gao @ 2019-11-28  0:34 UTC (permalink / raw)
  To: devel@edk2.groups.io, jbrasen@nvidia.com
  Cc: Kinney, Michael D, Wu, Hao A, Ni, Ray, Gao, Zhichao

Please separate the change per package. You can combine MdePkg changes into one patch. 

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Jeff Brasen
>Sent: Thursday, November 28, 2019 7:25 AM
>To: devel@edk2.groups.io
>Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
><ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
>jbrasen@nvidia.com
>Subject: [edk2-devel] [PATCH v2 2/3] MdePkg/MdeModule: Add support for
>RuntimeServicesSupported variable
>
>Add support for new global variable defined in the UEFI 2.8
>specification. This provides a bitmask of which calls are
>implemented by the firmware during runtime services.
>
>Change-Id: If871e16052ecd871fd03a0eef2e3ed5fa5beb93c
>Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>---
> .../Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c        | 11
>+++++++++++
> MdePkg/Include/Guid/GlobalVariable.h                          |  7 +++++++
> 2 files changed, 18 insertions(+)
>
>diff --git
>a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
>b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
>index e3bf04a..4264892 100644
>--- a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
>+++ b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
>@@ -553,6 +553,17 @@ UEFI_DEFINED_VARIABLE_ENTRY
>mGlobalVariableList[] = {
>     },
>     NULL
>   },
>+  {
>+    EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME,
>+    {
>+      VAR_CHECK_VARIABLE_PROPERTY_REVISION,
>+      VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY,
>+      VARIABLE_ATTRIBUTE_BS_RT,
>+      sizeof (UINT16),
>+      sizeof (UINT16)
>+    },
>+    NULL
>+  },
> };
>
> UEFI_DEFINED_VARIABLE_ENTRY mGlobalVariableList2[] = {
>diff --git a/MdePkg/Include/Guid/GlobalVariable.h
>b/MdePkg/Include/Guid/GlobalVariable.h
>index 7abc103..06a8a12 100644
>--- a/MdePkg/Include/Guid/GlobalVariable.h
>+++ b/MdePkg/Include/Guid/GlobalVariable.h
>@@ -182,5 +182,12 @@ extern EFI_GUID gEfiGlobalVariableGuid;
> /// Its attribute is BS+RT.
> ///
> #define EFI_VENDOR_KEYS_VARIABLE_NAME               L"VendorKeys"
>+///
>+/// Bitmask of which calls are implemented by the firmware during runtime
>services.
>+/// RT access is required only if GetVariable() is implemented by runtime
>services.
>+/// Should be treated as read-only.
>+/// Its attribute is BS+RT.
>+///
>+#define EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME
>L"RuntimeServicesSupported"
>
> #endif
>--
>2.7.4
>
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable
  2019-11-27 23:25 ` [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set " Jeff Brasen
@ 2019-11-28  5:02   ` Ni, Ray
  2019-11-28  5:29     ` Gao, Zhichao
  0 siblings, 1 reply; 10+ messages in thread
From: Ni, Ray @ 2019-11-28  5:02 UTC (permalink / raw)
  To: Jeff Brasen, devel@edk2.groups.io
  Cc: Gao, Liming, Wu, Hao A, Gao, Zhichao, Kinney, Michael D

Jeff,
I think I forgot to ask a very basic question on the new variable RuntimeServicesSupported.

What if the variable claims SetVariable() is not supported but OS still calls SetVariable()?
I think to behave in a consistent way, SetVariable() should reject to service.
But I cannot find it in your patch.

The similar thing was done for OsIndications variable.
When firmware claims BOOT_TO_SETUP is not supported, the request is ignored even
OS requests to boot to setup,.

I suggest we change all runtime services implementation to return unsupported when
the accordingly bit in the PCD is not set.

Thanks,
Ray


> -----Original Message-----
> From: Jeff Brasen <jbrasen@nvidia.com>
> Sent: Thursday, November 28, 2019 7:25 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> jbrasen@nvidia.com
> Subject: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set
> RuntimeServicesSupported variable
> 
> Add support for initializing and setting the UEFI 2.8 global variable
> RuntimeServicesSupported based on the value of a PCD.
> 
> Change-Id: I8fbd404d492ff8278466edde8aa37d203537318c
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
>  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  1 +
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35
> +++++++++++++++++++++++++++++++-
>  MdePkg/MdePkg.dec                        | 18 ++++++++++++++++
>  MdePkg/MdePkg.uni                        | 17 ++++++++++++++++
>  4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> index 9310b4d..52ec04f 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> @@ -90,6 +90,7 @@
>    gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultPlatformLang         ##
> CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdHardwareErrorRecordLevel                ##
> CONSUMES
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut                     ##
> CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported                ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor                    ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision                  ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand
> ## CONSUMES
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index d387dbe..16bc593 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -40,7 +40,8 @@ CHAR16  *mReadOnlyVariables[] = {
>    EFI_LANG_CODES_VARIABLE_NAME,
>    EFI_BOOT_OPTION_SUPPORT_VARIABLE_NAME,
>    EFI_HW_ERR_REC_SUPPORT_VARIABLE_NAME,
> -  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME
> +  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
> +  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME
>    };
> 
>  CHAR16 *mBdsLoadOptionName[] = {
> @@ -626,6 +627,33 @@ BdsFormalizeOSIndicationVariable (
> 
>  /**
> 
> +  Formalize RuntimeServicesSupported variable.
> +
> +**/
> +VOID
> +BdsFormalizeRuntimeServicesSupportedVariable (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  UINT16                          RuntimeServicesSupported;
> +
> +  RuntimeServicesSupported = PcdGet16 (PcdRuntimeServicesSupported);
> +  Status = gRT->SetVariable (
> +                  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME,
> +                  &gEfiGlobalVariableGuid,
> +                  EFI_VARIABLE_BOOTSERVICE_ACCESS |
> EFI_VARIABLE_RUNTIME_ACCESS,
> +                  sizeof(RuntimeServicesSupported),
> +                  &RuntimeServicesSupported
> +                  );
> +  //
> +  // Platform needs to make sure setting volatile variable before calling 3rd
> party code shouldn't fail.
> +  //
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> +/**
> +
>    Validate variables.
> 
>  **/
> @@ -645,6 +673,11 @@ BdsFormalizeEfiGlobalVariable (
>    // Validate OSIndication related variable.
>    //
>    BdsFormalizeOSIndicationVariable ();
> +
> +  //
> +  // Validate Runtime Services Supported variable.
> +  //
> +  BdsFormalizeRuntimeServicesSupportedVariable ();
>  }
> 
>  /**
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index d022cc5..cdcb2f9 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2297,5 +2297,23 @@
>    # @Prompt Boot Timeout (s)
> 
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x00
> 00002c
> 
> +  ## Bitmask of supported runtime services<BR>
> +  #  BIT0  - GetTime
> +  #  BIT1  - SetTime
> +  #  BIT2  - GetWakeupTime
> +  #  BIT3  - SetWakeupTime
> +  #  BIT4  - GetVariable
> +  #  BIT5  - GetNextVariableName
> +  #  BIT6  - SetVariable
> +  #  BIT7  - SetVirtualAddressMap
> +  #  BIT8  - ConvertPointer
> +  #  BIT9  - GetNextHighMonotonicCount
> +  #  BIT10 - ResetSystem
> +  #  BIT11 - UpdateCapsule
> +  #  BIT12 - QueryCapsuleCapabilites
> +  #  BIT13 - QueryVariableInfo
> +  # @Prompt Supported Runtime services bitmask.
> +
> gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported|0x3FFF|UINT16
> |0x0000002e
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    MdePkgExtra.uni
> diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
> index 5c1fa24..1edf681 100644
> --- a/MdePkg/MdePkg.uni
> +++ b/MdePkg/MdePkg.uni
> @@ -413,3 +413,20 @@
> 
>  #string
> STR_gEfiMdePkgTokenSpaceGuid_PcdUartDefaultReceiveFifoDepth_HELP
> #language en-US "Indicates the receive FIFO depth of UART
> controller.<BR><BR>"
> 
> +#string
> STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_PROMPT
> #language en-US "Supported Runtime Services bitmask"
> +
> +#string
> STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_HELP
> #language en-US "Bitmask of supported runtime services<BR><BR>\n"
> +                                                                                      "BIT0  - GetTime<BR>\n"
> +                                                                                      "BIT1  - SetTime<BR>\n"
> +                                                                                      "BIT2  - GetWakeupTime<BR>\n"
> +                                                                                      "BIT3  - SetWakeupTime<BR>\n"
> +                                                                                      "BIT4  - GetVariable<BR>\n"
> +                                                                                      "BIT5  -
> GetNextVariableName<BR>\n"
> +                                                                                      "BIT6  - SetVariable<BR>\n"
> +                                                                                      "BIT7  -
> SetVirtualAddressMap<BR>\n"
> +                                                                                      "BIT8  - ConvertPointer<BR>\n"
> +                                                                                      "BIT9  -
> GetNextHighMonotonicCount<BR>\n"
> +                                                                                      "BIT10 - ResetSystem<BR>\n"
> +                                                                                      "BIT11 - UpdateCapsule<BR>\n"
> +                                                                                      "BIT12 -
> QueryCapsuleCapabilities<BR>\n"
> +                                                                                      "BIT13 - QueryVariableInfo<BR>"
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable
  2019-11-28  5:02   ` Ni, Ray
@ 2019-11-28  5:29     ` Gao, Zhichao
  2019-12-02 17:51       ` Jeff Brasen
  0 siblings, 1 reply; 10+ messages in thread
From: Gao, Zhichao @ 2019-11-28  5:29 UTC (permalink / raw)
  To: Ni, Ray, Jeff Brasen, devel@edk2.groups.io
  Cc: Gao, Liming, Wu, Hao A, Kinney, Michael D

This bitmask value only affect the runtime service at runtime phase. So the implementation should be after the ExitBootServices() called.
I think the patch set is only implemented the basic setting of the variable but no implementation of the RuntimeServices.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, November 28, 2019 1:02 PM
> To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set
> RuntimeServicesSupported variable
> 
> Jeff,
> I think I forgot to ask a very basic question on the new variable
> RuntimeServicesSupported.
> 
> What if the variable claims SetVariable() is not supported but OS still calls
> SetVariable()?
> I think to behave in a consistent way, SetVariable() should reject to service.
> But I cannot find it in your patch.
> 
> The similar thing was done for OsIndications variable.
> When firmware claims BOOT_TO_SETUP is not supported, the request is ignored
> even OS requests to boot to setup,.
> 
> I suggest we change all runtime services implementation to return unsupported
> when the accordingly bit in the PCD is not set.
> 
> Thanks,
> Ray
> 
> 
> > -----Original Message-----
> > From: Jeff Brasen <jbrasen@nvidia.com>
> > Sent: Thursday, November 28, 2019 7:25 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> > jbrasen@nvidia.com
> > Subject: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set
> > RuntimeServicesSupported variable
> >
> > Add support for initializing and setting the UEFI 2.8 global variable
> > RuntimeServicesSupported based on the value of a PCD.
> >
> > Change-Id: I8fbd404d492ff8278466edde8aa37d203537318c
> > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> > ---
> >  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  1 +
> > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35
> > +++++++++++++++++++++++++++++++-
> >  MdePkg/MdePkg.dec                        | 18 ++++++++++++++++
> >  MdePkg/MdePkg.uni                        | 17 ++++++++++++++++
> >  4 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > index 9310b4d..52ec04f 100644
> > --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > @@ -90,6 +90,7 @@
> >    gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultPlatformLang         ##
> > CONSUMES
> >    gEfiMdePkgTokenSpaceGuid.PcdHardwareErrorRecordLevel                ##
> > CONSUMES
> >    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut                     ##
> > CONSUMES
> > +  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported                ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor                    ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision                  ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand
> > ## CONSUMES
> > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > index d387dbe..16bc593 100644
> > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > @@ -40,7 +40,8 @@ CHAR16  *mReadOnlyVariables[] = {
> >    EFI_LANG_CODES_VARIABLE_NAME,
> >    EFI_BOOT_OPTION_SUPPORT_VARIABLE_NAME,
> >    EFI_HW_ERR_REC_SUPPORT_VARIABLE_NAME,
> > -  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME
> > +  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
> > +  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME
> >    };
> >
> >  CHAR16 *mBdsLoadOptionName[] = {
> > @@ -626,6 +627,33 @@ BdsFormalizeOSIndicationVariable (
> >
> >  /**
> >
> > +  Formalize RuntimeServicesSupported variable.
> > +
> > +**/
> > +VOID
> > +BdsFormalizeRuntimeServicesSupportedVariable (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                      Status;
> > +  UINT16                          RuntimeServicesSupported;
> > +
> > +  RuntimeServicesSupported = PcdGet16 (PcdRuntimeServicesSupported);
> > + Status = gRT->SetVariable (
> > +                  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME,
> > +                  &gEfiGlobalVariableGuid,
> > +                  EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > EFI_VARIABLE_RUNTIME_ACCESS,
> > +                  sizeof(RuntimeServicesSupported),
> > +                  &RuntimeServicesSupported
> > +                  );
> > +  //
> > +  // Platform needs to make sure setting volatile variable before
> > + calling 3rd
> > party code shouldn't fail.
> > +  //
> > +  ASSERT_EFI_ERROR (Status);
> > +}
> > +
> > +/**
> > +
> >    Validate variables.
> >
> >  **/
> > @@ -645,6 +673,11 @@ BdsFormalizeEfiGlobalVariable (
> >    // Validate OSIndication related variable.
> >    //
> >    BdsFormalizeOSIndicationVariable ();
> > +
> > +  //
> > +  // Validate Runtime Services Supported variable.
> > +  //
> > +  BdsFormalizeRuntimeServicesSupportedVariable ();
> >  }
> >
> >  /**
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> > d022cc5..cdcb2f9 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -2297,5 +2297,23 @@
> >    # @Prompt Boot Timeout (s)
> >
> > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x00
> > 00002c
> >
> > +  ## Bitmask of supported runtime services<BR>  #  BIT0  - GetTime  #
> > + BIT1  - SetTime  #  BIT2  - GetWakeupTime  #  BIT3  - SetWakeupTime
> > + #  BIT4  - GetVariable  #  BIT5  - GetNextVariableName  #  BIT6  -
> > + SetVariable  #  BIT7  - SetVirtualAddressMap  #  BIT8  -
> > + ConvertPointer  #  BIT9  - GetNextHighMonotonicCount  #  BIT10 -
> > + ResetSystem  #  BIT11 - UpdateCapsule  #  BIT12 -
> > + QueryCapsuleCapabilites  #  BIT13 - QueryVariableInfo  # @Prompt
> > + Supported Runtime services bitmask.
> > +
> > gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported|0x3FFF|UINT16
> > |0x0000002e
> > +
> >  [UserExtensions.TianoCore."ExtraFiles"]
> >    MdePkgExtra.uni
> > diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni index
> > 5c1fa24..1edf681 100644
> > --- a/MdePkg/MdePkg.uni
> > +++ b/MdePkg/MdePkg.uni
> > @@ -413,3 +413,20 @@
> >
> >  #string
> > STR_gEfiMdePkgTokenSpaceGuid_PcdUartDefaultReceiveFifoDepth_HELP
> > #language en-US "Indicates the receive FIFO depth of UART
> > controller.<BR><BR>"
> >
> > +#string
> > STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_PROMPT
> > #language en-US "Supported Runtime Services bitmask"
> > +
> > +#string
> > STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_HELP
> > #language en-US "Bitmask of supported runtime services<BR><BR>\n"
> > +                                                                                      "BIT0  - GetTime<BR>\n"
> > +                                                                                      "BIT1  - SetTime<BR>\n"
> > +                                                                                      "BIT2  -
> GetWakeupTime<BR>\n"
> > +                                                                                      "BIT3  -
> SetWakeupTime<BR>\n"
> > +                                                                                      "BIT4  - GetVariable<BR>\n"
> > +
> > + "BIT5  -
> > GetNextVariableName<BR>\n"
> > +                                                                                      "BIT6  - SetVariable<BR>\n"
> > +
> > + "BIT7  -
> > SetVirtualAddressMap<BR>\n"
> > +                                                                                      "BIT8  -
> ConvertPointer<BR>\n"
> > +
> > + "BIT9  -
> > GetNextHighMonotonicCount<BR>\n"
> > +                                                                                      "BIT10 - ResetSystem<BR>\n"
> > +                                                                                      "BIT11 -
> UpdateCapsule<BR>\n"
> > +
> > + "BIT12 -
> > QueryCapsuleCapabilities<BR>\n"
> > +                                                                                      "BIT13 -
> QueryVariableInfo<BR>"
> > --
> > 2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable
  2019-11-28  5:29     ` Gao, Zhichao
@ 2019-12-02 17:51       ` Jeff Brasen
  0 siblings, 0 replies; 10+ messages in thread
From: Jeff Brasen @ 2019-12-02 17:51 UTC (permalink / raw)
  To: Gao, Zhichao, Ni, Ray, devel@edk2.groups.io
  Cc: Gao, Liming, Wu, Hao A, Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 9734 bytes --]

I will work on a patch to variable services and other runtime services that have centrallized implementations to have them check the value of this variable during ExitBootServices and then return unsupported if the bit is not set.


Thanks,

Jeff

________________________________
From: Gao, Zhichao <zhichao.gao@intel.com>
Sent: Wednesday, November 27, 2019 10:29 PM
To: Ni, Ray <ray.ni@intel.com>; Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Gao, Liming <liming.gao@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable

This bitmask value only affect the runtime service at runtime phase. So the implementation should be after the ExitBootServices() called.
I think the patch set is only implemented the basic setting of the variable but no implementation of the RuntimeServices.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, November 28, 2019 1:02 PM
> To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set
> RuntimeServicesSupported variable
>
> Jeff,
> I think I forgot to ask a very basic question on the new variable
> RuntimeServicesSupported.
>
> What if the variable claims SetVariable() is not supported but OS still calls
> SetVariable()?
> I think to behave in a consistent way, SetVariable() should reject to service.
> But I cannot find it in your patch.
>
> The similar thing was done for OsIndications variable.
> When firmware claims BOOT_TO_SETUP is not supported, the request is ignored
> even OS requests to boot to setup,.
>
> I suggest we change all runtime services implementation to return unsupported
> when the accordingly bit in the PCD is not set.
>
> Thanks,
> Ray
>
>
> > -----Original Message-----
> > From: Jeff Brasen <jbrasen@nvidia.com>
> > Sent: Thursday, November 28, 2019 7:25 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> > jbrasen@nvidia.com
> > Subject: [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set
> > RuntimeServicesSupported variable
> >
> > Add support for initializing and setting the UEFI 2.8 global variable
> > RuntimeServicesSupported based on the value of a PCD.
> >
> > Change-Id: I8fbd404d492ff8278466edde8aa37d203537318c
> > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> > ---
> >  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  1 +
> > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35
> > +++++++++++++++++++++++++++++++-
> >  MdePkg/MdePkg.dec                        | 18 ++++++++++++++++
> >  MdePkg/MdePkg.uni                        | 17 ++++++++++++++++
> >  4 files changed, 70 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > index 9310b4d..52ec04f 100644
> > --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > @@ -90,6 +90,7 @@
> >    gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultPlatformLang         ##
> > CONSUMES
> >    gEfiMdePkgTokenSpaceGuid.PcdHardwareErrorRecordLevel                ##
> > CONSUMES
> >    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut                     ##
> > CONSUMES
> > +  gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported                ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor                    ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision                  ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand
> > ## CONSUMES
> > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > index d387dbe..16bc593 100644
> > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> > @@ -40,7 +40,8 @@ CHAR16  *mReadOnlyVariables[] = {
> >    EFI_LANG_CODES_VARIABLE_NAME,
> >    EFI_BOOT_OPTION_SUPPORT_VARIABLE_NAME,
> >    EFI_HW_ERR_REC_SUPPORT_VARIABLE_NAME,
> > -  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME
> > +  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
> > +  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME
> >    };
> >
> >  CHAR16 *mBdsLoadOptionName[] = {
> > @@ -626,6 +627,33 @@ BdsFormalizeOSIndicationVariable (
> >
> >  /**
> >
> > +  Formalize RuntimeServicesSupported variable.
> > +
> > +**/
> > +VOID
> > +BdsFormalizeRuntimeServicesSupportedVariable (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                      Status;
> > +  UINT16                          RuntimeServicesSupported;
> > +
> > +  RuntimeServicesSupported = PcdGet16 (PcdRuntimeServicesSupported);
> > + Status = gRT->SetVariable (
> > +                  EFI_RUNTIME_SERVICES_SUPPORTED_VARIABLE_NAME,
> > +                  &gEfiGlobalVariableGuid,
> > +                  EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > EFI_VARIABLE_RUNTIME_ACCESS,
> > +                  sizeof(RuntimeServicesSupported),
> > +                  &RuntimeServicesSupported
> > +                  );
> > +  //
> > +  // Platform needs to make sure setting volatile variable before
> > + calling 3rd
> > party code shouldn't fail.
> > +  //
> > +  ASSERT_EFI_ERROR (Status);
> > +}
> > +
> > +/**
> > +
> >    Validate variables.
> >
> >  **/
> > @@ -645,6 +673,11 @@ BdsFormalizeEfiGlobalVariable (
> >    // Validate OSIndication related variable.
> >    //
> >    BdsFormalizeOSIndicationVariable ();
> > +
> > +  //
> > +  // Validate Runtime Services Supported variable.
> > +  //
> > +  BdsFormalizeRuntimeServicesSupportedVariable ();
> >  }
> >
> >  /**
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> > d022cc5..cdcb2f9 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -2297,5 +2297,23 @@
> >    # @Prompt Boot Timeout (s)
> >
> > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x00
> > 00002c
> >
> > +  ## Bitmask of supported runtime services<BR>  #  BIT0  - GetTime  #
> > + BIT1  - SetTime  #  BIT2  - GetWakeupTime  #  BIT3  - SetWakeupTime
> > + #  BIT4  - GetVariable  #  BIT5  - GetNextVariableName  #  BIT6  -
> > + SetVariable  #  BIT7  - SetVirtualAddressMap  #  BIT8  -
> > + ConvertPointer  #  BIT9  - GetNextHighMonotonicCount  #  BIT10 -
> > + ResetSystem  #  BIT11 - UpdateCapsule  #  BIT12 -
> > + QueryCapsuleCapabilites  #  BIT13 - QueryVariableInfo  # @Prompt
> > + Supported Runtime services bitmask.
> > +
> > gEfiMdePkgTokenSpaceGuid.PcdRuntimeServicesSupported|0x3FFF|UINT16
> > |0x0000002e
> > +
> >  [UserExtensions.TianoCore."ExtraFiles"]
> >    MdePkgExtra.uni
> > diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni index
> > 5c1fa24..1edf681 100644
> > --- a/MdePkg/MdePkg.uni
> > +++ b/MdePkg/MdePkg.uni
> > @@ -413,3 +413,20 @@
> >
> >  #string
> > STR_gEfiMdePkgTokenSpaceGuid_PcdUartDefaultReceiveFifoDepth_HELP
> > #language en-US "Indicates the receive FIFO depth of UART
> > controller.<BR><BR>"
> >
> > +#string
> > STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_PROMPT
> > #language en-US "Supported Runtime Services bitmask"
> > +
> > +#string
> > STR_gEfiMdePkgTokenSpaceGuid_PcdRuntimeServicesSupported_HELP
> > #language en-US "Bitmask of supported runtime services<BR><BR>\n"
> > +                                                                                      "BIT0  - GetTime<BR>\n"
> > +                                                                                      "BIT1  - SetTime<BR>\n"
> > +                                                                                      "BIT2  -
> GetWakeupTime<BR>\n"
> > +                                                                                      "BIT3  -
> SetWakeupTime<BR>\n"
> > +                                                                                      "BIT4  - GetVariable<BR>\n"
> > +
> > + "BIT5  -
> > GetNextVariableName<BR>\n"
> > +                                                                                      "BIT6  - SetVariable<BR>\n"
> > +
> > + "BIT7  -
> > SetVirtualAddressMap<BR>\n"
> > +                                                                                      "BIT8  -
> ConvertPointer<BR>\n"
> > +
> > + "BIT9  -
> > GetNextHighMonotonicCount<BR>\n"
> > +                                                                                      "BIT10 - ResetSystem<BR>\n"
> > +                                                                                      "BIT11 -
> UpdateCapsule<BR>\n"
> > +
> > + "BIT12 -
> > QueryCapsuleCapabilities<BR>\n"
> > +                                                                                      "BIT13 -
> QueryVariableInfo<BR>"
> > --
> > 2.7.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[-- Attachment #2: Type: text/html, Size: 19857 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v2 0/3] Support for RuntimeServicesSupported global variable
  2019-11-27 23:25 [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Jeff Brasen
                   ` (2 preceding siblings ...)
  2019-11-27 23:25 ` [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set " Jeff Brasen
@ 2019-12-17  8:17 ` Ard Biesheuvel
  3 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2019-12-17  8:17 UTC (permalink / raw)
  To: edk2-devel-groups-io, Jeff Brasen
  Cc: Gao, Liming, Kinney, Michael D, Wu, Hao A, Ni, Ray, Zhichao Gao

On Thu, 28 Nov 2019 at 01:25, Jeff Brasen <jbrasen@nvidia.com> wrote:
>
>  Add support for the new UEFI 2.8 runtime services supported variable
>  that is used to indicate which runtime services a platform supports.
>  Also, add support for initializing this variable based on a PCD.
>
> Change Log:
> v1 - Initial version
> v2 - Move pcd from MdeModulePkg to MdePkg and update uni file
>
>
> Jeff Brasen (3):
>   MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations
>   MdePkg/MdeModule: Add support for RuntimeServicesSupported variable
>   MdeModulePkg/BdsDxe: Set RuntimeServicesSupported variable
>

Hi Jeff,

Apologies if I am derailing your attempt at getting this support
merged, but I am actually proposing to the USWG to make this a config
table rather than a EFI variable, for the obvious reason that using a
runtime service to decide which runtime services are supported is a
bit problematic, and it forces the OS to invent its own way to
exchange this information between the OS loader and the OS proper.

A configuration table does not have this limitation, so it seems much
better suited for this.


>  .../VarCheckUefiLib/VarCheckUefiLibNullClass.c     | 11 +++++++
>  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf           |  1 +
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c           | 35 +++++++++++++++++++++-
>  MdePkg/Include/Guid/GlobalVariable.h               |  7 +++++
>  MdePkg/Include/Uefi/UefiSpec.h                     | 18 +++++++++++
>  MdePkg/MdePkg.dec                                  | 18 +++++++++++
>  MdePkg/MdePkg.uni                                  | 17 +++++++++++
>  7 files changed, 106 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>
>
> 
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-12-17  8:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-27 23:25 [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Jeff Brasen
2019-11-27 23:25 ` [PATCH v2 1/3] MdePkg-UefiSpec.h: Add UEFI 2.8 RuntimeServicesSuppported definations Jeff Brasen
2019-11-28  0:33   ` [edk2-devel] " Liming Gao
2019-11-27 23:25 ` [PATCH v2 2/3] MdePkg/MdeModule: Add support for RuntimeServicesSupported variable Jeff Brasen
2019-11-28  0:34   ` [edk2-devel] " Liming Gao
2019-11-27 23:25 ` [PATCH v2 3/3] MdeModulePkg/BdsDxe: Set " Jeff Brasen
2019-11-28  5:02   ` Ni, Ray
2019-11-28  5:29     ` Gao, Zhichao
2019-12-02 17:51       ` Jeff Brasen
2019-12-17  8:17 ` [edk2-devel] [PATCH v2 0/3] Support for RuntimeServicesSupported global variable Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox