[edk2-devel] [PATCH V1 1/1] MdeModulePkg: Add Definition of EDKII_PEI_VARIABLE_PPI
Yao, Jiewen
jiewen.yao at intel.com
Fri Jun 10 16:56:06 UTC 2022
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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Friday, June 10, 2022 10:00 AM
> To: devel at edk2.groups.io; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming
> <gaoliming at byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney at intel.com>; Oram, Isaac W <isaac.w.oram at intel.com>;
> Chiu, Chasel <chasel.chiu at intel.com>; Cheng, Gao <gao.cheng at intel.com>;
> Zhang, Di <di.zhang at intel.com>; Bu, Daocheng <daocheng.bu at intel.com>;
> Kubacki, Michael <michael.kubacki at 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 at intel.com>
> > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > Cc: Michael D Kinney <michael.d.kinney at intel.com>
> > Cc: Isaac Oram <isaac.w.oram at intel.com>
> > Cc: Chasel Chiu <chasel.chiu at intel.com>
> > Cc: Gao Cheng <gao.cheng at intel.com>
> > Cc: Di Zhang <di.zhang at intel.com>
> > Cc: Daocheng Bu <daocheng.bu at intel.com>
> > Cc: Michael Kubacki <michael.kubacki at microsoft.com>
> > Signed-off-by: Nate DeSimone <nathaniel.l.desimone at 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
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90446): https://edk2.groups.io/g/devel/message/90446
Mute This Topic: https://groups.io/mt/91659908/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list