* [PATCH V1 0/1] Add EDKII_PEI_VARIABLE_PPI
@ 2022-06-10 1:17 Nate DeSimone
2022-06-10 1:17 ` [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Nate DeSimone
0 siblings, 1 reply; 13+ messages in thread
From: Nate DeSimone @ 2022-06-10 1:17 UTC (permalink / raw)
To: devel
Cc: Jian J Wang, Liming Gao, Michael D Kinney, Isaac Oram,
Chasel Chiu, Gao Cheng, Di Zhang, Daocheng Bu, Michael Kubacki
This patch series proposes a new architectural PPI: EDKII_PEI_VARIABLE_PPI
This PPI provides services to read and write to UEFI variables during the PEI
environment. While an implementation of this PPI does not exist yet, Intel is
working on a new implementation of the PEI variable services that includes write
support.
As this is essentially a PEI version of the EFI_SMM_VARIABLE_PROTOCOL, I don't
expect it to be immensely controversial. Regardless, before this implementation
is done, we would like to provide the community with an opportunity to provide
input the on new architectural PPI.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Gao Cheng <gao.cheng@intel.com>
Cc: Di Zhang <di.zhang@intel.com>
Cc: Daocheng Bu <daocheng.bu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Nate DeSimone (1):
MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
MdeModulePkg/Include/Ppi/Variable.h | 189 ++++++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
2 files changed, 192 insertions(+)
create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
--
2.27.0.windows.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 1:17 [PATCH V1 0/1] Add EDKII_PEI_VARIABLE_PPI Nate DeSimone
@ 2022-06-10 1:17 ` Nate DeSimone
2022-06-10 1:59 ` [edk2-devel] " Michael Kubacki
0 siblings, 1 reply; 13+ messages in thread
From: Nate DeSimone @ 2022-06-10 1:17 UTC (permalink / raw)
To: devel
Cc: Jian J Wang, Liming Gao, Michael D Kinney, Isaac Oram,
Chasel Chiu, Gao Cheng, Di Zhang, Daocheng Bu, Michael Kubacki
Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor
to enabling variable writes in the PEI environment.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Gao Cheng <gao.cheng@intel.com>
Cc: Di Zhang <di.zhang@intel.com>
Cc: Daocheng Bu <daocheng.bu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
MdeModulePkg/Include/Ppi/Variable.h | 189 ++++++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
2 files changed, 192 insertions(+)
create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
diff --git a/MdeModulePkg/Include/Ppi/Variable.h b/MdeModulePkg/Include/Ppi/Variable.h
new file mode 100644
index 0000000000..97dc7ceefa
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/Variable.h
@@ -0,0 +1,189 @@
+/** @file
+ EDKII PEI Variable Protocol provides an implementation of variables
+ intended for use as a means to store data in the PEI environment.
+
+ Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __PEI_VARIABLE_PPI_H_
+#define __PEI_VARIABLE_PPI_H_
+
+#define EDKII_PEI_VARIABLE_PPI_GUID \
+ { \
+ 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } \
+ }
+
+typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
+
+/**
+ This service retrieves a variable's value using its name and GUID.
+
+ Read the specified variable from the UEFI variable store. If the Data
+ buffer is too small to hold the contents of the variable,
+ the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to the
+ required buffer size to obtain the data.
+
+ @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
+ @param[in] VariableName A pointer to a null-terminated string that is the variable's name.
+ @param[in] VariableGuid A pointer to an EFI_GUID that is the variable's GUID. The combination of
+ VariableGuid and VariableName must be unique.
+ @param[out] Attributes If non-NULL, on return, points to the variable's attributes.
+ @param[in, out] DataSize On entry, points to the size in bytes of the Data buffer.
+ On return, points to the size of the data returned in Data.
+ @param[out] Data Points to the buffer which will hold the returned variable value.
+ May be NULL with a zero DataSize in order to determine the size of the
+ buffer needed.
+
+ @retval EFI_SUCCESS The variable was read successfully.
+ @retval EFI_NOT_FOUND The variable was not found.
+ @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the resulting data.
+ DataSize is updated with the size required for
+ the specified variable.
+ @retval EFI_INVALID_PARAMETER VariableName, VariableGuid, DataSize or Data is NULL.
+ @retval EFI_DEVICE_ERROR The variable could not be retrieved because of a device error.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_GET_VARIABLE)(
+ IN CONST EDKII_PEI_VARIABLE_PPI *This,
+ IN CONST CHAR16 *VariableName,
+ IN CONST EFI_GUID *VariableGuid,
+ OUT UINT32 *Attributes,
+ IN OUT UINTN *DataSize,
+ OUT VOID *Data OPTIONAL
+ );
+
+/**
+ Return the next variable name and GUID.
+
+ This function is called multiple times to retrieve the VariableName
+ and VariableGuid of all variables currently available in the system.
+ On each call, the previous results are passed into the interface,
+ and, on return, the interface returns the data for the next
+ interface. When the entire variable list has been returned,
+ EFI_NOT_FOUND is returned.
+
+ @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
+ @param[in, out] VariableNameSize On entry, points to the size of the buffer pointed to by VariableName.
+ On return, the size of the variable name buffer.
+ @param[in, out] VariableName On entry, a pointer to a null-terminated string that is the variable's name.
+ On return, points to the next variable's null-terminated name string.
+ @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that is the variable's GUID.
+ On return, a pointer to the next variable's GUID.
+
+ @retval EFI_SUCCESS The variable was read successfully.
+ @retval EFI_NOT_FOUND The variable could not be found.
+ @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small for the resulting
+ data. VariableNameSize is updated with the size
+ required for the specified variable.
+ @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
+ VariableNameSize is NULL.
+ @retval EFI_DEVICE_ERROR The variable could not be retrieved because of a device error.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
+ IN CONST EDKII_PEI_VARIABLE_PPI *This,
+ IN OUT UINTN *VariableNameSize,
+ IN OUT CHAR16 *VariableName,
+ IN OUT EFI_GUID *VariableGuid
+ );
+
+/**
+ Sets the value of a variable.
+
+ @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
+ @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
+ Each VariableName is unique for each VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+ @param[in] VendorGuid A unique identifier for the vendor.
+ @param[in] Attributes Attributes bitmask to set for the variable.
+ @param[in] DataSize The size in bytes of the Data buffer. Unless the EFI_VARIABLE_APPEND_WRITE
+ attribute is set, a size of zero causes the variable to be deleted. When the
+ EFI_VARIABLE_APPEND_WRITE attribute is set, then a SetVariable() call with a
+ DataSize of zero will not cause any change to the variable value.
+ @param[in] Data The contents for the variable.
+
+ @retval EFI_SUCCESS The firmware has successfully stored the variable and its data as
+ defined by the Attributes.
+ @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits, name, and GUID was supplied, or the
+ DataSize exceeds the maximum allowed.
+ @retval EFI_INVALID_PARAMETER VariableName is an empty string.
+ @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold the variable and its data.
+ @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
+ @retval EFI_WRITE_PROTECTED The variable in question is read-only.
+ @retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
+ @retval EFI_SECURITY_VIOLATION The variable could not be written due to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
+ or EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
+ EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS being set. Writing to authenticated
+ variables is not supported in the PEI environment. Updates to authenticated
+ variables can be requested during PEI via the EFI_AUTHENTICATED_VARIABLE_HOB, but
+ these updates won't be written to non-volatile storage until later in DXE. See
+ MdeModulePkg/Include/Guid/VariableFormat.h for more details on
+ EFI_AUTHENTICATED_VARIABLE_HOB.
+ @retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_SET_VARIABLE)(
+ IN CONST EDKII_PEI_VARIABLE_PPI *This,
+ IN CHAR16 *VariableName,
+ IN EFI_GUID *VendorGuid,
+ IN UINT32 Attributes,
+ IN UINTN DataSize,
+ IN VOID *Data
+ );
+
+/**
+ Returns information about the UEFI variables.
+
+ @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
+ @param[in] Attributes Attributes bitmask to specify the type of variables on
+ which to return information.
+ @param[out] MaximumVariableStorageSize On output the maximum size of the storage space
+ available for the EFI variables associated with the
+ attributes specified.
+ @param[out] RemainingVariableStorageSize Returns the remaining size of the storage space
+ available for the EFI variables associated with the
+ attributes specified.
+ @param[out] MaximumVariableSize Returns the maximum size of the individual EFI
+ variables associated with the attributes specified.
+
+ @retval EFI_SUCCESS Valid answer returned.
+ @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits was supplied
+ @retval EFI_UNSUPPORTED The attribute is not supported on this platform, and the
+ MaximumVariableStorageSize,
+ RemainingVariableStorageSize, MaximumVariableSize
+ are undefined.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
+ IN CONST EDKII_PEI_VARIABLE_PPI *This,
+ IN UINT32 Attributes,
+ OUT UINT64 *MaximumVariableStorageSize,
+ OUT UINT64 *RemainingVariableStorageSize,
+ OUT UINT64 *MaximumVariableSize
+ );
+
+///
+/// PEI Variable Protocol is intended for use as a means
+/// to store data in the PEI environment.
+///
+struct _EDKII_PEI_VARIABLE_PPI {
+ EDKII_PEI_GET_VARIABLE GetVariable;
+ EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
+ EDKII_PEI_SET_VARIABLE SetVariable;
+ EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
+};
+
+extern EFI_GUID gEdkiiPeiVariablePpiGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 2bcb9f9453..4f4c48b81f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -513,6 +513,9 @@
gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d, { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7, 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
+ ## Include/Ppi/Variable.h
+ gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
+
[Protocols]
## Load File protocol provides capability to load and unload EFI image into memory and execute it.
# Include/Protocol/LoadPe32Image.h
--
2.27.0.windows.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 1:17 ` [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Nate DeSimone
@ 2022-06-10 1:59 ` Michael Kubacki
2022-06-10 2:07 ` Michael Kubacki
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Michael Kubacki @ 2022-06-10 1:59 UTC (permalink / raw)
To: devel, nathaniel.l.desimone
Cc: Jian J Wang, Liming Gao, Michael D Kinney, Isaac Oram,
Chasel Chiu, Gao Cheng, Di Zhang, Daocheng Bu, Michael Kubacki
Is this change just adding the interface to Tianocore or is there
additional implementation planned as well?
---
I thought we were following this convention now:
"#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
Some other comments are inline.
Regards,
Michael
On 6/9/2022 9:17 PM, Nate DeSimone wrote:
> Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor
> to enabling variable writes in the PEI environment.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Gao Cheng <gao.cheng@intel.com>
> Cc: Di Zhang <di.zhang@intel.com>
> Cc: Daocheng Bu <daocheng.bu@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
> MdeModulePkg/Include/Ppi/Variable.h | 189 ++++++++++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 2 files changed, 192 insertions(+)
> create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
>
> diff --git a/MdeModulePkg/Include/Ppi/Variable.h b/MdeModulePkg/Include/Ppi/Variable.h
> new file mode 100644
> index 0000000000..97dc7ceefa
> --- /dev/null
> +++ b/MdeModulePkg/Include/Ppi/Variable.h
> @@ -0,0 +1,189 @@
> +/** @file
> + EDKII PEI Variable Protocol provides an implementation of variables
[MK] Was "EDKII PEI Variable PPI" intended?
> + intended for use as a means to store data in the PEI environment.
> +
> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __PEI_VARIABLE_PPI_H_
> +#define __PEI_VARIABLE_PPI_H_
> +
> +#define EDKII_PEI_VARIABLE_PPI_GUID \
> + { \
> + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } \
> + }
> +
> +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
> +
> +/**
> + This service retrieves a variable's value using its name and GUID.
> +
> + Read the specified variable from the UEFI variable store. If the Data
> + buffer is too small to hold the contents of the variable,
> + the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to the
> + required buffer size to obtain the data.
> +
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in] VariableName A pointer to a null-terminated string that is the variable's name.
> + @param[in] VariableGuid A pointer to an EFI_GUID that is the variable's GUID. The combination of
> + VariableGuid and VariableName must be unique.
> + @param[out] Attributes If non-NULL, on return, points to the variable's attributes.
> + @param[in, out] DataSize On entry, points to the size in bytes of the Data buffer.
> + On return, points to the size of the data returned in Data.
> + @param[out] Data Points to the buffer which will hold the returned variable value.
> + May be NULL with a zero DataSize in order to determine the size of the
> + buffer needed.
> +
> + @retval EFI_SUCCESS The variable was read successfully.
> + @retval EFI_NOT_FOUND The variable was not found.
> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the resulting data.
> + DataSize is updated with the size required for
> + the specified variable.
> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid, DataSize or Data is NULL.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved because of a device error.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN CONST CHAR16 *VariableName,
> + IN CONST EFI_GUID *VariableGuid,
> + OUT UINT32 *Attributes,
[MK] Based on the description, Attributes should be marked "OPTIONAL".
> + IN OUT UINTN *DataSize,
> + OUT VOID *Data OPTIONAL
> + );
> +
> +/**
> + Return the next variable name and GUID.
> +
> + This function is called multiple times to retrieve the VariableName
> + and VariableGuid of all variables currently available in the system.
> + On each call, the previous results are passed into the interface,
> + and, on return, the interface returns the data for the next
> + interface. When the entire variable list has been returned,
> + EFI_NOT_FOUND is returned.
> +
[MK] I know other descriptions don't usually have it but it would be
nice to describe the initial calling values expected.
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in, out] VariableNameSize On entry, points to the size of the buffer pointed to by VariableName.
> + On return, the size of the variable name buffer.
> + @param[in, out] VariableName On entry, a pointer to a null-terminated string that is the variable's name.
> + On return, points to the next variable's null-terminated name string.
> + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that is the variable's GUID.
> + On return, a pointer to the next variable's GUID.
> +
> + @retval EFI_SUCCESS The variable was read successfully.
> + @retval EFI_NOT_FOUND The variable could not be found.
> + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small for the resulting
> + data. VariableNameSize is updated with the size
> + required for the specified variable.
> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
> + VariableNameSize is NULL.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved because of a device error.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN OUT UINTN *VariableNameSize,
> + IN OUT CHAR16 *VariableName,
> + IN OUT EFI_GUID *VariableGuid
> + );
> +
> +/**
> + Sets the value of a variable.
> +
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
> + Each VariableName is unique for each VendorGuid. VariableName must
> + contain 1 or more characters. If VariableName is an empty string,
> + then EFI_INVALID_PARAMETER is returned.
> + @param[in] VendorGuid A unique identifier for the vendor.
> + @param[in] Attributes Attributes bitmask to set for the variable.
> + @param[in] DataSize The size in bytes of the Data buffer. Unless the EFI_VARIABLE_APPEND_WRITE
> + attribute is set, a size of zero causes the variable to be deleted. When the
> + EFI_VARIABLE_APPEND_WRITE attribute is set, then a SetVariable() call with a
> + DataSize of zero will not cause any change to the variable value.
> + @param[in] Data The contents for the variable.
> +
> + @retval EFI_SUCCESS The firmware has successfully stored the variable and its data as
> + defined by the Attributes.
> + @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits, name, and GUID was supplied, or the
> + DataSize exceeds the maximum allowed.
> + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
> + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold the variable and its data.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
> + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> + @retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
> + @retval EFI_SECURITY_VIOLATION The variable could not be written due to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
> + or EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
> + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS being set. Writing to authenticated
> + variables is not supported in the PEI environment. Updates to authenticated
> + variables can be requested during PEI via the EFI_AUTHENTICATED_VARIABLE_HOB, but
> + these updates won't be written to non-volatile storage until later in DXE. See
> + MdeModulePkg/Include/Guid/VariableFormat.h for more details on
> + EFI_AUTHENTICATED_VARIABLE_HOB.
[MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
VariableFormat.h.
[MK] It seems that if a contract for producing and then consuming this
HOB is going to be defined between the HOB producer and consumer phase,
it should be described in something like the PI Spec.
> + @retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN CHAR16 *VariableName,
> + IN EFI_GUID *VendorGuid,
> + IN UINT32 Attributes,
> + IN UINTN DataSize,
> + IN VOID *Data
> + );
> +
> +/**
> + Returns information about the UEFI variables.
> +
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in] Attributes Attributes bitmask to specify the type of variables on
> + which to return information.
> + @param[out] MaximumVariableStorageSize On output the maximum size of the storage space
> + available for the EFI variables associated with the
> + attributes specified.
> + @param[out] RemainingVariableStorageSize Returns the remaining size of the storage space
> + available for the EFI variables associated with the
> + attributes specified.
> + @param[out] MaximumVariableSize Returns the maximum size of the individual EFI
> + variables associated with the attributes specified.
> +
> + @retval EFI_SUCCESS Valid answer returned.
> + @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits was supplied
> + @retval EFI_UNSUPPORTED The attribute is not supported on this platform, and the
> + MaximumVariableStorageSize,
> + RemainingVariableStorageSize, MaximumVariableSize
> + are undefined.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN UINT32 Attributes,
> + OUT UINT64 *MaximumVariableStorageSize,
> + OUT UINT64 *RemainingVariableStorageSize,
> + OUT UINT64 *MaximumVariableSize
> + );
> +
> +///
> +/// PEI Variable Protocol is intended for use as a means
[MK] Was "PEI Variable PPI" intended?
> +/// to store data in the PEI environment.
> +///
> +struct _EDKII_PEI_VARIABLE_PPI {
> + EDKII_PEI_GET_VARIABLE GetVariable;
> + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
> + EDKII_PEI_SET_VARIABLE SetVariable;
> + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
> +};
> +
> +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
> +
> +#endif
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 2bcb9f9453..4f4c48b81f 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -513,6 +513,9 @@
> gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d, { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7, 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
>
> + ## Include/Ppi/Variable.h
> + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
> +
> [Protocols]
> ## Load File protocol provides capability to load and unload EFI image into memory and execute it.
> # Include/Protocol/LoadPe32Image.h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 1:59 ` [edk2-devel] " Michael Kubacki
@ 2022-06-10 2:07 ` Michael Kubacki
2022-06-10 16:56 ` Yao, Jiewen
2022-06-10 21:20 ` Nate DeSimone
2 siblings, 0 replies; 13+ messages in thread
From: Michael Kubacki @ 2022-06-10 2:07 UTC (permalink / raw)
To: devel, nathaniel.l.desimone
Cc: Jian J Wang, Liming Gao, Michael D Kinney, Isaac Oram,
Chasel Chiu, Gao Cheng, Di Zhang, Daocheng Bu, Michael Kubacki
I just saw the comment in the cover letter about the additional
implementation.
Thanks,
Michael
On 6/9/2022 9:59 PM, Michael Kubacki wrote:
> Is this change just adding the interface to Tianocore or is there
> additional implementation planned as well?
>
> ---
>
> I thought we were following this convention now:
>
> "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
>
> Some other comments are inline.
>
> Regards,
> Michael
>
> On 6/9/2022 9:17 PM, Nate DeSimone wrote:
>> Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor
>> to enabling variable writes in the PEI environment.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Isaac Oram <isaac.w.oram@intel.com>
>> Cc: Chasel Chiu <chasel.chiu@intel.com>
>> Cc: Gao Cheng <gao.cheng@intel.com>
>> Cc: Di Zhang <di.zhang@intel.com>
>> Cc: Daocheng Bu <daocheng.bu@intel.com>
>> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
>> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
>> ---
>> MdeModulePkg/Include/Ppi/Variable.h | 189 ++++++++++++++++++++++++++++
>> MdeModulePkg/MdeModulePkg.dec | 3 +
>> 2 files changed, 192 insertions(+)
>> create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
>>
>> diff --git a/MdeModulePkg/Include/Ppi/Variable.h
>> b/MdeModulePkg/Include/Ppi/Variable.h
>> new file mode 100644
>> index 0000000000..97dc7ceefa
>> --- /dev/null
>> +++ b/MdeModulePkg/Include/Ppi/Variable.h
>> @@ -0,0 +1,189 @@
>> +/** @file
>> + EDKII PEI Variable Protocol provides an implementation of variables
>
> [MK] Was "EDKII PEI Variable PPI" intended?
>
>> + intended for use as a means to store data in the PEI environment.
>> +
>> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef __PEI_VARIABLE_PPI_H_
>> +#define __PEI_VARIABLE_PPI_H_
>> +
>> +#define EDKII_PEI_VARIABLE_PPI_GUID \
>> + { \
>> + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66,
>> 0x4a, 0xb0 } \
>> + }
>> +
>> +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
>> +
>> +/**
>> + This service retrieves a variable's value using its name and GUID.
>> +
>> + Read the specified variable from the UEFI variable store. If the Data
>> + buffer is too small to hold the contents of the variable,
>> + the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to the
>> + required buffer size to obtain the data.
>> +
>> + @param[in] This A pointer to this instance of
>> the EDKII_PEI_VARIABLE_PPI.
>> + @param[in] VariableName A pointer to a null-terminated
>> string that is the variable's name.
>> + @param[in] VariableGuid A pointer to an EFI_GUID that
>> is the variable's GUID. The combination of
>> + VariableGuid and VariableName
>> must be unique.
>> + @param[out] Attributes If non-NULL, on return, points
>> to the variable's attributes.
>> + @param[in, out] DataSize On entry, points to the size in
>> bytes of the Data buffer.
>> + On return, points to the size
>> of the data returned in Data.
>> + @param[out] Data Points to the buffer which will
>> hold the returned variable value.
>> + May be NULL with a zero
>> DataSize in order to determine the size of the
>> + buffer needed.
>> +
>> + @retval EFI_SUCCESS The variable was read
>> successfully.
>> + @retval EFI_NOT_FOUND The variable was not found.
>> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for
>> the resulting data.
>> + DataSize is updated with the
>> size required for
>> + the specified variable.
>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
>> DataSize or Data is NULL.
>> + @retval EFI_DEVICE_ERROR The variable could not be
>> retrieved because of a device error.
>> +
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>> + IN CONST CHAR16 *VariableName,
>> + IN CONST EFI_GUID *VariableGuid,
>> + OUT UINT32 *Attributes,
>
> [MK] Based on the description, Attributes should be marked "OPTIONAL".
>
>> + IN OUT UINTN *DataSize,
>> + OUT VOID *Data OPTIONAL
>> + );
>> +
>> +/**
>> + Return the next variable name and GUID.
>> +
>> + This function is called multiple times to retrieve the VariableName
>> + and VariableGuid of all variables currently available in the system.
>> + On each call, the previous results are passed into the interface,
>> + and, on return, the interface returns the data for the next
>> + interface. When the entire variable list has been returned,
>> + EFI_NOT_FOUND is returned.
>> +
>
> [MK] I know other descriptions don't usually have it but it would be
> nice to describe the initial calling values expected.
>
>> + @param[in] This A pointer to this instance of
>> the EDKII_PEI_VARIABLE_PPI.
>> + @param[in, out] VariableNameSize On entry, points to the size of
>> the buffer pointed to by VariableName.
>> + On return, the size of the
>> variable name buffer.
>> + @param[in, out] VariableName On entry, a pointer to a
>> null-terminated string that is the variable's name.
>> + On return, points to the next
>> variable's null-terminated name string.
>> + @param[in, out] VariableGuid On entry, a pointer to an
>> EFI_GUID that is the variable's GUID.
>> + On return, a pointer to the
>> next variable's GUID.
>> +
>> + @retval EFI_SUCCESS The variable was read
>> successfully.
>> + @retval EFI_NOT_FOUND The variable could not be found.
>> + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too
>> small for the resulting
>> + data. VariableNameSize is
>> updated with the size
>> + required for the specified
>> variable.
>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
>> + VariableNameSize is NULL.
>> + @retval EFI_DEVICE_ERROR The variable could not be
>> retrieved because of a device error.
>> +
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>> + IN OUT UINTN *VariableNameSize,
>> + IN OUT CHAR16 *VariableName,
>> + IN OUT EFI_GUID *VariableGuid
>> + );
>> +
>> +/**
>> + Sets the value of a variable.
>> +
>> + @param[in] This A pointer to this instance of
>> the EDKII_PEI_VARIABLE_PPI.
>> + @param[in] VariableName A Null-terminated string that
>> is the name of the vendor's variable.
>> + Each VariableName is unique for
>> each VendorGuid. VariableName must
>> + contain 1 or more characters.
>> If VariableName is an empty string,
>> + then EFI_INVALID_PARAMETER is
>> returned.
>> + @param[in] VendorGuid A unique identifier for the
>> vendor.
>> + @param[in] Attributes Attributes bitmask to set for
>> the variable.
>> + @param[in] DataSize The size in bytes of the Data
>> buffer. Unless the EFI_VARIABLE_APPEND_WRITE
>> + attribute is set, a size of
>> zero causes the variable to be deleted. When the
>> + EFI_VARIABLE_APPEND_WRITE
>> attribute is set, then a SetVariable() call with a
>> + DataSize of zero will not cause
>> any change to the variable value.
>> + @param[in] Data The contents for the variable.
>> +
>> + @retval EFI_SUCCESS The firmware has successfully
>> stored the variable and its data as
>> + defined by the Attributes.
>> + @retval EFI_INVALID_PARAMETER An invalid combination of
>> attribute bits, name, and GUID was supplied, or the
>> + DataSize exceeds the maximum
>> allowed.
>> + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
>> + @retval EFI_OUT_OF_RESOURCES Not enough storage is available
>> to hold the variable and its data.
>> + @retval EFI_DEVICE_ERROR The variable could not be
>> retrieved due to a hardware error.
>> + @retval EFI_WRITE_PROTECTED The variable in question is
>> read-only.
>> + @retval EFI_WRITE_PROTECTED The variable in question cannot
>> be deleted.
>> + @retval EFI_SECURITY_VIOLATION The variable could not be
>> written due to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
>> + or
>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
>> +
>> EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS being set. Writing to
>> authenticated
>> + variables is not supported in
>> the PEI environment. Updates to authenticated
>> + variables can be requested
>> during PEI via the EFI_AUTHENTICATED_VARIABLE_HOB, but
>> + these updates won't be written
>> to non-volatile storage until later in DXE. See
>> +
>> MdeModulePkg/Include/Guid/VariableFormat.h for more details on
>> + EFI_AUTHENTICATED_VARIABLE_HOB.
>
> [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
> VariableFormat.h.
>
> [MK] It seems that if a contract for producing and then consuming this
> HOB is going to be defined between the HOB producer and consumer phase,
> it should be described in something like the PI Spec.
>
>> + @retval EFI_NOT_FOUND The variable trying to be
>> updated or deleted was not found.
>> +
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>> + IN CHAR16 *VariableName,
>> + IN EFI_GUID *VendorGuid,
>> + IN UINT32 Attributes,
>> + IN UINTN DataSize,
>> + IN VOID *Data
>> + );
>> +
>> +/**
>> + Returns information about the UEFI variables.
>> +
>> + @param[in] This A pointer to this
>> instance of the EDKII_PEI_VARIABLE_PPI.
>> + @param[in] Attributes Attributes bitmask
>> to specify the type of variables on
>> + which to return
>> information.
>> + @param[out] MaximumVariableStorageSize On output the
>> maximum size of the storage space
>> + available for the
>> EFI variables associated with the
>> + attributes specified.
>> + @param[out] RemainingVariableStorageSize Returns the
>> remaining size of the storage space
>> + available for the
>> EFI variables associated with the
>> + attributes specified.
>> + @param[out] MaximumVariableSize Returns the maximum
>> size of the individual EFI
>> + variables
>> associated with the attributes specified.
>> +
>> + @retval EFI_SUCCESS Valid answer returned.
>> + @retval EFI_INVALID_PARAMETER An invalid
>> combination of attribute bits was supplied
>> + @retval EFI_UNSUPPORTED The attribute is
>> not supported on this platform, and the
>> +
>> MaximumVariableStorageSize,
>> +
>> RemainingVariableStorageSize, MaximumVariableSize
>> + are undefined.
>> +
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>> + IN UINT32 Attributes,
>> + OUT UINT64 *MaximumVariableStorageSize,
>> + OUT UINT64 *RemainingVariableStorageSize,
>> + OUT UINT64 *MaximumVariableSize
>> + );
>> +
>> +///
>> +/// PEI Variable Protocol is intended for use as a means
>
> [MK] Was "PEI Variable PPI" intended?
>
>> +/// to store data in the PEI environment.
>> +///
>> +struct _EDKII_PEI_VARIABLE_PPI {
>> + EDKII_PEI_GET_VARIABLE GetVariable;
>> + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
>> + EDKII_PEI_SET_VARIABLE SetVariable;
>> + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
>> +};
>> +
>> +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
>> +
>> +#endif
>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>> b/MdeModulePkg/MdeModulePkg.dec
>> index 2bcb9f9453..4f4c48b81f 100644
>> --- a/MdeModulePkg/MdeModulePkg.dec
>> +++ b/MdeModulePkg/MdeModulePkg.dec
>> @@ -513,6 +513,9 @@
>> gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35,
>> 0x4a5d, { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
>> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
>> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
>> + ## Include/Ppi/Variable.h
>> + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14,
>> 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
>> +
>> [Protocols]
>> ## Load File protocol provides capability to load and unload EFI
>> image into memory and execute it.
>> # Include/Protocol/LoadPe32Image.h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 1:59 ` [edk2-devel] " Michael Kubacki
2022-06-10 2:07 ` Michael Kubacki
@ 2022-06-10 16:56 ` Yao, Jiewen
2022-06-10 21:48 ` Nate DeSimone
2022-06-10 21:20 ` Nate DeSimone
2 siblings, 1 reply; 13+ messages in thread
From: Yao, Jiewen @ 2022-06-10 16:56 UTC (permalink / raw)
To: devel@edk2.groups.io, michael.kubacki@outlook.com,
Desimone, Nathaniel L
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
Hi
I am curious why we need this interface. Why we need write variable capability in PEI phase?
Where is the implementation of this? I prefer to see an implementation submitted together with header file.
For example, what is the impact to secure boot related feature, how to write auth variable in PEI, how PEI write variable cowork with SMM version in S3 resume phase, how to support variable atomicity, etc.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Friday, June 10, 2022 10:00 AM
> To: devel@edk2.groups.io; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
> Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
> EDKII_PEI_VARIABLE_PPI
>
> Is this change just adding the interface to Tianocore or is there
> additional implementation planned as well?
>
> ---
>
> I thought we were following this convention now:
>
> "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
>
> Some other comments are inline.
>
> Regards,
> Michael
>
> On 6/9/2022 9:17 PM, Nate DeSimone wrote:
> > Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor
> > to enabling variable writes in the PEI environment.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Isaac Oram <isaac.w.oram@intel.com>
> > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > Cc: Gao Cheng <gao.cheng@intel.com>
> > Cc: Di Zhang <di.zhang@intel.com>
> > Cc: Daocheng Bu <daocheng.bu@intel.com>
> > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > ---
> > MdeModulePkg/Include/Ppi/Variable.h | 189
> ++++++++++++++++++++++++++++
> > MdeModulePkg/MdeModulePkg.dec | 3 +
> > 2 files changed, 192 insertions(+)
> > create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
> >
> > diff --git a/MdeModulePkg/Include/Ppi/Variable.h
> b/MdeModulePkg/Include/Ppi/Variable.h
> > new file mode 100644
> > index 0000000000..97dc7ceefa
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Ppi/Variable.h
> > @@ -0,0 +1,189 @@
> > +/** @file
> > + EDKII PEI Variable Protocol provides an implementation of variables
>
> [MK] Was "EDKII PEI Variable PPI" intended?
>
> > + intended for use as a means to store data in the PEI environment.
> > +
> > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef __PEI_VARIABLE_PPI_H_
> > +#define __PEI_VARIABLE_PPI_H_
> > +
> > +#define EDKII_PEI_VARIABLE_PPI_GUID \
> > + { \
> > + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a,
> 0xb0 } \
> > + }
> > +
> > +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
> > +
> > +/**
> > + This service retrieves a variable's value using its name and GUID.
> > +
> > + Read the specified variable from the UEFI variable store. If the Data
> > + buffer is too small to hold the contents of the variable,
> > + the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to the
> > + required buffer size to obtain the data.
> > +
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in] VariableName A pointer to a null-terminated string that
> is the variable's name.
> > + @param[in] VariableGuid A pointer to an EFI_GUID that is the
> variable's GUID. The combination of
> > + VariableGuid and VariableName must be unique.
> > + @param[out] Attributes If non-NULL, on return, points to the
> variable's attributes.
> > + @param[in, out] DataSize On entry, points to the size in bytes of the
> Data buffer.
> > + On return, points to the size of the data returned in Data.
> > + @param[out] Data Points to the buffer which will hold the
> returned variable value.
> > + May be NULL with a zero DataSize in order to determine
> the size of the
> > + buffer needed.
> > +
> > + @retval EFI_SUCCESS The variable was read successfully.
> > + @retval EFI_NOT_FOUND The variable was not found.
> > + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the
> resulting data.
> > + DataSize is updated with the size required for
> > + the specified variable.
> > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
> DataSize or Data is NULL.
> > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> because of a device error.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN CONST CHAR16 *VariableName,
> > + IN CONST EFI_GUID *VariableGuid,
> > + OUT UINT32 *Attributes,
>
> [MK] Based on the description, Attributes should be marked "OPTIONAL".
>
> > + IN OUT UINTN *DataSize,
> > + OUT VOID *Data OPTIONAL
> > + );
> > +
> > +/**
> > + Return the next variable name and GUID.
> > +
> > + This function is called multiple times to retrieve the VariableName
> > + and VariableGuid of all variables currently available in the system.
> > + On each call, the previous results are passed into the interface,
> > + and, on return, the interface returns the data for the next
> > + interface. When the entire variable list has been returned,
> > + EFI_NOT_FOUND is returned.
> > +
>
> [MK] I know other descriptions don't usually have it but it would be
> nice to describe the initial calling values expected.
>
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in, out] VariableNameSize On entry, points to the size of the
> buffer pointed to by VariableName.
> > + On return, the size of the variable name buffer.
> > + @param[in, out] VariableName On entry, a pointer to a null-terminated
> string that is the variable's name.
> > + On return, points to the next variable's null-terminated
> name string.
> > + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that is
> the variable's GUID.
> > + On return, a pointer to the next variable's GUID.
> > +
> > + @retval EFI_SUCCESS The variable was read successfully.
> > + @retval EFI_NOT_FOUND The variable could not be found.
> > + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small for
> the resulting
> > + data. VariableNameSize is updated with the size
> > + required for the specified variable.
> > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
> > + VariableNameSize is NULL.
> > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> because of a device error.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN OUT UINTN *VariableNameSize,
> > + IN OUT CHAR16 *VariableName,
> > + IN OUT EFI_GUID *VariableGuid
> > + );
> > +
> > +/**
> > + Sets the value of a variable.
> > +
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in] VariableName A Null-terminated string that is the name
> of the vendor's variable.
> > + Each VariableName is unique for each VendorGuid.
> VariableName must
> > + contain 1 or more characters. If VariableName is an
> empty string,
> > + then EFI_INVALID_PARAMETER is returned.
> > + @param[in] VendorGuid A unique identifier for the vendor.
> > + @param[in] Attributes Attributes bitmask to set for the variable.
> > + @param[in] DataSize The size in bytes of the Data buffer. Unless
> the EFI_VARIABLE_APPEND_WRITE
> > + attribute is set, a size of zero causes the variable to be
> deleted. When the
> > + EFI_VARIABLE_APPEND_WRITE attribute is set, then a
> SetVariable() call with a
> > + DataSize of zero will not cause any change to the
> variable value.
> > + @param[in] Data The contents for the variable.
> > +
> > + @retval EFI_SUCCESS The firmware has successfully stored the
> variable and its data as
> > + defined by the Attributes.
> > + @retval EFI_INVALID_PARAMETER An invalid combination of attribute
> bits, name, and GUID was supplied, or the
> > + DataSize exceeds the maximum allowed.
> > + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
> > + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to
> hold the variable and its data.
> > + @retval EFI_DEVICE_ERROR The variable could not be retrieved due
> to a hardware error.
> > + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> > + @retval EFI_WRITE_PROTECTED The variable in question cannot be
> deleted.
> > + @retval EFI_SECURITY_VIOLATION The variable could not be written due
> to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
> > + or
> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
> > + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
> being set. Writing to authenticated
> > + variables is not supported in the PEI environment.
> Updates to authenticated
> > + variables can be requested during PEI via the
> EFI_AUTHENTICATED_VARIABLE_HOB, but
> > + these updates won't be written to non-volatile storage
> until later in DXE. See
> > + MdeModulePkg/Include/Guid/VariableFormat.h for
> more details on
> > + EFI_AUTHENTICATED_VARIABLE_HOB.
>
> [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
> VariableFormat.h.
>
> [MK] It seems that if a contract for producing and then consuming this
> HOB is going to be defined between the HOB producer and consumer phase,
> it should be described in something like the PI Spec.
>
> > + @retval EFI_NOT_FOUND The variable trying to be updated or
> deleted was not found.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN CHAR16 *VariableName,
> > + IN EFI_GUID *VendorGuid,
> > + IN UINT32 Attributes,
> > + IN UINTN DataSize,
> > + IN VOID *Data
> > + );
> > +
> > +/**
> > + Returns information about the UEFI variables.
> > +
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in] Attributes Attributes bitmask to specify the type
> of variables on
> > + which to return information.
> > + @param[out] MaximumVariableStorageSize On output the maximum
> size of the storage space
> > + available for the EFI variables associated with the
> > + attributes specified.
> > + @param[out] RemainingVariableStorageSize Returns the remaining size
> of the storage space
> > + available for the EFI variables associated with the
> > + attributes specified.
> > + @param[out] MaximumVariableSize Returns the maximum size of
> the individual EFI
> > + variables associated with the attributes specified.
> > +
> > + @retval EFI_SUCCESS Valid answer returned.
> > + @retval EFI_INVALID_PARAMETER An invalid combination of
> attribute bits was supplied
> > + @retval EFI_UNSUPPORTED The attribute is not supported on
> this platform, and the
> > + MaximumVariableStorageSize,
> > + RemainingVariableStorageSize,
> MaximumVariableSize
> > + are undefined.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN UINT32 Attributes,
> > + OUT UINT64 *MaximumVariableStorageSize,
> > + OUT UINT64 *RemainingVariableStorageSize,
> > + OUT UINT64 *MaximumVariableSize
> > + );
> > +
> > +///
> > +/// PEI Variable Protocol is intended for use as a means
>
> [MK] Was "PEI Variable PPI" intended?
>
> > +/// to store data in the PEI environment.
> > +///
> > +struct _EDKII_PEI_VARIABLE_PPI {
> > + EDKII_PEI_GET_VARIABLE GetVariable;
> > + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
> > + EDKII_PEI_SET_VARIABLE SetVariable;
> > + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
> > +};
> > +
> > +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
> > +
> > +#endif
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> > index 2bcb9f9453..4f4c48b81f 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -513,6 +513,9 @@
> > gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d,
> { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> > gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
> >
> > + ## Include/Ppi/Variable.h
> > + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7,
> 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
> > +
> > [Protocols]
> > ## Load File protocol provides capability to load and unload EFI image into
> memory and execute it.
> > # Include/Protocol/LoadPe32Image.h
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 1:59 ` [edk2-devel] " Michael Kubacki
2022-06-10 2:07 ` Michael Kubacki
2022-06-10 16:56 ` Yao, Jiewen
@ 2022-06-10 21:20 ` Nate DeSimone
2 siblings, 0 replies; 13+ messages in thread
From: Nate DeSimone @ 2022-06-10 21:20 UTC (permalink / raw)
To: devel@edk2.groups.io, michael.kubacki@outlook.com
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
Hi Michael,
Thanks for the great feedback! I believe I have addressed all of it in the V2 patch. Please take a look when you get a chance.
Per your comment on the EFI_AUTHENTICATED_VARIABLE_HOB... this is one of those old data structures migrated from the edk1 that has the EFI_ prefix even though it was never added to the PI spec proper. The name wasn't changed for backwards compatibility reasons (there is a huge amount of code dependent on those old names.) There are a fair number of instances of this in the variable services, EFI_SMM_VARIABLE_PROTOCOL perhaps being the most visible.
Thanks,
Nate
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Thursday, June 9, 2022 7:00 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>; Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
Is this change just adding the interface to Tianocore or is there additional implementation planned as well?
---
I thought we were following this convention now:
"#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
Some other comments are inline.
Regards,
Michael
On 6/9/2022 9:17 PM, Nate DeSimone wrote:
> Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to enabling
> variable writes in the PEI environment.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Gao Cheng <gao.cheng@intel.com>
> Cc: Di Zhang <di.zhang@intel.com>
> Cc: Daocheng Bu <daocheng.bu@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
> MdeModulePkg/Include/Ppi/Variable.h | 189 ++++++++++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 2 files changed, 192 insertions(+)
> create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
>
> diff --git a/MdeModulePkg/Include/Ppi/Variable.h
> b/MdeModulePkg/Include/Ppi/Variable.h
> new file mode 100644
> index 0000000000..97dc7ceefa
> --- /dev/null
> +++ b/MdeModulePkg/Include/Ppi/Variable.h
> @@ -0,0 +1,189 @@
> +/** @file
> + EDKII PEI Variable Protocol provides an implementation of variables
[MK] Was "EDKII PEI Variable PPI" intended?
> + intended for use as a means to store data in the PEI environment.
> +
> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __PEI_VARIABLE_PPI_H_
> +#define __PEI_VARIABLE_PPI_H_
> +
> +#define EDKII_PEI_VARIABLE_PPI_GUID \
> + { \
> + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66,
> +0x4a, 0xb0 } \
> + }
> +
> +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
> +
> +/**
> + This service retrieves a variable's value using its name and GUID.
> +
> + Read the specified variable from the UEFI variable store. If the
> + Data buffer is too small to hold the contents of the variable, the
> + error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to the
> + required buffer size to obtain the data.
> +
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in] VariableName A pointer to a null-terminated string that is the variable's name.
> + @param[in] VariableGuid A pointer to an EFI_GUID that is the variable's GUID. The combination of
> + VariableGuid and VariableName must be unique.
> + @param[out] Attributes If non-NULL, on return, points to the variable's attributes.
> + @param[in, out] DataSize On entry, points to the size in bytes of the Data buffer.
> + On return, points to the size of the data returned in Data.
> + @param[out] Data Points to the buffer which will hold the returned variable value.
> + May be NULL with a zero DataSize in order to determine the size of the
> + buffer needed.
> +
> + @retval EFI_SUCCESS The variable was read successfully.
> + @retval EFI_NOT_FOUND The variable was not found.
> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the resulting data.
> + DataSize is updated with the size required for
> + the specified variable.
> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid, DataSize or Data is NULL.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved because of a device error.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN CONST CHAR16 *VariableName,
> + IN CONST EFI_GUID *VariableGuid,
> + OUT UINT32 *Attributes,
[MK] Based on the description, Attributes should be marked "OPTIONAL".
> + IN OUT UINTN *DataSize,
> + OUT VOID *Data OPTIONAL
> + );
> +
> +/**
> + Return the next variable name and GUID.
> +
> + This function is called multiple times to retrieve the VariableName
> + and VariableGuid of all variables currently available in the system.
> + On each call, the previous results are passed into the interface,
> + and, on return, the interface returns the data for the next
> + interface. When the entire variable list has been returned,
> + EFI_NOT_FOUND is returned.
> +
[MK] I know other descriptions don't usually have it but it would be nice to describe the initial calling values expected.
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in, out] VariableNameSize On entry, points to the size of the buffer pointed to by VariableName.
> + On return, the size of the variable name buffer.
> + @param[in, out] VariableName On entry, a pointer to a null-terminated string that is the variable's name.
> + On return, points to the next variable's null-terminated name string.
> + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that is the variable's GUID.
> + On return, a pointer to the next variable's GUID.
> +
> + @retval EFI_SUCCESS The variable was read successfully.
> + @retval EFI_NOT_FOUND The variable could not be found.
> + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small for the resulting
> + data. VariableNameSize is updated with the size
> + required for the specified variable.
> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
> + VariableNameSize is NULL.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved because of a device error.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN OUT UINTN *VariableNameSize,
> + IN OUT CHAR16 *VariableName,
> + IN OUT EFI_GUID *VariableGuid
> + );
> +
> +/**
> + Sets the value of a variable.
> +
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
> + Each VariableName is unique for each VendorGuid. VariableName must
> + contain 1 or more characters. If VariableName is an empty string,
> + then EFI_INVALID_PARAMETER is returned.
> + @param[in] VendorGuid A unique identifier for the vendor.
> + @param[in] Attributes Attributes bitmask to set for the variable.
> + @param[in] DataSize The size in bytes of the Data buffer. Unless the EFI_VARIABLE_APPEND_WRITE
> + attribute is set, a size of zero causes the variable to be deleted. When the
> + EFI_VARIABLE_APPEND_WRITE attribute is set, then a SetVariable() call with a
> + DataSize of zero will not cause any change to the variable value.
> + @param[in] Data The contents for the variable.
> +
> + @retval EFI_SUCCESS The firmware has successfully stored the variable and its data as
> + defined by the Attributes.
> + @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits, name, and GUID was supplied, or the
> + DataSize exceeds the maximum allowed.
> + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
> + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to hold the variable and its data.
> + @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
> + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> + @retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
> + @retval EFI_SECURITY_VIOLATION The variable could not be written due to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
> + or EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
> + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS being set. Writing to authenticated
> + variables is not supported in the PEI environment. Updates to authenticated
> + variables can be requested during PEI via the EFI_AUTHENTICATED_VARIABLE_HOB, but
> + these updates won't be written to non-volatile storage until later in DXE. See
> + MdeModulePkg/Include/Guid/VariableFormat.h for more details on
> + EFI_AUTHENTICATED_VARIABLE_HOB.
[MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in VariableFormat.h.
[MK] It seems that if a contract for producing and then consuming this HOB is going to be defined between the HOB producer and consumer phase, it should be described in something like the PI Spec.
> + @retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN CHAR16 *VariableName,
> + IN EFI_GUID *VendorGuid,
> + IN UINT32 Attributes,
> + IN UINTN DataSize,
> + IN VOID *Data
> + );
> +
> +/**
> + Returns information about the UEFI variables.
> +
> + @param[in] This A pointer to this instance of the EDKII_PEI_VARIABLE_PPI.
> + @param[in] Attributes Attributes bitmask to specify the type of variables on
> + which to return information.
> + @param[out] MaximumVariableStorageSize On output the maximum size of the storage space
> + available for the EFI variables associated with the
> + attributes specified.
> + @param[out] RemainingVariableStorageSize Returns the remaining size of the storage space
> + available for the EFI variables associated with the
> + attributes specified.
> + @param[out] MaximumVariableSize Returns the maximum size of the individual EFI
> + variables associated with the attributes specified.
> +
> + @retval EFI_SUCCESS Valid answer returned.
> + @retval EFI_INVALID_PARAMETER An invalid combination of attribute bits was supplied
> + @retval EFI_UNSUPPORTED The attribute is not supported on this platform, and the
> + MaximumVariableStorageSize,
> + RemainingVariableStorageSize, MaximumVariableSize
> + are undefined.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> + IN UINT32 Attributes,
> + OUT UINT64 *MaximumVariableStorageSize,
> + OUT UINT64 *RemainingVariableStorageSize,
> + OUT UINT64 *MaximumVariableSize
> + );
> +
> +///
> +/// PEI Variable Protocol is intended for use as a means
[MK] Was "PEI Variable PPI" intended?
> +/// to store data in the PEI environment.
> +///
> +struct _EDKII_PEI_VARIABLE_PPI {
> + EDKII_PEI_GET_VARIABLE GetVariable;
> + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
> + EDKII_PEI_SET_VARIABLE SetVariable;
> + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
> +};
> +
> +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
> +
> +#endif
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 2bcb9f9453..4f4c48b81f 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -513,6 +513,9 @@
> gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d, { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7, 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
>
> + ## Include/Ppi/Variable.h
> + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
> +
> [Protocols]
> ## Load File protocol provides capability to load and unload EFI image into memory and execute it.
> # Include/Protocol/LoadPe32Image.h
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 16:56 ` Yao, Jiewen
@ 2022-06-10 21:48 ` Nate DeSimone
2022-06-11 1:09 ` Yao, Jiewen
0 siblings, 1 reply; 13+ messages in thread
From: Nate DeSimone @ 2022-06-10 21:48 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, michael.kubacki@outlook.com
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
Hi Jiewen,
Thanks for the feedback, per your questions:
1. The primary use case for this is to enable UEFI variable writes before permanent memory is available.
2. The implementation is a work in progress. We will provide it shortly. As this will be a rather large patch set, I would like to get this piece in place beforehand so that the reviewers can focus on the implementation separate from the API definition.
3. No impact to secure boot. We are not going to support writing to authenticated variables in PEI. As mentioned in the comments, if a PEIM wishes to update any of the authenticated variables it must use the existing HOB mechanism to have a later DXE phase perform the update.
4. With regard to atomicity, we have a complete implementation of the fault tolerant write services operational in Pre-Memory PEI.
5. Good point on the S3 resume, we will need to add an SMI to have the variable services re-initialize the mNvVariableCache.
Hope that helps,
Nate
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, June 10, 2022 9:56 AM
To: devel@edk2.groups.io; michael.kubacki@outlook.com; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>; Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
Hi
I am curious why we need this interface. Why we need write variable capability in PEI phase?
Where is the implementation of this? I prefer to see an implementation submitted together with header file.
For example, what is the impact to secure boot related feature, how to write auth variable in PEI, how PEI write variable cowork with SMM version in S3 resume phase, how to support variable atomicity, etc.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Friday, June 10, 2022 10:00 AM
> To: devel@edk2.groups.io; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
> <daocheng.bu@intel.com>; Kubacki, Michael
> <michael.kubacki@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
> of EDKII_PEI_VARIABLE_PPI
>
> Is this change just adding the interface to Tianocore or is there
> additional implementation planned as well?
>
> ---
>
> I thought we were following this convention now:
>
> "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
>
> Some other comments are inline.
>
> Regards,
> Michael
>
> On 6/9/2022 9:17 PM, Nate DeSimone wrote:
> > Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to enabling
> > variable writes in the PEI environment.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Isaac Oram <isaac.w.oram@intel.com>
> > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > Cc: Gao Cheng <gao.cheng@intel.com>
> > Cc: Di Zhang <di.zhang@intel.com>
> > Cc: Daocheng Bu <daocheng.bu@intel.com>
> > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > ---
> > MdeModulePkg/Include/Ppi/Variable.h | 189
> ++++++++++++++++++++++++++++
> > MdeModulePkg/MdeModulePkg.dec | 3 +
> > 2 files changed, 192 insertions(+)
> > create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
> >
> > diff --git a/MdeModulePkg/Include/Ppi/Variable.h
> b/MdeModulePkg/Include/Ppi/Variable.h
> > new file mode 100644
> > index 0000000000..97dc7ceefa
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Ppi/Variable.h
> > @@ -0,0 +1,189 @@
> > +/** @file
> > + EDKII PEI Variable Protocol provides an implementation of
> > +variables
>
> [MK] Was "EDKII PEI Variable PPI" intended?
>
> > + intended for use as a means to store data in the PEI environment.
> > +
> > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef __PEI_VARIABLE_PPI_H_
> > +#define __PEI_VARIABLE_PPI_H_
> > +
> > +#define EDKII_PEI_VARIABLE_PPI_GUID \
> > + { \
> > + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b,
> > +0x66, 0x4a,
> 0xb0 } \
> > + }
> > +
> > +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
> > +
> > +/**
> > + This service retrieves a variable's value using its name and GUID.
> > +
> > + Read the specified variable from the UEFI variable store. If the
> > + Data buffer is too small to hold the contents of the variable,
> > + the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to
> > + the required buffer size to obtain the data.
> > +
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in] VariableName A pointer to a null-terminated string that
> is the variable's name.
> > + @param[in] VariableGuid A pointer to an EFI_GUID that is the
> variable's GUID. The combination of
> > + VariableGuid and VariableName must be unique.
> > + @param[out] Attributes If non-NULL, on return, points to the
> variable's attributes.
> > + @param[in, out] DataSize On entry, points to the size in bytes of the
> Data buffer.
> > + On return, points to the size of the data returned in Data.
> > + @param[out] Data Points to the buffer which will hold the
> returned variable value.
> > + May be NULL with a zero
> > + DataSize in order to determine
> the size of the
> > + buffer needed.
> > +
> > + @retval EFI_SUCCESS The variable was read successfully.
> > + @retval EFI_NOT_FOUND The variable was not found.
> > + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the
> resulting data.
> > + DataSize is updated with the size required for
> > + the specified variable.
> > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
> DataSize or Data is NULL.
> > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> because of a device error.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN CONST CHAR16 *VariableName,
> > + IN CONST EFI_GUID *VariableGuid,
> > + OUT UINT32 *Attributes,
>
> [MK] Based on the description, Attributes should be marked "OPTIONAL".
>
> > + IN OUT UINTN *DataSize,
> > + OUT VOID *Data OPTIONAL
> > + );
> > +
> > +/**
> > + Return the next variable name and GUID.
> > +
> > + This function is called multiple times to retrieve the
> > + VariableName and VariableGuid of all variables currently available in the system.
> > + On each call, the previous results are passed into the interface,
> > + and, on return, the interface returns the data for the next
> > + interface. When the entire variable list has been returned,
> > + EFI_NOT_FOUND is returned.
> > +
>
> [MK] I know other descriptions don't usually have it but it would be
> nice to describe the initial calling values expected.
>
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in, out] VariableNameSize On entry, points to the size of the
> buffer pointed to by VariableName.
> > + On return, the size of the variable name buffer.
> > + @param[in, out] VariableName On entry, a pointer to a null-terminated
> string that is the variable's name.
> > + On return, points to the next
> > + variable's null-terminated
> name string.
> > + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that is
> the variable's GUID.
> > + On return, a pointer to the next variable's GUID.
> > +
> > + @retval EFI_SUCCESS The variable was read successfully.
> > + @retval EFI_NOT_FOUND The variable could not be found.
> > + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small for
> the resulting
> > + data. VariableNameSize is updated with the size
> > + required for the specified variable.
> > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
> > + VariableNameSize is NULL.
> > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> because of a device error.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN OUT UINTN *VariableNameSize,
> > + IN OUT CHAR16 *VariableName,
> > + IN OUT EFI_GUID *VariableGuid
> > + );
> > +
> > +/**
> > + Sets the value of a variable.
> > +
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in] VariableName A Null-terminated string that is the name
> of the vendor's variable.
> > + Each VariableName is unique for each VendorGuid.
> VariableName must
> > + contain 1 or more characters.
> > + If VariableName is an
> empty string,
> > + then EFI_INVALID_PARAMETER is returned.
> > + @param[in] VendorGuid A unique identifier for the vendor.
> > + @param[in] Attributes Attributes bitmask to set for the variable.
> > + @param[in] DataSize The size in bytes of the Data buffer. Unless
> the EFI_VARIABLE_APPEND_WRITE
> > + attribute is set, a size of
> > + zero causes the variable to be
> deleted. When the
> > + EFI_VARIABLE_APPEND_WRITE
> > + attribute is set, then a
> SetVariable() call with a
> > + DataSize of zero will not
> > + cause any change to the
> variable value.
> > + @param[in] Data The contents for the variable.
> > +
> > + @retval EFI_SUCCESS The firmware has successfully stored the
> variable and its data as
> > + defined by the Attributes.
> > + @retval EFI_INVALID_PARAMETER An invalid combination of attribute
> bits, name, and GUID was supplied, or the
> > + DataSize exceeds the maximum allowed.
> > + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
> > + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to
> hold the variable and its data.
> > + @retval EFI_DEVICE_ERROR The variable could not be retrieved due
> to a hardware error.
> > + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> > + @retval EFI_WRITE_PROTECTED The variable in question cannot be
> deleted.
> > + @retval EFI_SECURITY_VIOLATION The variable could not be written due
> to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
> > + or
> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
> > +
> > + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
> being set. Writing to authenticated
> > + variables is not supported in the PEI environment.
> Updates to authenticated
> > + variables can be requested
> > + during PEI via the
> EFI_AUTHENTICATED_VARIABLE_HOB, but
> > + these updates won't be
> > + written to non-volatile storage
> until later in DXE. See
> > +
> > + MdeModulePkg/Include/Guid/VariableFormat.h for
> more details on
> > + EFI_AUTHENTICATED_VARIABLE_HOB.
>
> [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
> VariableFormat.h.
>
> [MK] It seems that if a contract for producing and then consuming this
> HOB is going to be defined between the HOB producer and consumer
> phase, it should be described in something like the PI Spec.
>
> > + @retval EFI_NOT_FOUND The variable trying to be updated or
> deleted was not found.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN CHAR16 *VariableName,
> > + IN EFI_GUID *VendorGuid,
> > + IN UINT32 Attributes,
> > + IN UINTN DataSize,
> > + IN VOID *Data
> > + );
> > +
> > +/**
> > + Returns information about the UEFI variables.
> > +
> > + @param[in] This A pointer to this instance of the
> EDKII_PEI_VARIABLE_PPI.
> > + @param[in] Attributes Attributes bitmask to specify the type
> of variables on
> > + which to return information.
> > + @param[out] MaximumVariableStorageSize On output the maximum
> size of the storage space
> > + available for the EFI variables associated with the
> > + attributes specified.
> > + @param[out] RemainingVariableStorageSize Returns the remaining size
> of the storage space
> > + available for the EFI variables associated with the
> > + attributes specified.
> > + @param[out] MaximumVariableSize Returns the maximum size of
> the individual EFI
> > + variables associated with the attributes specified.
> > +
> > + @retval EFI_SUCCESS Valid answer returned.
> > + @retval EFI_INVALID_PARAMETER An invalid combination of
> attribute bits was supplied
> > + @retval EFI_UNSUPPORTED The attribute is not supported on
> this platform, and the
> > + MaximumVariableStorageSize,
> > +
> > + RemainingVariableStorageSize,
> MaximumVariableSize
> > + are undefined.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
> > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > + IN UINT32 Attributes,
> > + OUT UINT64 *MaximumVariableStorageSize,
> > + OUT UINT64 *RemainingVariableStorageSize,
> > + OUT UINT64 *MaximumVariableSize
> > + );
> > +
> > +///
> > +/// PEI Variable Protocol is intended for use as a means
>
> [MK] Was "PEI Variable PPI" intended?
>
> > +/// to store data in the PEI environment.
> > +///
> > +struct _EDKII_PEI_VARIABLE_PPI {
> > + EDKII_PEI_GET_VARIABLE GetVariable;
> > + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
> > + EDKII_PEI_SET_VARIABLE SetVariable;
> > + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
> > +};
> > +
> > +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
> > +
> > +#endif
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> > index 2bcb9f9453..4f4c48b81f 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -513,6 +513,9 @@
> > gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d,
> { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> > gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
> >
> > + ## Include/Ppi/Variable.h
> > + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7,
> 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
> > +
> > [Protocols]
> > ## Load File protocol provides capability to load and unload EFI
> > image into
> memory and execute it.
> > # Include/Protocol/LoadPe32Image.h
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-10 21:48 ` Nate DeSimone
@ 2022-06-11 1:09 ` Yao, Jiewen
2022-06-11 1:14 ` Michael Kubacki
2022-06-13 21:31 ` Nate DeSimone
0 siblings, 2 replies; 13+ messages in thread
From: Yao, Jiewen @ 2022-06-11 1:09 UTC (permalink / raw)
To: Desimone, Nathaniel L, devel@edk2.groups.io,
michael.kubacki@outlook.com
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
Thanks for the response.
1) Why we need "enable UEFI variable write before permanent memory is available"?
2) If the implementation is not ready, I do have concern to add it so early in EDKII.
If I don’t have a big picture, I am not sure how to review the completeness.
Can we put it to EDKII-staging (https://github.com/tianocore/edk2-staging) for a moment?
I don’t see the need to add the interface now for work-in-progress feature, since there is no consumer and no producer.
Another reason is that I happen to know other feature (in EDKII stage) is impacting variable driver.
https://github.com/tianocore/edk2-staging/tree/ProtectedVariable/libs
Please do consider that as well - how to write a protected variable in PEI phase.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Saturday, June 11, 2022 5:49 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
> michael.kubacki@outlook.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
> Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
> EDKII_PEI_VARIABLE_PPI
>
> Hi Jiewen,
>
> Thanks for the feedback, per your questions:
>
> 1. The primary use case for this is to enable UEFI variable writes before
> permanent memory is available.
> 2. The implementation is a work in progress. We will provide it shortly. As this
> will be a rather large patch set, I would like to get this piece in place beforehand
> so that the reviewers can focus on the implementation separate from the API
> definition.
> 3. No impact to secure boot. We are not going to support writing to
> authenticated variables in PEI. As mentioned in the comments, if a PEIM wishes
> to update any of the authenticated variables it must use the existing HOB
> mechanism to have a later DXE phase perform the update.
> 4. With regard to atomicity, we have a complete implementation of the fault
> tolerant write services operational in Pre-Memory PEI.
> 5. Good point on the S3 resume, we will need to add an SMI to have the variable
> services re-initialize the mNvVariableCache.
>
> Hope that helps,
> Nate
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, June 10, 2022 9:56 AM
> To: devel@edk2.groups.io; michael.kubacki@outlook.com; Desimone,
> Nathaniel L <nathaniel.l.desimone@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
> Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
> EDKII_PEI_VARIABLE_PPI
>
> Hi
>
> I am curious why we need this interface. Why we need write variable capability
> in PEI phase?
>
> Where is the implementation of this? I prefer to see an implementation
> submitted together with header file.
> For example, what is the impact to secure boot related feature, how to write
> auth variable in PEI, how PEI write variable cowork with SMM version in S3
> resume phase, how to support variable atomicity, etc.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> > Kubacki
> > Sent: Friday, June 10, 2022 10:00 AM
> > To: devel@edk2.groups.io; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> > Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
> > <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
> > <daocheng.bu@intel.com>; Kubacki, Michael
> > <michael.kubacki@microsoft.com>
> > Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
> > of EDKII_PEI_VARIABLE_PPI
> >
> > Is this change just adding the interface to Tianocore or is there
> > additional implementation planned as well?
> >
> > ---
> >
> > I thought we were following this convention now:
> >
> > "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
> >
> > Some other comments are inline.
> >
> > Regards,
> > Michael
> >
> > On 6/9/2022 9:17 PM, Nate DeSimone wrote:
> > > Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to enabling
> > > variable writes in the PEI environment.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Isaac Oram <isaac.w.oram@intel.com>
> > > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > > Cc: Gao Cheng <gao.cheng@intel.com>
> > > Cc: Di Zhang <di.zhang@intel.com>
> > > Cc: Daocheng Bu <daocheng.bu@intel.com>
> > > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > > ---
> > > MdeModulePkg/Include/Ppi/Variable.h | 189
> > ++++++++++++++++++++++++++++
> > > MdeModulePkg/MdeModulePkg.dec | 3 +
> > > 2 files changed, 192 insertions(+)
> > > create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
> > >
> > > diff --git a/MdeModulePkg/Include/Ppi/Variable.h
> > b/MdeModulePkg/Include/Ppi/Variable.h
> > > new file mode 100644
> > > index 0000000000..97dc7ceefa
> > > --- /dev/null
> > > +++ b/MdeModulePkg/Include/Ppi/Variable.h
> > > @@ -0,0 +1,189 @@
> > > +/** @file
> > > + EDKII PEI Variable Protocol provides an implementation of
> > > +variables
> >
> > [MK] Was "EDKII PEI Variable PPI" intended?
> >
> > > + intended for use as a means to store data in the PEI environment.
> > > +
> > > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#ifndef __PEI_VARIABLE_PPI_H_
> > > +#define __PEI_VARIABLE_PPI_H_
> > > +
> > > +#define EDKII_PEI_VARIABLE_PPI_GUID \
> > > + { \
> > > + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b,
> > > +0x66, 0x4a,
> > 0xb0 } \
> > > + }
> > > +
> > > +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
> > > +
> > > +/**
> > > + This service retrieves a variable's value using its name and GUID.
> > > +
> > > + Read the specified variable from the UEFI variable store. If the
> > > + Data buffer is too small to hold the contents of the variable,
> > > + the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to
> > > + the required buffer size to obtain the data.
> > > +
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in] VariableName A pointer to a null-terminated string that
> > is the variable's name.
> > > + @param[in] VariableGuid A pointer to an EFI_GUID that is the
> > variable's GUID. The combination of
> > > + VariableGuid and VariableName must be unique.
> > > + @param[out] Attributes If non-NULL, on return, points to the
> > variable's attributes.
> > > + @param[in, out] DataSize On entry, points to the size in bytes of the
> > Data buffer.
> > > + On return, points to the size of the data returned in
> Data.
> > > + @param[out] Data Points to the buffer which will hold the
> > returned variable value.
> > > + May be NULL with a zero
> > > + DataSize in order to determine
> > the size of the
> > > + buffer needed.
> > > +
> > > + @retval EFI_SUCCESS The variable was read successfully.
> > > + @retval EFI_NOT_FOUND The variable was not found.
> > > + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the
> > resulting data.
> > > + DataSize is updated with the size required for
> > > + the specified variable.
> > > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
> > DataSize or Data is NULL.
> > > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> > because of a device error.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN CONST CHAR16 *VariableName,
> > > + IN CONST EFI_GUID *VariableGuid,
> > > + OUT UINT32 *Attributes,
> >
> > [MK] Based on the description, Attributes should be marked "OPTIONAL".
> >
> > > + IN OUT UINTN *DataSize,
> > > + OUT VOID *Data OPTIONAL
> > > + );
> > > +
> > > +/**
> > > + Return the next variable name and GUID.
> > > +
> > > + This function is called multiple times to retrieve the
> > > + VariableName and VariableGuid of all variables currently available in the
> system.
> > > + On each call, the previous results are passed into the interface,
> > > + and, on return, the interface returns the data for the next
> > > + interface. When the entire variable list has been returned,
> > > + EFI_NOT_FOUND is returned.
> > > +
> >
> > [MK] I know other descriptions don't usually have it but it would be
> > nice to describe the initial calling values expected.
> >
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in, out] VariableNameSize On entry, points to the size of the
> > buffer pointed to by VariableName.
> > > + On return, the size of the variable name buffer.
> > > + @param[in, out] VariableName On entry, a pointer to a null-
> terminated
> > string that is the variable's name.
> > > + On return, points to the next
> > > + variable's null-terminated
> > name string.
> > > + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that
> is
> > the variable's GUID.
> > > + On return, a pointer to the next variable's GUID.
> > > +
> > > + @retval EFI_SUCCESS The variable was read successfully.
> > > + @retval EFI_NOT_FOUND The variable could not be found.
> > > + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small
> for
> > the resulting
> > > + data. VariableNameSize is updated with the size
> > > + required for the specified variable.
> > > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
> > > + VariableNameSize is NULL.
> > > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> > because of a device error.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN OUT UINTN *VariableNameSize,
> > > + IN OUT CHAR16 *VariableName,
> > > + IN OUT EFI_GUID *VariableGuid
> > > + );
> > > +
> > > +/**
> > > + Sets the value of a variable.
> > > +
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in] VariableName A Null-terminated string that is the name
> > of the vendor's variable.
> > > + Each VariableName is unique for each VendorGuid.
> > VariableName must
> > > + contain 1 or more characters.
> > > + If VariableName is an
> > empty string,
> > > + then EFI_INVALID_PARAMETER is returned.
> > > + @param[in] VendorGuid A unique identifier for the vendor.
> > > + @param[in] Attributes Attributes bitmask to set for the variable.
> > > + @param[in] DataSize The size in bytes of the Data buffer. Unless
> > the EFI_VARIABLE_APPEND_WRITE
> > > + attribute is set, a size of
> > > + zero causes the variable to be
> > deleted. When the
> > > + EFI_VARIABLE_APPEND_WRITE
> > > + attribute is set, then a
> > SetVariable() call with a
> > > + DataSize of zero will not
> > > + cause any change to the
> > variable value.
> > > + @param[in] Data The contents for the variable.
> > > +
> > > + @retval EFI_SUCCESS The firmware has successfully stored the
> > variable and its data as
> > > + defined by the Attributes.
> > > + @retval EFI_INVALID_PARAMETER An invalid combination of attribute
> > bits, name, and GUID was supplied, or the
> > > + DataSize exceeds the maximum allowed.
> > > + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
> > > + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to
> > hold the variable and its data.
> > > + @retval EFI_DEVICE_ERROR The variable could not be retrieved due
> > to a hardware error.
> > > + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> > > + @retval EFI_WRITE_PROTECTED The variable in question cannot be
> > deleted.
> > > + @retval EFI_SECURITY_VIOLATION The variable could not be written
> due
> > to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
> > > + or
> > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
> > > +
> > > + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
> > being set. Writing to authenticated
> > > + variables is not supported in the PEI environment.
> > Updates to authenticated
> > > + variables can be requested
> > > + during PEI via the
> > EFI_AUTHENTICATED_VARIABLE_HOB, but
> > > + these updates won't be
> > > + written to non-volatile storage
> > until later in DXE. See
> > > +
> > > + MdeModulePkg/Include/Guid/VariableFormat.h for
> > more details on
> > > + EFI_AUTHENTICATED_VARIABLE_HOB.
> >
> > [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
> > VariableFormat.h.
> >
> > [MK] It seems that if a contract for producing and then consuming this
> > HOB is going to be defined between the HOB producer and consumer
> > phase, it should be described in something like the PI Spec.
> >
> > > + @retval EFI_NOT_FOUND The variable trying to be updated or
> > deleted was not found.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN CHAR16 *VariableName,
> > > + IN EFI_GUID *VendorGuid,
> > > + IN UINT32 Attributes,
> > > + IN UINTN DataSize,
> > > + IN VOID *Data
> > > + );
> > > +
> > > +/**
> > > + Returns information about the UEFI variables.
> > > +
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in] Attributes Attributes bitmask to specify the type
> > of variables on
> > > + which to return information.
> > > + @param[out] MaximumVariableStorageSize On output the maximum
> > size of the storage space
> > > + available for the EFI variables associated with
> the
> > > + attributes specified.
> > > + @param[out] RemainingVariableStorageSize Returns the remaining
> size
> > of the storage space
> > > + available for the EFI variables associated with
> the
> > > + attributes specified.
> > > + @param[out] MaximumVariableSize Returns the maximum size of
> > the individual EFI
> > > + variables associated with the attributes
> specified.
> > > +
> > > + @retval EFI_SUCCESS Valid answer returned.
> > > + @retval EFI_INVALID_PARAMETER An invalid combination of
> > attribute bits was supplied
> > > + @retval EFI_UNSUPPORTED The attribute is not supported
> on
> > this platform, and the
> > > + MaximumVariableStorageSize,
> > > +
> > > + RemainingVariableStorageSize,
> > MaximumVariableSize
> > > + are undefined.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN UINT32 Attributes,
> > > + OUT UINT64 *MaximumVariableStorageSize,
> > > + OUT UINT64 *RemainingVariableStorageSize,
> > > + OUT UINT64 *MaximumVariableSize
> > > + );
> > > +
> > > +///
> > > +/// PEI Variable Protocol is intended for use as a means
> >
> > [MK] Was "PEI Variable PPI" intended?
> >
> > > +/// to store data in the PEI environment.
> > > +///
> > > +struct _EDKII_PEI_VARIABLE_PPI {
> > > + EDKII_PEI_GET_VARIABLE GetVariable;
> > > + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
> > > + EDKII_PEI_SET_VARIABLE SetVariable;
> > > + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
> > > +};
> > > +
> > > +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
> > > +
> > > +#endif
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec
> > > index 2bcb9f9453..4f4c48b81f 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -513,6 +513,9 @@
> > > gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d,
> > { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> > > gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
> > 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
> > >
> > > + ## Include/Ppi/Variable.h
> > > + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7,
> > 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
> > > +
> > > [Protocols]
> > > ## Load File protocol provides capability to load and unload EFI
> > > image into
> > memory and execute it.
> > > # Include/Protocol/LoadPe32Image.h
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-11 1:09 ` Yao, Jiewen
@ 2022-06-11 1:14 ` Michael Kubacki
2022-06-13 21:31 ` Nate DeSimone
1 sibling, 0 replies; 13+ messages in thread
From: Michael Kubacki @ 2022-06-11 1:14 UTC (permalink / raw)
To: devel, jiewen.yao, Desimone, Nathaniel L
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
mikuback@linux.microsoft.com
The file looks okay to me as an interface proposal but I agree with
Jiewen about deferring the edk2 submission until an implementation can
be reviewed with it as well.
Regards,
Michael
On 6/10/2022 9:09 PM, Yao, Jiewen wrote:
> Thanks for the response.
>
> 1) Why we need "enable UEFI variable write before permanent memory is available"?
>
> 2) If the implementation is not ready, I do have concern to add it so early in EDKII.
> If I don’t have a big picture, I am not sure how to review the completeness.
>
> Can we put it to EDKII-staging (https://github.com/tianocore/edk2-staging) for a moment?
> I don’t see the need to add the interface now for work-in-progress feature, since there is no consumer and no producer.
>
> Another reason is that I happen to know other feature (in EDKII stage) is impacting variable driver.
> https://github.com/tianocore/edk2-staging/tree/ProtectedVariable/libs
>
> Please do consider that as well - how to write a protected variable in PEI phase.
>
> Thank you
> Yao Jiewen
>
>> -----Original Message-----
>> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
>> Sent: Saturday, June 11, 2022 5:49 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
>> michael.kubacki@outlook.com
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
>> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
>> Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
>> Kubacki, Michael <michael.kubacki@microsoft.com>
>> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
>> EDKII_PEI_VARIABLE_PPI
>>
>> Hi Jiewen,
>>
>> Thanks for the feedback, per your questions:
>>
>> 1. The primary use case for this is to enable UEFI variable writes before
>> permanent memory is available.
>> 2. The implementation is a work in progress. We will provide it shortly. As this
>> will be a rather large patch set, I would like to get this piece in place beforehand
>> so that the reviewers can focus on the implementation separate from the API
>> definition.
>> 3. No impact to secure boot. We are not going to support writing to
>> authenticated variables in PEI. As mentioned in the comments, if a PEIM wishes
>> to update any of the authenticated variables it must use the existing HOB
>> mechanism to have a later DXE phase perform the update.
>> 4. With regard to atomicity, we have a complete implementation of the fault
>> tolerant write services operational in Pre-Memory PEI.
>> 5. Good point on the S3 resume, we will need to add an SMI to have the variable
>> services re-initialize the mNvVariableCache.
>>
>> Hope that helps,
>> Nate
>>
>> -----Original Message-----
>> From: Yao, Jiewen <jiewen.yao@intel.com>
>> Sent: Friday, June 10, 2022 9:56 AM
>> To: devel@edk2.groups.io; michael.kubacki@outlook.com; Desimone,
>> Nathaniel L <nathaniel.l.desimone@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
>> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
>> Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
>> Kubacki, Michael <michael.kubacki@microsoft.com>
>> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
>> EDKII_PEI_VARIABLE_PPI
>>
>> Hi
>>
>> I am curious why we need this interface. Why we need write variable capability
>> in PEI phase?
>>
>> Where is the implementation of this? I prefer to see an implementation
>> submitted together with header file.
>> For example, what is the impact to secure boot related feature, how to write
>> auth variable in PEI, how PEI write variable cowork with SMM version in S3
>> resume phase, how to support variable atomicity, etc.
>>
>> Thank you
>> Yao Jiewen
>>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
>>> Kubacki
>>> Sent: Friday, June 10, 2022 10:00 AM
>>> To: devel@edk2.groups.io; Desimone, Nathaniel L
>>> <nathaniel.l.desimone@intel.com>
>>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
>>> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
>>> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
>>> <daocheng.bu@intel.com>; Kubacki, Michael
>>> <michael.kubacki@microsoft.com>
>>> Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
>>> of EDKII_PEI_VARIABLE_PPI
>>>
>>> Is this change just adding the interface to Tianocore or is there
>>> additional implementation planned as well?
>>>
>>> ---
>>>
>>> I thought we were following this convention now:
>>>
>>> "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
>>>
>>> Some other comments are inline.
>>>
>>> Regards,
>>> Michael
>>>
>>> On 6/9/2022 9:17 PM, Nate DeSimone wrote:
>>>> Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to enabling
>>>> variable writes in the PEI environment.
>>>>
>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Cc: Isaac Oram <isaac.w.oram@intel.com>
>>>> Cc: Chasel Chiu <chasel.chiu@intel.com>
>>>> Cc: Gao Cheng <gao.cheng@intel.com>
>>>> Cc: Di Zhang <di.zhang@intel.com>
>>>> Cc: Daocheng Bu <daocheng.bu@intel.com>
>>>> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
>>>> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
>>>> ---
>>>> MdeModulePkg/Include/Ppi/Variable.h | 189
>>> ++++++++++++++++++++++++++++
>>>> MdeModulePkg/MdeModulePkg.dec | 3 +
>>>> 2 files changed, 192 insertions(+)
>>>> create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
>>>>
>>>> diff --git a/MdeModulePkg/Include/Ppi/Variable.h
>>> b/MdeModulePkg/Include/Ppi/Variable.h
>>>> new file mode 100644
>>>> index 0000000000..97dc7ceefa
>>>> --- /dev/null
>>>> +++ b/MdeModulePkg/Include/Ppi/Variable.h
>>>> @@ -0,0 +1,189 @@
>>>> +/** @file
>>>> + EDKII PEI Variable Protocol provides an implementation of
>>>> +variables
>>>
>>> [MK] Was "EDKII PEI Variable PPI" intended?
>>>
>>>> + intended for use as a means to store data in the PEI environment.
>>>> +
>>>> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>>>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> +
>>>> +**/
>>>> +
>>>> +#ifndef __PEI_VARIABLE_PPI_H_
>>>> +#define __PEI_VARIABLE_PPI_H_
>>>> +
>>>> +#define EDKII_PEI_VARIABLE_PPI_GUID \
>>>> + { \
>>>> + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b,
>>>> +0x66, 0x4a,
>>> 0xb0 } \
>>>> + }
>>>> +
>>>> +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
>>>> +
>>>> +/**
>>>> + This service retrieves a variable's value using its name and GUID.
>>>> +
>>>> + Read the specified variable from the UEFI variable store. If the
>>>> + Data buffer is too small to hold the contents of the variable,
>>>> + the error EFI_BUFFER_TOO_SMALL is returned and DataSize is set to
>>>> + the required buffer size to obtain the data.
>>>> +
>>>> + @param[in] This A pointer to this instance of the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in] VariableName A pointer to a null-terminated string that
>>> is the variable's name.
>>>> + @param[in] VariableGuid A pointer to an EFI_GUID that is the
>>> variable's GUID. The combination of
>>>> + VariableGuid and VariableName must be unique.
>>>> + @param[out] Attributes If non-NULL, on return, points to the
>>> variable's attributes.
>>>> + @param[in, out] DataSize On entry, points to the size in bytes of the
>>> Data buffer.
>>>> + On return, points to the size of the data returned in
>> Data.
>>>> + @param[out] Data Points to the buffer which will hold the
>>> returned variable value.
>>>> + May be NULL with a zero
>>>> + DataSize in order to determine
>>> the size of the
>>>> + buffer needed.
>>>> +
>>>> + @retval EFI_SUCCESS The variable was read successfully.
>>>> + @retval EFI_NOT_FOUND The variable was not found.
>>>> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the
>>> resulting data.
>>>> + DataSize is updated with the size required for
>>>> + the specified variable.
>>>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
>>> DataSize or Data is NULL.
>>>> + @retval EFI_DEVICE_ERROR The variable could not be retrieved
>>> because of a device error.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN CONST CHAR16 *VariableName,
>>>> + IN CONST EFI_GUID *VariableGuid,
>>>> + OUT UINT32 *Attributes,
>>>
>>> [MK] Based on the description, Attributes should be marked "OPTIONAL".
>>>
>>>> + IN OUT UINTN *DataSize,
>>>> + OUT VOID *Data OPTIONAL
>>>> + );
>>>> +
>>>> +/**
>>>> + Return the next variable name and GUID.
>>>> +
>>>> + This function is called multiple times to retrieve the
>>>> + VariableName and VariableGuid of all variables currently available in the
>> system.
>>>> + On each call, the previous results are passed into the interface,
>>>> + and, on return, the interface returns the data for the next
>>>> + interface. When the entire variable list has been returned,
>>>> + EFI_NOT_FOUND is returned.
>>>> +
>>>
>>> [MK] I know other descriptions don't usually have it but it would be
>>> nice to describe the initial calling values expected.
>>>
>>>> + @param[in] This A pointer to this instance of the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in, out] VariableNameSize On entry, points to the size of the
>>> buffer pointed to by VariableName.
>>>> + On return, the size of the variable name buffer.
>>>> + @param[in, out] VariableName On entry, a pointer to a null-
>> terminated
>>> string that is the variable's name.
>>>> + On return, points to the next
>>>> + variable's null-terminated
>>> name string.
>>>> + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that
>> is
>>> the variable's GUID.
>>>> + On return, a pointer to the next variable's GUID.
>>>> +
>>>> + @retval EFI_SUCCESS The variable was read successfully.
>>>> + @retval EFI_NOT_FOUND The variable could not be found.
>>>> + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small
>> for
>>> the resulting
>>>> + data. VariableNameSize is updated with the size
>>>> + required for the specified variable.
>>>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
>>>> + VariableNameSize is NULL.
>>>> + @retval EFI_DEVICE_ERROR The variable could not be retrieved
>>> because of a device error.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN OUT UINTN *VariableNameSize,
>>>> + IN OUT CHAR16 *VariableName,
>>>> + IN OUT EFI_GUID *VariableGuid
>>>> + );
>>>> +
>>>> +/**
>>>> + Sets the value of a variable.
>>>> +
>>>> + @param[in] This A pointer to this instance of the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in] VariableName A Null-terminated string that is the name
>>> of the vendor's variable.
>>>> + Each VariableName is unique for each VendorGuid.
>>> VariableName must
>>>> + contain 1 or more characters.
>>>> + If VariableName is an
>>> empty string,
>>>> + then EFI_INVALID_PARAMETER is returned.
>>>> + @param[in] VendorGuid A unique identifier for the vendor.
>>>> + @param[in] Attributes Attributes bitmask to set for the variable.
>>>> + @param[in] DataSize The size in bytes of the Data buffer. Unless
>>> the EFI_VARIABLE_APPEND_WRITE
>>>> + attribute is set, a size of
>>>> + zero causes the variable to be
>>> deleted. When the
>>>> + EFI_VARIABLE_APPEND_WRITE
>>>> + attribute is set, then a
>>> SetVariable() call with a
>>>> + DataSize of zero will not
>>>> + cause any change to the
>>> variable value.
>>>> + @param[in] Data The contents for the variable.
>>>> +
>>>> + @retval EFI_SUCCESS The firmware has successfully stored the
>>> variable and its data as
>>>> + defined by the Attributes.
>>>> + @retval EFI_INVALID_PARAMETER An invalid combination of attribute
>>> bits, name, and GUID was supplied, or the
>>>> + DataSize exceeds the maximum allowed.
>>>> + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
>>>> + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to
>>> hold the variable and its data.
>>>> + @retval EFI_DEVICE_ERROR The variable could not be retrieved due
>>> to a hardware error.
>>>> + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
>>>> + @retval EFI_WRITE_PROTECTED The variable in question cannot be
>>> deleted.
>>>> + @retval EFI_SECURITY_VIOLATION The variable could not be written
>> due
>>> to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
>>>> + or
>>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
>>>> +
>>>> + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
>>> being set. Writing to authenticated
>>>> + variables is not supported in the PEI environment.
>>> Updates to authenticated
>>>> + variables can be requested
>>>> + during PEI via the
>>> EFI_AUTHENTICATED_VARIABLE_HOB, but
>>>> + these updates won't be
>>>> + written to non-volatile storage
>>> until later in DXE. See
>>>> +
>>>> + MdeModulePkg/Include/Guid/VariableFormat.h for
>>> more details on
>>>> + EFI_AUTHENTICATED_VARIABLE_HOB.
>>>
>>> [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
>>> VariableFormat.h.
>>>
>>> [MK] It seems that if a contract for producing and then consuming this
>>> HOB is going to be defined between the HOB producer and consumer
>>> phase, it should be described in something like the PI Spec.
>>>
>>>> + @retval EFI_NOT_FOUND The variable trying to be updated or
>>> deleted was not found.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN CHAR16 *VariableName,
>>>> + IN EFI_GUID *VendorGuid,
>>>> + IN UINT32 Attributes,
>>>> + IN UINTN DataSize,
>>>> + IN VOID *Data
>>>> + );
>>>> +
>>>> +/**
>>>> + Returns information about the UEFI variables.
>>>> +
>>>> + @param[in] This A pointer to this instance of the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in] Attributes Attributes bitmask to specify the type
>>> of variables on
>>>> + which to return information.
>>>> + @param[out] MaximumVariableStorageSize On output the maximum
>>> size of the storage space
>>>> + available for the EFI variables associated with
>> the
>>>> + attributes specified.
>>>> + @param[out] RemainingVariableStorageSize Returns the remaining
>> size
>>> of the storage space
>>>> + available for the EFI variables associated with
>> the
>>>> + attributes specified.
>>>> + @param[out] MaximumVariableSize Returns the maximum size of
>>> the individual EFI
>>>> + variables associated with the attributes
>> specified.
>>>> +
>>>> + @retval EFI_SUCCESS Valid answer returned.
>>>> + @retval EFI_INVALID_PARAMETER An invalid combination of
>>> attribute bits was supplied
>>>> + @retval EFI_UNSUPPORTED The attribute is not supported
>> on
>>> this platform, and the
>>>> + MaximumVariableStorageSize,
>>>> +
>>>> + RemainingVariableStorageSize,
>>> MaximumVariableSize
>>>> + are undefined.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN UINT32 Attributes,
>>>> + OUT UINT64 *MaximumVariableStorageSize,
>>>> + OUT UINT64 *RemainingVariableStorageSize,
>>>> + OUT UINT64 *MaximumVariableSize
>>>> + );
>>>> +
>>>> +///
>>>> +/// PEI Variable Protocol is intended for use as a means
>>>
>>> [MK] Was "PEI Variable PPI" intended?
>>>
>>>> +/// to store data in the PEI environment.
>>>> +///
>>>> +struct _EDKII_PEI_VARIABLE_PPI {
>>>> + EDKII_PEI_GET_VARIABLE GetVariable;
>>>> + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
>>>> + EDKII_PEI_SET_VARIABLE SetVariable;
>>>> + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
>>>> +};
>>>> +
>>>> +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
>>>> +
>>>> +#endif
>>>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>>> b/MdeModulePkg/MdeModulePkg.dec
>>>> index 2bcb9f9453..4f4c48b81f 100644
>>>> --- a/MdeModulePkg/MdeModulePkg.dec
>>>> +++ b/MdeModulePkg/MdeModulePkg.dec
>>>> @@ -513,6 +513,9 @@
>>>> gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d,
>>> { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
>>>> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
>>> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
>>>>
>>>> + ## Include/Ppi/Variable.h
>>>> + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7,
>>> 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
>>>> +
>>>> [Protocols]
>>>> ## Load File protocol provides capability to load and unload EFI
>>>> image into
>>> memory and execute it.
>>>> # Include/Protocol/LoadPe32Image.h
>>>
>>>
>>>
>>>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-11 1:09 ` Yao, Jiewen
2022-06-11 1:14 ` Michael Kubacki
@ 2022-06-13 21:31 ` Nate DeSimone
2022-06-14 10:44 ` Albecki, Mateusz
2022-06-22 19:51 ` Brian J. Johnson
1 sibling, 2 replies; 13+ messages in thread
From: Nate DeSimone @ 2022-06-13 21:31 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, michael.kubacki@outlook.com
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
Hi Jiewen,
I am fine with deferring the submission of this to edk2 until the implementation is ready for review. I just wanted to get feedback on the API so that once the implementation patch series arrives we will at least that that piece of the review done. I would say this thread achieved that goal. Thank you for the pointer to the protected variable code, we will review it and make sure that no issues would arise from the pre-memory PEI implementation.
Thanks,
Nate
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, June 10, 2022 6:09 PM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io; michael.kubacki@outlook.com
Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>; Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
Thanks for the response.
1) Why we need "enable UEFI variable write before permanent memory is available"?
2) If the implementation is not ready, I do have concern to add it so early in EDKII.
If I don’t have a big picture, I am not sure how to review the completeness.
Can we put it to EDKII-staging (https://github.com/tianocore/edk2-staging) for a moment?
I don’t see the need to add the interface now for work-in-progress feature, since there is no consumer and no producer.
Another reason is that I happen to know other feature (in EDKII stage) is impacting variable driver.
https://github.com/tianocore/edk2-staging/tree/ProtectedVariable/libs
Please do consider that as well - how to write a protected variable in PEI phase.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Saturday, June 11, 2022 5:49 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
> michael.kubacki@outlook.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
> <daocheng.bu@intel.com>; Kubacki, Michael
> <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
> of EDKII_PEI_VARIABLE_PPI
>
> Hi Jiewen,
>
> Thanks for the feedback, per your questions:
>
> 1. The primary use case for this is to enable UEFI variable writes
> before permanent memory is available.
> 2. The implementation is a work in progress. We will provide it
> shortly. As this will be a rather large patch set, I would like to get
> this piece in place beforehand so that the reviewers can focus on the
> implementation separate from the API definition.
> 3. No impact to secure boot. We are not going to support writing to
> authenticated variables in PEI. As mentioned in the comments, if a
> PEIM wishes to update any of the authenticated variables it must use
> the existing HOB mechanism to have a later DXE phase perform the update.
> 4. With regard to atomicity, we have a complete implementation of the
> fault tolerant write services operational in Pre-Memory PEI.
> 5. Good point on the S3 resume, we will need to add an SMI to have the
> variable services re-initialize the mNvVariableCache.
>
> Hope that helps,
> Nate
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, June 10, 2022 9:56 AM
> To: devel@edk2.groups.io; michael.kubacki@outlook.com; Desimone,
> Nathaniel L <nathaniel.l.desimone@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
> <daocheng.bu@intel.com>; Kubacki, Michael
> <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
> of EDKII_PEI_VARIABLE_PPI
>
> Hi
>
> I am curious why we need this interface. Why we need write variable
> capability in PEI phase?
>
> Where is the implementation of this? I prefer to see an implementation
> submitted together with header file.
> For example, what is the impact to secure boot related feature, how to
> write auth variable in PEI, how PEI write variable cowork with SMM
> version in S3 resume phase, how to support variable atomicity, etc.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Michael Kubacki
> > Sent: Friday, June 10, 2022 10:00 AM
> > To: devel@edk2.groups.io; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Oram, Isaac W
> > <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> > Cheng, Gao <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>;
> > Bu, Daocheng <daocheng.bu@intel.com>; Kubacki, Michael
> > <michael.kubacki@microsoft.com>
> > Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add
> > Definition of EDKII_PEI_VARIABLE_PPI
> >
> > Is this change just adding the interface to Tianocore or is there
> > additional implementation planned as well?
> >
> > ---
> >
> > I thought we were following this convention now:
> >
> > "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
> >
> > Some other comments are inline.
> >
> > Regards,
> > Michael
> >
> > On 6/9/2022 9:17 PM, Nate DeSimone wrote:
> > > Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to
> > > enabling variable writes in the PEI environment.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Isaac Oram <isaac.w.oram@intel.com>
> > > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > > Cc: Gao Cheng <gao.cheng@intel.com>
> > > Cc: Di Zhang <di.zhang@intel.com>
> > > Cc: Daocheng Bu <daocheng.bu@intel.com>
> > > Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> > > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > > ---
> > > MdeModulePkg/Include/Ppi/Variable.h | 189
> > ++++++++++++++++++++++++++++
> > > MdeModulePkg/MdeModulePkg.dec | 3 +
> > > 2 files changed, 192 insertions(+)
> > > create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
> > >
> > > diff --git a/MdeModulePkg/Include/Ppi/Variable.h
> > b/MdeModulePkg/Include/Ppi/Variable.h
> > > new file mode 100644
> > > index 0000000000..97dc7ceefa
> > > --- /dev/null
> > > +++ b/MdeModulePkg/Include/Ppi/Variable.h
> > > @@ -0,0 +1,189 @@
> > > +/** @file
> > > + EDKII PEI Variable Protocol provides an implementation of
> > > +variables
> >
> > [MK] Was "EDKII PEI Variable PPI" intended?
> >
> > > + intended for use as a means to store data in the PEI environment.
> > > +
> > > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#ifndef __PEI_VARIABLE_PPI_H_
> > > +#define __PEI_VARIABLE_PPI_H_
> > > +
> > > +#define EDKII_PEI_VARIABLE_PPI_GUID \
> > > + { \
> > > + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b,
> > > +0x66, 0x4a,
> > 0xb0 } \
> > > + }
> > > +
> > > +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
> > > +
> > > +/**
> > > + This service retrieves a variable's value using its name and GUID.
> > > +
> > > + Read the specified variable from the UEFI variable store. If
> > > + the Data buffer is too small to hold the contents of the
> > > + variable, the error EFI_BUFFER_TOO_SMALL is returned and
> > > + DataSize is set to the required buffer size to obtain the data.
> > > +
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in] VariableName A pointer to a null-terminated string that
> > is the variable's name.
> > > + @param[in] VariableGuid A pointer to an EFI_GUID that is the
> > variable's GUID. The combination of
> > > + VariableGuid and VariableName must be unique.
> > > + @param[out] Attributes If non-NULL, on return, points to the
> > variable's attributes.
> > > + @param[in, out] DataSize On entry, points to the size in bytes of the
> > Data buffer.
> > > + On return, points to the
> > > + size of the data returned in
> Data.
> > > + @param[out] Data Points to the buffer which will hold the
> > returned variable value.
> > > + May be NULL with a zero
> > > + DataSize in order to determine
> > the size of the
> > > + buffer needed.
> > > +
> > > + @retval EFI_SUCCESS The variable was read successfully.
> > > + @retval EFI_NOT_FOUND The variable was not found.
> > > + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the
> > resulting data.
> > > + DataSize is updated with the size required for
> > > + the specified variable.
> > > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
> > DataSize or Data is NULL.
> > > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> > because of a device error.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN CONST CHAR16 *VariableName,
> > > + IN CONST EFI_GUID *VariableGuid,
> > > + OUT UINT32 *Attributes,
> >
> > [MK] Based on the description, Attributes should be marked "OPTIONAL".
> >
> > > + IN OUT UINTN *DataSize,
> > > + OUT VOID *Data OPTIONAL
> > > + );
> > > +
> > > +/**
> > > + Return the next variable name and GUID.
> > > +
> > > + This function is called multiple times to retrieve the
> > > + VariableName and VariableGuid of all variables currently
> > > + available in the
> system.
> > > + On each call, the previous results are passed into the
> > > + interface, and, on return, the interface returns the data for
> > > + the next interface. When the entire variable list has been
> > > + returned, EFI_NOT_FOUND is returned.
> > > +
> >
> > [MK] I know other descriptions don't usually have it but it would be
> > nice to describe the initial calling values expected.
> >
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in, out] VariableNameSize On entry, points to the size of the
> > buffer pointed to by VariableName.
> > > + On return, the size of the variable name buffer.
> > > + @param[in, out] VariableName On entry, a pointer to a null-
> terminated
> > string that is the variable's name.
> > > + On return, points to the
> > > + next variable's null-terminated
> > name string.
> > > + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that
> is
> > the variable's GUID.
> > > + On return, a pointer to the next variable's GUID.
> > > +
> > > + @retval EFI_SUCCESS The variable was read successfully.
> > > + @retval EFI_NOT_FOUND The variable could not be found.
> > > + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small
> for
> > the resulting
> > > + data. VariableNameSize is updated with the size
> > > + required for the specified variable.
> > > + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
> > > + VariableNameSize is NULL.
> > > + @retval EFI_DEVICE_ERROR The variable could not be retrieved
> > because of a device error.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN OUT UINTN *VariableNameSize,
> > > + IN OUT CHAR16 *VariableName,
> > > + IN OUT EFI_GUID *VariableGuid
> > > + );
> > > +
> > > +/**
> > > + Sets the value of a variable.
> > > +
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in] VariableName A Null-terminated string that is the name
> > of the vendor's variable.
> > > + Each VariableName is unique for each VendorGuid.
> > VariableName must
> > > + contain 1 or more characters.
> > > + If VariableName is an
> > empty string,
> > > + then EFI_INVALID_PARAMETER is returned.
> > > + @param[in] VendorGuid A unique identifier for the vendor.
> > > + @param[in] Attributes Attributes bitmask to set for the variable.
> > > + @param[in] DataSize The size in bytes of the Data buffer. Unless
> > the EFI_VARIABLE_APPEND_WRITE
> > > + attribute is set, a size of
> > > + zero causes the variable to be
> > deleted. When the
> > > + EFI_VARIABLE_APPEND_WRITE
> > > + attribute is set, then a
> > SetVariable() call with a
> > > + DataSize of zero will not
> > > + cause any change to the
> > variable value.
> > > + @param[in] Data The contents for the variable.
> > > +
> > > + @retval EFI_SUCCESS The firmware has successfully stored the
> > variable and its data as
> > > + defined by the Attributes.
> > > + @retval EFI_INVALID_PARAMETER An invalid combination of attribute
> > bits, name, and GUID was supplied, or the
> > > + DataSize exceeds the maximum allowed.
> > > + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
> > > + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to
> > hold the variable and its data.
> > > + @retval EFI_DEVICE_ERROR The variable could not be retrieved due
> > to a hardware error.
> > > + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> > > + @retval EFI_WRITE_PROTECTED The variable in question cannot be
> > deleted.
> > > + @retval EFI_SECURITY_VIOLATION The variable could not be written
> due
> > to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
> > > + or
> > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
> > > +
> > > + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
> > being set. Writing to authenticated
> > > + variables is not supported in the PEI environment.
> > Updates to authenticated
> > > + variables can be requested
> > > + during PEI via the
> > EFI_AUTHENTICATED_VARIABLE_HOB, but
> > > + these updates won't be
> > > + written to non-volatile storage
> > until later in DXE. See
> > > +
> > > + MdeModulePkg/Include/Guid/VariableFormat.h for
> > more details on
> > > + EFI_AUTHENTICATED_VARIABLE_HOB.
> >
> > [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
> > VariableFormat.h.
> >
> > [MK] It seems that if a contract for producing and then consuming
> > this HOB is going to be defined between the HOB producer and
> > consumer phase, it should be described in something like the PI Spec.
> >
> > > + @retval EFI_NOT_FOUND The variable trying to be updated or
> > deleted was not found.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN CHAR16 *VariableName,
> > > + IN EFI_GUID *VendorGuid,
> > > + IN UINT32 Attributes,
> > > + IN UINTN DataSize,
> > > + IN VOID *Data
> > > + );
> > > +
> > > +/**
> > > + Returns information about the UEFI variables.
> > > +
> > > + @param[in] This A pointer to this instance of the
> > EDKII_PEI_VARIABLE_PPI.
> > > + @param[in] Attributes Attributes bitmask to specify the type
> > of variables on
> > > + which to return information.
> > > + @param[out] MaximumVariableStorageSize On output the maximum
> > size of the storage space
> > > + available for
> > > + the EFI variables associated with
> the
> > > + attributes specified.
> > > + @param[out] RemainingVariableStorageSize Returns the remaining
> size
> > of the storage space
> > > + available for
> > > + the EFI variables associated with
> the
> > > + attributes specified.
> > > + @param[out] MaximumVariableSize Returns the maximum size of
> > the individual EFI
> > > + variables
> > > + associated with the attributes
> specified.
> > > +
> > > + @retval EFI_SUCCESS Valid answer returned.
> > > + @retval EFI_INVALID_PARAMETER An invalid combination of
> > attribute bits was supplied
> > > + @retval EFI_UNSUPPORTED The attribute is not supported
> on
> > this platform, and the
> > > +
> > > + MaximumVariableStorageSize,
> > > +
> > > + RemainingVariableStorageSize,
> > MaximumVariableSize
> > > + are undefined.
> > > +
> > > +**/
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
> > > + IN CONST EDKII_PEI_VARIABLE_PPI *This,
> > > + IN UINT32 Attributes,
> > > + OUT UINT64 *MaximumVariableStorageSize,
> > > + OUT UINT64 *RemainingVariableStorageSize,
> > > + OUT UINT64 *MaximumVariableSize
> > > + );
> > > +
> > > +///
> > > +/// PEI Variable Protocol is intended for use as a means
> >
> > [MK] Was "PEI Variable PPI" intended?
> >
> > > +/// to store data in the PEI environment.
> > > +///
> > > +struct _EDKII_PEI_VARIABLE_PPI {
> > > + EDKII_PEI_GET_VARIABLE GetVariable;
> > > + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
> > > + EDKII_PEI_SET_VARIABLE SetVariable;
> > > + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
> > > +};
> > > +
> > > +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
> > > +
> > > +#endif
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec
> > > index 2bcb9f9453..4f4c48b81f 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -513,6 +513,9 @@
> > > gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d,
> > { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> > > gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
> > 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
> > >
> > > + ## Include/Ppi/Variable.h
> > > + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7,
> > 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
> > > +
> > > [Protocols]
> > > ## Load File protocol provides capability to load and unload
> > > EFI image into
> > memory and execute it.
> > > # Include/Protocol/LoadPe32Image.h
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-13 21:31 ` Nate DeSimone
@ 2022-06-14 10:44 ` Albecki, Mateusz
2022-06-22 19:51 ` Brian J. Johnson
1 sibling, 0 replies; 13+ messages in thread
From: Albecki, Mateusz @ 2022-06-14 10:44 UTC (permalink / raw)
To: Nate DeSimone, devel
[-- Attachment #1: Type: text/plain, Size: 216 bytes --]
Hi,
I think it would be better to avoid using PEI and PPI in the structure names. This would allow us to install this structure as protocol in DXE/SMM and potentially use it in a common code.
Thanks,
Mateusz
[-- Attachment #2: Type: text/html, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-13 21:31 ` Nate DeSimone
2022-06-14 10:44 ` Albecki, Mateusz
@ 2022-06-22 19:51 ` Brian J. Johnson
2022-06-22 21:02 ` Michael Kubacki
1 sibling, 1 reply; 13+ messages in thread
From: Brian J. Johnson @ 2022-06-22 19:51 UTC (permalink / raw)
To: devel, nathaniel.l.desimone, Yao, Jiewen,
michael.kubacki@outlook.com
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
Nate,
FWIW I welcome having a standard interface to write EFI variables in
PEI. Not all platforms store variables in flash, so they don't all have
tricky issues with fault-tolerant writes to h/w which is also being used
for code fetches. And writing variables early makes it possible to
modify settings in response to h/w changes and external requests,
without having to boot all the way to DXE, write the variables, then
trigger a reset.
Brian J. Johnson
-------- Original Message --------
From: Nate DeSimone [mailto:nathaniel.l.desimone@intel.com]
Sent: Monday, June 13, 2022, 4:31 PM
To: Yao, Jiewen <jiewen.yao@intel.com>, devel@edk2.groups.io
<devel@edk2.groups.io>, michael.kubacki@outlook.com
<michael.kubacki@outlook.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>, Gao, Liming
<gaoliming@byosoft.com.cn>, Kinney, Michael D
<michael.d.kinney@intel.com>, Oram, Isaac W <isaac.w.oram@intel.com>,
Chiu, Chasel <chasel.chiu@intel.com>, Cheng, Gao <gao.cheng@intel.com>,
Zhang, Di <di.zhang@intel.com>, Bu, Daocheng <daocheng.bu@intel.com>,
Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
EDKII_PEI_VARIABLE_PPI
Hi Jiewen,
I am fine with deferring the submission of this to edk2 until the
implementation is ready for review. I just wanted to get feedback on the
API so that once the implementation patch series arrives we will at
least that that piece of the review done. I would say this thread
achieved that goal. Thank you for the pointer to the protected variable
code, we will review it and make sure that no issues would arise from
the pre-memory PEI implementation.
Thanks,
Nate
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, June 10, 2022 6:09 PM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
devel@edk2.groups.io; michael.kubacki@outlook.com
Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
<gaoliming@byosoft.com.cn>; Kinney, Michael D
<michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
EDKII_PEI_VARIABLE_PPI
Thanks for the response.
1) Why we need "enable UEFI variable write before permanent memory is
available"?
2) If the implementation is not ready, I do have concern to add it so
early in EDKII.
If I don’t have a big picture, I am not sure how to review the completeness.
Can we put it to EDKII-staging
(https://github.com/tianocore/edk2-staging) for a moment?
I don’t see the need to add the interface now for work-in-progress
feature, since there is no consumer and no producer.
Another reason is that I happen to know other feature (in EDKII stage)
is impacting variable driver.
https://github.com/tianocore/edk2-staging/tree/ProtectedVariable/libs
Please do consider that as well - how to write a protected variable in
PEI phase.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Saturday, June 11, 2022 5:49 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
> michael.kubacki@outlook.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
> <daocheng.bu@intel.com>; Kubacki, Michael
> <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
> of EDKII_PEI_VARIABLE_PPI
>
> Hi Jiewen,
>
> Thanks for the feedback, per your questions:
>
> 1. The primary use case for this is to enable UEFI variable writes
> before permanent memory is available.
> 2. The implementation is a work in progress. We will provide it
> shortly. As this will be a rather large patch set, I would like to get
> this piece in place beforehand so that the reviewers can focus on the
> implementation separate from the API definition.
> 3. No impact to secure boot. We are not going to support writing to
> authenticated variables in PEI. As mentioned in the comments, if a
> PEIM wishes to update any of the authenticated variables it must use
> the existing HOB mechanism to have a later DXE phase perform the update.
> 4. With regard to atomicity, we have a complete implementation of the
> fault tolerant write services operational in Pre-Memory PEI.
> 5. Good point on the S3 resume, we will need to add an SMI to have the
> variable services re-initialize the mNvVariableCache.
>
> Hope that helps,
> Nate
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, June 10, 2022 9:56 AM
> To: devel@edk2.groups.io; michael.kubacki@outlook.com; Desimone,
> Nathaniel L <nathaniel.l.desimone@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
> <daocheng.bu@intel.com>; Kubacki, Michael
> <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
> of EDKII_PEI_VARIABLE_PPI
>
> Hi
>
> I am curious why we need this interface. Why we need write variable
> capability in PEI phase?
>
> Where is the implementation of this? I prefer to see an implementation
> submitted together with header file.
> For example, what is the impact to secure boot related feature, how to
> write auth variable in PEI, how PEI write variable cowork with SMM
> version in S3 resume phase, how to support variable atomicity, etc.
>
> Thank you
> Yao Jiewen
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>> Michael Kubacki
>> Sent: Friday, June 10, 2022 10:00 AM
>> To: devel@edk2.groups.io; Desimone, Nathaniel L
>> <nathaniel.l.desimone@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Oram, Isaac W
>> <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
>> Cheng, Gao <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>;
>> Bu, Daocheng <daocheng.bu@intel.com>; Kubacki, Michael
>> <michael.kubacki@microsoft.com>
>> Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add
>> Definition of EDKII_PEI_VARIABLE_PPI
>>
>> Is this change just adding the interface to Tianocore or is there
>> additional implementation planned as well?
>>
>> ---
>>
>> I thought we were following this convention now:
>>
>> "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
>>
>> Some other comments are inline.
>>
>> Regards,
>> Michael
>>
>> On 6/9/2022 9:17 PM, Nate DeSimone wrote:
>>> Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to
>>> enabling variable writes in the PEI environment.
>>>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Isaac Oram <isaac.w.oram@intel.com>
>>> Cc: Chasel Chiu <chasel.chiu@intel.com>
>>> Cc: Gao Cheng <gao.cheng@intel.com>
>>> Cc: Di Zhang <di.zhang@intel.com>
>>> Cc: Daocheng Bu <daocheng.bu@intel.com>
>>> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
>>> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
>>> ---
>>> MdeModulePkg/Include/Ppi/Variable.h | 189
>> ++++++++++++++++++++++++++++
>>> MdeModulePkg/MdeModulePkg.dec | 3 +
>>> 2 files changed, 192 insertions(+)
>>> create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
>>>
>>> diff --git a/MdeModulePkg/Include/Ppi/Variable.h
>> b/MdeModulePkg/Include/Ppi/Variable.h
>>> new file mode 100644
>>> index 0000000000..97dc7ceefa
>>> --- /dev/null
>>> +++ b/MdeModulePkg/Include/Ppi/Variable.h
>>> @@ -0,0 +1,189 @@
>>> +/** @file
>>> + EDKII PEI Variable Protocol provides an implementation of
>>> +variables
>>
>> [MK] Was "EDKII PEI Variable PPI" intended?
>>
>>> + intended for use as a means to store data in the PEI environment.
>>> +
>>> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +
>>> +**/
>>> +
>>> +#ifndef __PEI_VARIABLE_PPI_H_
>>> +#define __PEI_VARIABLE_PPI_H_
>>> +
>>> +#define EDKII_PEI_VARIABLE_PPI_GUID \
>>> + { \
>>> + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b,
>>> +0x66, 0x4a,
>> 0xb0 } \
>>> + }
>>> +
>>> +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
>>> +
>>> +/**
>>> + This service retrieves a variable's value using its name and GUID.
>>> +
>>> + Read the specified variable from the UEFI variable store. If
>>> + the Data buffer is too small to hold the contents of the
>>> + variable, the error EFI_BUFFER_TOO_SMALL is returned and
>>> + DataSize is set to the required buffer size to obtain the data.
>>> +
>>> + @param[in] This A pointer to this instance of the
>> EDKII_PEI_VARIABLE_PPI.
>>> + @param[in] VariableName A pointer to a null-terminated string that
>> is the variable's name.
>>> + @param[in] VariableGuid A pointer to an EFI_GUID that is the
>> variable's GUID. The combination of
>>> + VariableGuid and VariableName must be unique.
>>> + @param[out] Attributes If non-NULL, on return, points to the
>> variable's attributes.
>>> + @param[in, out] DataSize On entry, points to the size in bytes of the
>> Data buffer.
>>> + On return, points to the
>>> + size of the data returned in
> Data.
>>> + @param[out] Data Points to the buffer which will hold the
>> returned variable value.
>>> + May be NULL with a zero
>>> + DataSize in order to determine
>> the size of the
>>> + buffer needed.
>>> +
>>> + @retval EFI_SUCCESS The variable was read successfully.
>>> + @retval EFI_NOT_FOUND The variable was not found.
>>> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for the
>> resulting data.
>>> + DataSize is updated with the size required for
>>> + the specified variable.
>>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
>> DataSize or Data is NULL.
>>> + @retval EFI_DEVICE_ERROR The variable could not be retrieved
>> because of a device error.
>>> +
>>> +**/
>>> +typedef
>>> +EFI_STATUS
>>> +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>> + IN CONST CHAR16 *VariableName,
>>> + IN CONST EFI_GUID *VariableGuid,
>>> + OUT UINT32 *Attributes,
>>
>> [MK] Based on the description, Attributes should be marked "OPTIONAL".
>>
>>> + IN OUT UINTN *DataSize,
>>> + OUT VOID *Data OPTIONAL
>>> + );
>>> +
>>> +/**
>>> + Return the next variable name and GUID.
>>> +
>>> + This function is called multiple times to retrieve the
>>> + VariableName and VariableGuid of all variables currently
>>> + available in the
> system.
>>> + On each call, the previous results are passed into the
>>> + interface, and, on return, the interface returns the data for
>>> + the next interface. When the entire variable list has been
>>> + returned, EFI_NOT_FOUND is returned.
>>> +
>>
>> [MK] I know other descriptions don't usually have it but it would be
>> nice to describe the initial calling values expected.
>>
>>> + @param[in] This A pointer to this instance of the
>> EDKII_PEI_VARIABLE_PPI.
>>> + @param[in, out] VariableNameSize On entry, points to the size of the
>> buffer pointed to by VariableName.
>>> + On return, the size of the variable name buffer.
>>> + @param[in, out] VariableName On entry, a pointer to a null-
> terminated
>> string that is the variable's name.
>>> + On return, points to the
>>> + next variable's null-terminated
>> name string.
>>> + @param[in, out] VariableGuid On entry, a pointer to an EFI_GUID that
> is
>> the variable's GUID.
>>> + On return, a pointer to the next variable's GUID.
>>> +
>>> + @retval EFI_SUCCESS The variable was read successfully.
>>> + @retval EFI_NOT_FOUND The variable could not be found.
>>> + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too small
> for
>> the resulting
>>> + data. VariableNameSize is updated with the size
>>> + required for the specified variable.
>>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
>>> + VariableNameSize is NULL.
>>> + @retval EFI_DEVICE_ERROR The variable could not be retrieved
>> because of a device error.
>>> +
>>> +**/
>>> +typedef
>>> +EFI_STATUS
>>> +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>> + IN OUT UINTN *VariableNameSize,
>>> + IN OUT CHAR16 *VariableName,
>>> + IN OUT EFI_GUID *VariableGuid
>>> + );
>>> +
>>> +/**
>>> + Sets the value of a variable.
>>> +
>>> + @param[in] This A pointer to this instance of the
>> EDKII_PEI_VARIABLE_PPI.
>>> + @param[in] VariableName A Null-terminated string that is the name
>> of the vendor's variable.
>>> + Each VariableName is unique for each VendorGuid.
>> VariableName must
>>> + contain 1 or more characters.
>>> + If VariableName is an
>> empty string,
>>> + then EFI_INVALID_PARAMETER is returned.
>>> + @param[in] VendorGuid A unique identifier for the vendor.
>>> + @param[in] Attributes Attributes bitmask to set for the variable.
>>> + @param[in] DataSize The size in bytes of the Data buffer. Unless
>> the EFI_VARIABLE_APPEND_WRITE
>>> + attribute is set, a size of
>>> + zero causes the variable to be
>> deleted. When the
>>> + EFI_VARIABLE_APPEND_WRITE
>>> + attribute is set, then a
>> SetVariable() call with a
>>> + DataSize of zero will not
>>> + cause any change to the
>> variable value.
>>> + @param[in] Data The contents for the variable.
>>> +
>>> + @retval EFI_SUCCESS The firmware has successfully stored the
>> variable and its data as
>>> + defined by the Attributes.
>>> + @retval EFI_INVALID_PARAMETER An invalid combination of attribute
>> bits, name, and GUID was supplied, or the
>>> + DataSize exceeds the maximum allowed.
>>> + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
>>> + @retval EFI_OUT_OF_RESOURCES Not enough storage is available to
>> hold the variable and its data.
>>> + @retval EFI_DEVICE_ERROR The variable could not be retrieved due
>> to a hardware error.
>>> + @retval EFI_WRITE_PROTECTED The variable in question is read-only.
>>> + @retval EFI_WRITE_PROTECTED The variable in question cannot be
>> deleted.
>>> + @retval EFI_SECURITY_VIOLATION The variable could not be written
> due
>> to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
>>> + or
>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
>>> +
>>> + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
>> being set. Writing to authenticated
>>> + variables is not supported in the PEI environment.
>> Updates to authenticated
>>> + variables can be requested
>>> + during PEI via the
>> EFI_AUTHENTICATED_VARIABLE_HOB, but
>>> + these updates won't be
>>> + written to non-volatile storage
>> until later in DXE. See
>>> +
>>> + MdeModulePkg/Include/Guid/VariableFormat.h for
>> more details on
>>> + EFI_AUTHENTICATED_VARIABLE_HOB.
>>
>> [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
>> VariableFormat.h.
>>
>> [MK] It seems that if a contract for producing and then consuming
>> this HOB is going to be defined between the HOB producer and
>> consumer phase, it should be described in something like the PI Spec.
>>
>>> + @retval EFI_NOT_FOUND The variable trying to be updated or
>> deleted was not found.
>>> +
>>> +**/
>>> +typedef
>>> +EFI_STATUS
>>> +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>> + IN CHAR16 *VariableName,
>>> + IN EFI_GUID *VendorGuid,
>>> + IN UINT32 Attributes,
>>> + IN UINTN DataSize,
>>> + IN VOID *Data
>>> + );
>>> +
>>> +/**
>>> + Returns information about the UEFI variables.
>>> +
>>> + @param[in] This A pointer to this instance of the
>> EDKII_PEI_VARIABLE_PPI.
>>> + @param[in] Attributes Attributes bitmask to specify the type
>> of variables on
>>> + which to return information.
>>> + @param[out] MaximumVariableStorageSize On output the maximum
>> size of the storage space
>>> + available for
>>> + the EFI variables associated with
> the
>>> + attributes specified.
>>> + @param[out] RemainingVariableStorageSize Returns the remaining
> size
>> of the storage space
>>> + available for
>>> + the EFI variables associated with
> the
>>> + attributes specified.
>>> + @param[out] MaximumVariableSize Returns the maximum size of
>> the individual EFI
>>> + variables
>>> + associated with the attributes
> specified.
>>> +
>>> + @retval EFI_SUCCESS Valid answer returned.
>>> + @retval EFI_INVALID_PARAMETER An invalid combination of
>> attribute bits was supplied
>>> + @retval EFI_UNSUPPORTED The attribute is not supported
> on
>> this platform, and the
>>> +
>>> + MaximumVariableStorageSize,
>>> +
>>> + RemainingVariableStorageSize,
>> MaximumVariableSize
>>> + are undefined.
>>> +
>>> +**/
>>> +typedef
>>> +EFI_STATUS
>>> +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>> + IN UINT32 Attributes,
>>> + OUT UINT64 *MaximumVariableStorageSize,
>>> + OUT UINT64 *RemainingVariableStorageSize,
>>> + OUT UINT64 *MaximumVariableSize
>>> + );
>>> +
>>> +///
>>> +/// PEI Variable Protocol is intended for use as a means
>>
>> [MK] Was "PEI Variable PPI" intended?
>>
>>> +/// to store data in the PEI environment.
>>> +///
>>> +struct _EDKII_PEI_VARIABLE_PPI {
>>> + EDKII_PEI_GET_VARIABLE GetVariable;
>>> + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
>>> + EDKII_PEI_SET_VARIABLE SetVariable;
>>> + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
>>> +};
>>> +
>>> +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
>>> +
>>> +#endif
>>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>> b/MdeModulePkg/MdeModulePkg.dec
>>> index 2bcb9f9453..4f4c48b81f 100644
>>> --- a/MdeModulePkg/MdeModulePkg.dec
>>> +++ b/MdeModulePkg/MdeModulePkg.dec
>>> @@ -513,6 +513,9 @@
>>> gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61, 0x5a35, 0x4a5d,
>> { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
>>> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
>> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
>>>
>>> + ## Include/Ppi/Variable.h
>>> + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7,
>> 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
>>> +
>>> [Protocols]
>>> ## Load File protocol provides capability to load and unload
>>> EFI image into
>> memory and execute it.
>>> # Include/Protocol/LoadPe32Image.h
>>
>>
>>
>>
--
Brian
--------------------------------------------------------------------
"Not to find even more ways of saving time, but to look for ways to
spend it"
-- The secret to "celebrating the sidelines of life" - Adair Lara
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
2022-06-22 19:51 ` Brian J. Johnson
@ 2022-06-22 21:02 ` Michael Kubacki
0 siblings, 0 replies; 13+ messages in thread
From: Michael Kubacki @ 2022-06-22 21:02 UTC (permalink / raw)
To: devel, brian.johnson, nathaniel.l.desimone, Yao, Jiewen,
michael.kubacki@outlook.com
Cc: Wang, Jian J, Gao, Liming, Kinney, Michael D, Oram, Isaac W,
Chiu, Chasel, Cheng, Gao, Zhang, Di, Bu, Daocheng,
Kubacki, Michael
There's a lot of major pending changes I'm aware of in the variable
stack. At least:
1. "Variable Protection" - Extensions for encryption and replay protection
https://github.com/judahvang/edk2/tree/rpmc-update
2. "Storage Agnostic Variables / UEFI Variable Router"
https://github.com/makubacki/edk2/tree/storage_agnostic_uefi_variables_w_runtime_cache
I've rebased this a few times and probably will again.
3. "PEI write support"
This email thread
4. Host-based UEFI variable testing we are working on at Microsoft
Each of these capabilities is nice but, the reality is, the UEFI
variable driver already has so much technical debt that each of these
individually are going to significantly impact others. From what I've
observed, most large variable contributions tend to fix similar problems
that affect their code - duplicated helper logic between PEI and DXE
drivers, lack of modularity so individual functions are difficult to
test and reuse, etc. Major vendors often have driver overrides because
variables have evolved over the years and they need improved
traceability, data offload, security features, etc.
(4) is an attempt to manage some of the integration complexity to some
extent by increasing test coverage in an quick and controlled manner.
Since these are all mostly new features, I would prefer to build them in
a new clean design that considers modern use cases upfront rather than
continuing to bolt onto the existing code (and at the minimum, introduce
a lot of merge conflicts between everyone in the process).
It could reuse a lot of preexisting work but integrate things more
cohesively and with better testing infrastructure in place.
Does anyone think a workstream or feature branch for that would be a
better approach?
Thanks,
Michael
On 6/22/2022 3:51 PM, Brian J. Johnson wrote:
> Nate,
>
> FWIW I welcome having a standard interface to write EFI variables in
> PEI. Not all platforms store variables in flash, so they don't all have
> tricky issues with fault-tolerant writes to h/w which is also being used
> for code fetches. And writing variables early makes it possible to
> modify settings in response to h/w changes and external requests,
> without having to boot all the way to DXE, write the variables, then
> trigger a reset.
>
> Brian J. Johnson
> -------- Original Message --------
> From: Nate DeSimone [mailto:nathaniel.l.desimone@intel.com]
> Sent: Monday, June 13, 2022, 4:31 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>, devel@edk2.groups.io
> <devel@edk2.groups.io>, michael.kubacki@outlook.com
> <michael.kubacki@outlook.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>, Gao, Liming
> <gaoliming@byosoft.com.cn>, Kinney, Michael D
> <michael.d.kinney@intel.com>, Oram, Isaac W <isaac.w.oram@intel.com>,
> Chiu, Chasel <chasel.chiu@intel.com>, Cheng, Gao <gao.cheng@intel.com>,
> Zhang, Di <di.zhang@intel.com>, Bu, Daocheng <daocheng.bu@intel.com>,
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
> EDKII_PEI_VARIABLE_PPI
>
> Hi Jiewen,
>
> I am fine with deferring the submission of this to edk2 until the
> implementation is ready for review. I just wanted to get feedback on the
> API so that once the implementation patch series arrives we will at
> least that that piece of the review done. I would say this thread
> achieved that goal. Thank you for the pointer to the protected variable
> code, we will review it and make sure that no issues would arise from
> the pre-memory PEI implementation.
>
> Thanks,
> Nate
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, June 10, 2022 6:09 PM
> To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> devel@edk2.groups.io; michael.kubacki@outlook.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao <gao.cheng@intel.com>;
> Zhang, Di <di.zhang@intel.com>; Bu, Daocheng <daocheng.bu@intel.com>;
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of
> EDKII_PEI_VARIABLE_PPI
>
> Thanks for the response.
>
> 1) Why we need "enable UEFI variable write before permanent memory is
> available"?
>
> 2) If the implementation is not ready, I do have concern to add it so
> early in EDKII.
> If I don’t have a big picture, I am not sure how to review the
> completeness.
>
> Can we put it to EDKII-staging
> (https://github.com/tianocore/edk2-staging) for a moment?
> I don’t see the need to add the interface now for work-in-progress
> feature, since there is no consumer and no producer.
>
> Another reason is that I happen to know other feature (in EDKII stage)
> is impacting variable driver.
> https://github.com/tianocore/edk2-staging/tree/ProtectedVariable/libs
>
> Please do consider that as well - how to write a protected variable in
> PEI phase.
>
> Thank you
> Yao Jiewen
>
>> -----Original Message-----
>> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
>> Sent: Saturday, June 11, 2022 5:49 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
>> michael.kubacki@outlook.com
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
>> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
>> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
>> <daocheng.bu@intel.com>; Kubacki, Michael
>> <michael.kubacki@microsoft.com>
>> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
>> of EDKII_PEI_VARIABLE_PPI
>>
>> Hi Jiewen,
>>
>> Thanks for the feedback, per your questions:
>>
>> 1. The primary use case for this is to enable UEFI variable writes
>> before permanent memory is available.
>> 2. The implementation is a work in progress. We will provide it
>> shortly. As this will be a rather large patch set, I would like to get
>> this piece in place beforehand so that the reviewers can focus on the
>> implementation separate from the API definition.
>> 3. No impact to secure boot. We are not going to support writing to
>> authenticated variables in PEI. As mentioned in the comments, if a
>> PEIM wishes to update any of the authenticated variables it must use
>> the existing HOB mechanism to have a later DXE phase perform the update.
>> 4. With regard to atomicity, we have a complete implementation of the
>> fault tolerant write services operational in Pre-Memory PEI.
>> 5. Good point on the S3 resume, we will need to add an SMI to have the
>> variable services re-initialize the mNvVariableCache.
>>
>> Hope that helps,
>> Nate
>>
>> -----Original Message-----
>> From: Yao, Jiewen <jiewen.yao@intel.com>
>> Sent: Friday, June 10, 2022 9:56 AM
>> To: devel@edk2.groups.io; michael.kubacki@outlook.com; Desimone,
>> Nathaniel L <nathaniel.l.desimone@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
>> Chiu, Chasel <chasel.chiu@intel.com>; Cheng, Gao
>> <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>; Bu, Daocheng
>> <daocheng.bu@intel.com>; Kubacki, Michael
>> <michael.kubacki@microsoft.com>
>> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition
>> of EDKII_PEI_VARIABLE_PPI
>>
>> Hi
>>
>> I am curious why we need this interface. Why we need write variable
>> capability in PEI phase?
>>
>> Where is the implementation of this? I prefer to see an implementation
>> submitted together with header file.
>> For example, what is the impact to secure boot related feature, how to
>> write auth variable in PEI, how PEI write variable cowork with SMM
>> version in S3 resume phase, how to support variable atomicity, etc.
>>
>> Thank you
>> Yao Jiewen
>>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>>> Michael Kubacki
>>> Sent: Friday, June 10, 2022 10:00 AM
>>> To: devel@edk2.groups.io; Desimone, Nathaniel L
>>> <nathaniel.l.desimone@intel.com>
>>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
>>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Oram, Isaac W
>>> <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
>>> Cheng, Gao <gao.cheng@intel.com>; Zhang, Di <di.zhang@intel.com>;
>>> Bu, Daocheng <daocheng.bu@intel.com>; Kubacki, Michael
>>> <michael.kubacki@microsoft.com>
>>> Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add
>>> Definition of EDKII_PEI_VARIABLE_PPI
>>>
>>> Is this change just adding the interface to Tianocore or is there
>>> additional implementation planned as well?
>>>
>>> ---
>>>
>>> I thought we were following this convention now:
>>>
>>> "#ifndef __PEI_VARIABLE_PPI_H_" -> "#ifndef PEI_VARIABLE_PPI_H_"
>>>
>>> Some other comments are inline.
>>>
>>> Regards,
>>> Michael
>>>
>>> On 6/9/2022 9:17 PM, Nate DeSimone wrote:
>>>> Adds definition of EDKII_PEI_VARIABLE_PPI, a pre-cursor to
>>>> enabling variable writes in the PEI environment.
>>>>
>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Cc: Isaac Oram <isaac.w.oram@intel.com>
>>>> Cc: Chasel Chiu <chasel.chiu@intel.com>
>>>> Cc: Gao Cheng <gao.cheng@intel.com>
>>>> Cc: Di Zhang <di.zhang@intel.com>
>>>> Cc: Daocheng Bu <daocheng.bu@intel.com>
>>>> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
>>>> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
>>>> ---
>>>> MdeModulePkg/Include/Ppi/Variable.h | 189
>>> ++++++++++++++++++++++++++++
>>>> MdeModulePkg/MdeModulePkg.dec | 3 +
>>>> 2 files changed, 192 insertions(+)
>>>> create mode 100644 MdeModulePkg/Include/Ppi/Variable.h
>>>>
>>>> diff --git a/MdeModulePkg/Include/Ppi/Variable.h
>>> b/MdeModulePkg/Include/Ppi/Variable.h
>>>> new file mode 100644
>>>> index 0000000000..97dc7ceefa
>>>> --- /dev/null
>>>> +++ b/MdeModulePkg/Include/Ppi/Variable.h
>>>> @@ -0,0 +1,189 @@
>>>> +/** @file
>>>> + EDKII PEI Variable Protocol provides an implementation of
>>>> +variables
>>>
>>> [MK] Was "EDKII PEI Variable PPI" intended?
>>>
>>>> + intended for use as a means to store data in the PEI environment.
>>>> +
>>>> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>>>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> +
>>>> +**/
>>>> +
>>>> +#ifndef __PEI_VARIABLE_PPI_H_
>>>> +#define __PEI_VARIABLE_PPI_H_
>>>> +
>>>> +#define EDKII_PEI_VARIABLE_PPI_GUID \
>>>> + { \
>>>> + 0xe7b2cd04, 0x4b14, 0x44c2, { 0xb7, 0x48, 0xce, 0xaf, 0x2b,
>>>> +0x66, 0x4a,
>>> 0xb0 } \
>>>> + }
>>>> +
>>>> +typedef struct _EDKII_PEI_VARIABLE_PPI EDKII_PEI_VARIABLE_PPI;
>>>> +
>>>> +/**
>>>> + This service retrieves a variable's value using its name and GUID.
>>>> +
>>>> + Read the specified variable from the UEFI variable store. If
>>>> + the Data buffer is too small to hold the contents of the
>>>> + variable, the error EFI_BUFFER_TOO_SMALL is returned and
>>>> + DataSize is set to the required buffer size to obtain the data.
>>>> +
>>>> + @param[in] This A pointer to this instance of
>>>> the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in] VariableName A pointer to a
>>>> null-terminated string that
>>> is the variable's name.
>>>> + @param[in] VariableGuid A pointer to an EFI_GUID that
>>>> is the
>>> variable's GUID. The combination of
>>>> + VariableGuid and VariableName
>>>> must be unique.
>>>> + @param[out] Attributes If non-NULL, on return,
>>>> points to the
>>> variable's attributes.
>>>> + @param[in, out] DataSize On entry, points to the size
>>>> in bytes of the
>>> Data buffer.
>>>> + On return, points to the
>>>> + size of the data returned in
>> Data.
>>>> + @param[out] Data Points to the buffer which
>>>> will hold the
>>> returned variable value.
>>>> + May be NULL with a zero
>>>> + DataSize in order to determine
>>> the size of the
>>>> + buffer needed.
>>>> +
>>>> + @retval EFI_SUCCESS The variable was read
>>>> successfully.
>>>> + @retval EFI_NOT_FOUND The variable was not found.
>>>> + @retval EFI_BUFFER_TOO_SMALL The DataSize is too small for
>>>> the
>>> resulting data.
>>>> + DataSize is updated with the
>>>> size required for
>>>> + the specified variable.
>>>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid,
>>> DataSize or Data is NULL.
>>>> + @retval EFI_DEVICE_ERROR The variable could not be
>>>> retrieved
>>> because of a device error.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_GET_VARIABLE)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN CONST CHAR16 *VariableName,
>>>> + IN CONST EFI_GUID *VariableGuid,
>>>> + OUT UINT32 *Attributes,
>>>
>>> [MK] Based on the description, Attributes should be marked "OPTIONAL".
>>>
>>>> + IN OUT UINTN *DataSize,
>>>> + OUT VOID *Data OPTIONAL
>>>> + );
>>>> +
>>>> +/**
>>>> + Return the next variable name and GUID.
>>>> +
>>>> + This function is called multiple times to retrieve the
>>>> + VariableName and VariableGuid of all variables currently
>>>> + available in the
>> system.
>>>> + On each call, the previous results are passed into the
>>>> + interface, and, on return, the interface returns the data for
>>>> + the next interface. When the entire variable list has been
>>>> + returned, EFI_NOT_FOUND is returned.
>>>> +
>>>
>>> [MK] I know other descriptions don't usually have it but it would be
>>> nice to describe the initial calling values expected.
>>>
>>>> + @param[in] This A pointer to this instance of
>>>> the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in, out] VariableNameSize On entry, points to the size
>>>> of the
>>> buffer pointed to by VariableName.
>>>> + On return, the size of the
>>>> variable name buffer.
>>>> + @param[in, out] VariableName On entry, a pointer to a null-
>> terminated
>>> string that is the variable's name.
>>>> + On return, points to the
>>>> + next variable's null-terminated
>>> name string.
>>>> + @param[in, out] VariableGuid On entry, a pointer to an
>>>> EFI_GUID that
>> is
>>> the variable's GUID.
>>>> + On return, a pointer to the
>>>> next variable's GUID.
>>>> +
>>>> + @retval EFI_SUCCESS The variable was read
>>>> successfully.
>>>> + @retval EFI_NOT_FOUND The variable could not be found.
>>>> + @retval EFI_BUFFER_TOO_SMALL The VariableNameSize is too
>>>> small
>> for
>>> the resulting
>>>> + data. VariableNameSize is
>>>> updated with the size
>>>> + required for the specified
>>>> variable.
>>>> + @retval EFI_INVALID_PARAMETER VariableName, VariableGuid or
>>>> + VariableNameSize is NULL.
>>>> + @retval EFI_DEVICE_ERROR The variable could not be
>>>> retrieved
>>> because of a device error.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_GET_NEXT_VARIABLE_NAME)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN OUT UINTN *VariableNameSize,
>>>> + IN OUT CHAR16 *VariableName,
>>>> + IN OUT EFI_GUID *VariableGuid
>>>> + );
>>>> +
>>>> +/**
>>>> + Sets the value of a variable.
>>>> +
>>>> + @param[in] This A pointer to this instance of
>>>> the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in] VariableName A Null-terminated string that
>>>> is the name
>>> of the vendor's variable.
>>>> + Each VariableName is unique
>>>> for each VendorGuid.
>>> VariableName must
>>>> + contain 1 or more characters.
>>>> + If VariableName is an
>>> empty string,
>>>> + then EFI_INVALID_PARAMETER is
>>>> returned.
>>>> + @param[in] VendorGuid A unique identifier for the
>>>> vendor.
>>>> + @param[in] Attributes Attributes bitmask to set for
>>>> the variable.
>>>> + @param[in] DataSize The size in bytes of the Data
>>>> buffer. Unless
>>> the EFI_VARIABLE_APPEND_WRITE
>>>> + attribute is set, a size of
>>>> + zero causes the variable to be
>>> deleted. When the
>>>> + EFI_VARIABLE_APPEND_WRITE
>>>> + attribute is set, then a
>>> SetVariable() call with a
>>>> + DataSize of zero will not
>>>> + cause any change to the
>>> variable value.
>>>> + @param[in] Data The contents for the variable.
>>>> +
>>>> + @retval EFI_SUCCESS The firmware has successfully
>>>> stored the
>>> variable and its data as
>>>> + defined by the Attributes.
>>>> + @retval EFI_INVALID_PARAMETER An invalid combination of
>>>> attribute
>>> bits, name, and GUID was supplied, or the
>>>> + DataSize exceeds the maximum
>>>> allowed.
>>>> + @retval EFI_INVALID_PARAMETER VariableName is an empty string.
>>>> + @retval EFI_OUT_OF_RESOURCES Not enough storage is
>>>> available to
>>> hold the variable and its data.
>>>> + @retval EFI_DEVICE_ERROR The variable could not be
>>>> retrieved due
>>> to a hardware error.
>>>> + @retval EFI_WRITE_PROTECTED The variable in question is
>>>> read-only.
>>>> + @retval EFI_WRITE_PROTECTED The variable in question
>>>> cannot be
>>> deleted.
>>>> + @retval EFI_SECURITY_VIOLATION The variable could not be
>>>> written
>> due
>>> to EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS,
>>>> + or
>>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS, or
>>>> +
>>>> + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
>>> being set. Writing to authenticated
>>>> + variables is not supported in
>>>> the PEI environment.
>>> Updates to authenticated
>>>> + variables can be requested
>>>> + during PEI via the
>>> EFI_AUTHENTICATED_VARIABLE_HOB, but
>>>> + these updates won't be
>>>> + written to non-volatile storage
>>> until later in DXE. See
>>>> +
>>>> + MdeModulePkg/Include/Guid/VariableFormat.h for
>>> more details on
>>>> + EFI_AUTHENTICATED_VARIABLE_HOB.
>>>
>>> [MK] I didn't see "EFI_AUTHENTICATED_VARIABLE_HOB" mentioned in
>>> VariableFormat.h.
>>>
>>> [MK] It seems that if a contract for producing and then consuming
>>> this HOB is going to be defined between the HOB producer and
>>> consumer phase, it should be described in something like the PI Spec.
>>>
>>>> + @retval EFI_NOT_FOUND The variable trying to be
>>>> updated or
>>> deleted was not found.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_SET_VARIABLE)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN CHAR16 *VariableName,
>>>> + IN EFI_GUID *VendorGuid,
>>>> + IN UINT32 Attributes,
>>>> + IN UINTN DataSize,
>>>> + IN VOID *Data
>>>> + );
>>>> +
>>>> +/**
>>>> + Returns information about the UEFI variables.
>>>> +
>>>> + @param[in] This A pointer to this
>>>> instance of the
>>> EDKII_PEI_VARIABLE_PPI.
>>>> + @param[in] Attributes Attributes
>>>> bitmask to specify the type
>>> of variables on
>>>> + which to return
>>>> information.
>>>> + @param[out] MaximumVariableStorageSize On output the
>>>> maximum
>>> size of the storage space
>>>> + available for
>>>> + the EFI variables associated with
>> the
>>>> + attributes
>>>> specified.
>>>> + @param[out] RemainingVariableStorageSize Returns the
>>>> remaining
>> size
>>> of the storage space
>>>> + available for
>>>> + the EFI variables associated with
>> the
>>>> + attributes
>>>> specified.
>>>> + @param[out] MaximumVariableSize Returns the
>>>> maximum size of
>>> the individual EFI
>>>> + variables
>>>> + associated with the attributes
>> specified.
>>>> +
>>>> + @retval EFI_SUCCESS Valid answer
>>>> returned.
>>>> + @retval EFI_INVALID_PARAMETER An invalid
>>>> combination of
>>> attribute bits was supplied
>>>> + @retval EFI_UNSUPPORTED The attribute is
>>>> not supported
>> on
>>> this platform, and the
>>>> +
>>>> + MaximumVariableStorageSize,
>>>> +
>>>> + RemainingVariableStorageSize,
>>> MaximumVariableSize
>>>> + are undefined.
>>>> +
>>>> +**/
>>>> +typedef
>>>> +EFI_STATUS
>>>> +(EFIAPI *EDKII_PEI_QUERY_VARIABLE_INFO)(
>>>> + IN CONST EDKII_PEI_VARIABLE_PPI *This,
>>>> + IN UINT32 Attributes,
>>>> + OUT UINT64 *MaximumVariableStorageSize,
>>>> + OUT UINT64 *RemainingVariableStorageSize,
>>>> + OUT UINT64 *MaximumVariableSize
>>>> + );
>>>> +
>>>> +///
>>>> +/// PEI Variable Protocol is intended for use as a means
>>>
>>> [MK] Was "PEI Variable PPI" intended?
>>>
>>>> +/// to store data in the PEI environment.
>>>> +///
>>>> +struct _EDKII_PEI_VARIABLE_PPI {
>>>> + EDKII_PEI_GET_VARIABLE GetVariable;
>>>> + EDKII_PEI_GET_NEXT_VARIABLE_NAME GetNextVariableName;
>>>> + EDKII_PEI_SET_VARIABLE SetVariable;
>>>> + EDKII_PEI_QUERY_VARIABLE_INFO QueryVariableInfo;
>>>> +};
>>>> +
>>>> +extern EFI_GUID gEdkiiPeiVariablePpiGuid;
>>>> +
>>>> +#endif
>>>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>>> b/MdeModulePkg/MdeModulePkg.dec
>>>> index 2bcb9f9453..4f4c48b81f 100644
>>>> --- a/MdeModulePkg/MdeModulePkg.dec
>>>> +++ b/MdeModulePkg/MdeModulePkg.dec
>>>> @@ -513,6 +513,9 @@
>>>> gEdkiiPeiCapsuleOnDiskPpiGuid = { 0x71a9ea61,
>>>> 0x5a35, 0x4a5d,
>>> { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
>>>> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid = { 0xb08a11e4, 0xe2b7,
>>> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1 } }
>>>>
>>>> + ## Include/Ppi/Variable.h
>>>> + gEdkiiPeiVariablePpiGuid = { 0xe7b2cd04, 0x4b14,
>>>> 0x44c2, { 0xb7,
>>> 0x48, 0xce, 0xaf, 0x2b, 0x66, 0x4a, 0xb0 } }
>>>> +
>>>> [Protocols]
>>>> ## Load File protocol provides capability to load and unload
>>>> EFI image into
>>> memory and execute it.
>>>> # Include/Protocol/LoadPe32Image.h
>>>
>>>
>>>
>>>
>
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-06-22 21:02 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-10 1:17 [PATCH V1 0/1] Add EDKII_PEI_VARIABLE_PPI Nate DeSimone
2022-06-10 1:17 ` [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI Nate DeSimone
2022-06-10 1:59 ` [edk2-devel] " Michael Kubacki
2022-06-10 2:07 ` Michael Kubacki
2022-06-10 16:56 ` Yao, Jiewen
2022-06-10 21:48 ` Nate DeSimone
2022-06-11 1:09 ` Yao, Jiewen
2022-06-11 1:14 ` Michael Kubacki
2022-06-13 21:31 ` Nate DeSimone
2022-06-14 10:44 ` Albecki, Mateusz
2022-06-22 19:51 ` Brian J. Johnson
2022-06-22 21:02 ` Michael Kubacki
2022-06-10 21:20 ` Nate DeSimone
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox