public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Nate DeSimone" <nathaniel.l.desimone@intel.com>
To: "Chiu, Chasel" <chasel.chiu@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Ma, Maurice" <maurice.ma@intel.com>, "Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH] IntelFsp2Pkg: Adopt FSP 2.3 specification.
Date: Fri, 1 Oct 2021 22:15:00 +0000	[thread overview]
Message-ID: <MWHPR1101MB21605BE47197537CB42909C8CDAB9@MWHPR1101MB2160.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20211001082848.2710-1-chasel.chiu@intel.com>

Hi Chasel,

My only feedback is let's not add the EDK1 style GUID definition to FspNonVolatileStorageHob2.h. It is redundant with the EDK2 style definition. Please see inline.

With that change...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Friday, October 1, 2021 1:29 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH] IntelFsp2Pkg: Adopt FSP 2.3 specification.
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3674
> 
> Add ExtendedImageRevision in FSP_INFO_HEADER structure, also add
> FSP_NON_VOLATILE_STORAGE_HOB2 header.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  IntelFsp2Pkg/Include/Guid/FspHeaderFile.h             | 33
> ++++++++++++++++++++++++++++++++-
>  IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h | 27
> +++++++++++++++++++++++++++
>  IntelFsp2Pkg/IntelFsp2Pkg.dec                         |  1 +
>  3 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> index 3474bac1de..11e0ede65b 100644
> --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> @@ -2,7 +2,7 @@
>    Intel FSP Header File definition from Intel Firmware Support Package
> External
> 
>    Architecture Specification v2.0 and above.
> 
> 
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -44,14 +44,28 @@ typedef struct {
>    UINT8   Reserved1[2];
> 
>    ///
> 
>    /// Byte 0x0A: Indicates compliance with a revision of this specification in
> the BCD format.
> 
> +  ///            For revision v2.3 the value will be 0x23.
> 
>    ///
> 
>    UINT8   SpecVersion;
> 
>    ///
> 
>    /// Byte 0x0B: Revision of the FSP Information Header.
> 
> +  ///            The Current value for this field is 0x6.
> 
>    ///
> 
>    UINT8   HeaderRevision;
> 
>    ///
> 
>    /// Byte 0x0C: Revision of the FSP binary.
> 
> +  ///            Major.Minor.Revision.Build
> 
> +  ///            If FSP HeaderRevision is <= 5, the ImageRevision can be decoded
> as follows:
> 
> +  ///               7 : 0  - Build Number
> 
> +  ///              15 : 8  - Revision
> 
> +  ///              23 : 16 - Minor Version
> 
> +  ///              31 : 24 - Major Version
> 
> +  ///            If FSP HeaderRevision is >= 6, ImageRevision specifies the low-
> order bytes of the build number and revision
> 
> +  ///            while ExtendedImageRevision specifies the high-order bytes of
> the build number and revision.
> 
> +  ///               7 : 0  - Low Byte of Build Number
> 
> +  ///              15 : 8  - Low Byte of Revision
> 
> +  ///              23 : 16 - Minor Version
> 
> +  ///              31 : 24 - Major Version
> 
>    ///
> 
>    UINT32  ImageRevision;
> 
>    ///
> 
> @@ -116,6 +130,23 @@ typedef struct {
>    ///            If the value is set to 0x00000000, then this API is not available in
> this component.
> 
>    ///
> 
>    UINT32  FspMultiPhaseSiInitEntryOffset;
> 
> +  ///
> 
> +  /// Byte 0x4C: Extended revision of the FSP binary.
> 
> +  ///            This value is only valid if FSP HeaderRevision is >= 6.
> 
> +  ///            ExtendedImageRevision specifies the high-order byte of the
> revision and build number in the FSP binary revision.
> 
> +  ///               7 : 0 - High Byte of Build Number
> 
> +  ///              15 : 8 - High Byte of Revision
> 
> +  ///            The FSP binary build number can be decoded as follows:
> 
> +  ///            Build Number = (ExtendedImageRevision[7:0] << 8) |
> ImageRevision[7:0]
> 
> +  ///            Revision = (ExtendedImageRevision[15:8] << 8) |
> ImageRevision[15:8]
> 
> +  ///            Minor Version = ImageRevision[23:16]
> 
> +  ///            Major Version = ImageRevision[31:24]
> 
> +  ///
> 
> +  UINT16  ExtendedImageRevision;
> 
> +  ///
> 
> +  /// Byte 0x4E: Reserved4.
> 
> +  ///
> 
> +  UINT16  Reserved4;
> 
>  } FSP_INFO_HEADER;
> 
> 
> 
>  ///
> 
> diff --git a/IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h
> b/IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h
> new file mode 100644
> index 0000000000..8c07dc7e7e
> --- /dev/null
> +++ b/IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h
> @@ -0,0 +1,27 @@
> +/** @file
> 
> +  Intel FSP Non-Volatile Storage (NVS) HOB version 2 definition from
> 
> +  Intel Firmware Support Package External Architecture Specification v2.3.
> 
> +
> 
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#ifndef __FSP_NON_VOLATILE_STORAGE_HOB2_H__
> 
> +#define __FSP_NON_VOLATILE_STORAGE_HOB2_H__
> 
> +
> 
> +///
> 
> +/// The Non-Volatile Storage (NVS) HOB version 2 provides > 64KB buffer
> support.
> 
> +///
> 
> +#define FSP_NON_VOLATILE_STORAGE_HOB2_GUID \
> 
> +  { 0x4866788f, 0x6ba8, 0x47d8, { 0x83, 0x06, 0xac, 0xf7, 0x7f, 0x55,
> + 0x10, 0x46 } }

Please delete #define FSP_NON_VOLATILE_STORAGE_HOB2_GUID. This is the old EDK1 method of defining GUIDs and is redundant with the EDK2 style definition (extern EFI_GUID gFspNonVolatileStorageHob2Guid).

> 
> +
> 
> +typedef struct {
> 
> +  EFI_HOB_GUID_TYPE    GuidHob;
> 
> +  EFI_PHYSICAL_ADDRESS NvsDataPtr;
> 
> +  UINT64               NvsDataLength;
> 
> +} FSP_NON_VOLATILE_STORAGE_HOB2;
> 
> +
> 
> +extern EFI_GUID gFspNonVolatileStorageHob2Guid;
> 
> +
> 
> +#endif
> 
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dec b/IntelFsp2Pkg/IntelFsp2Pkg.dec
> index ec7b9a7702..58eac7220d 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dec
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dec
> @@ -68,6 +68,7 @@
>    # Guid define in FSP EAS
> 
>    gFspHeaderFileGuid                    = { 0x912740BE, 0x2284, 0x4734, { 0xB9, 0x71,
> 0x84, 0xB0, 0x27, 0x35, 0x3F, 0x0C } }
> 
>    gFspReservedMemoryResourceHobGuid     = { 0x69a79759, 0x1373, 0x4367,
> { 0xa6, 0xc4, 0xc7, 0xf5, 0x9e, 0xfd, 0x98, 0x6e } }
> 
> +  gFspNonVolatileStorageHob2Guid        = { 0x4866788f, 0x6ba8, 0x47d8, {
> 0x83, 0x06, 0xac, 0xf7, 0x7f, 0x55, 0x10, 0x46 } }
> 
>    gFspNonVolatileStorageHobGuid         = { 0x721acf02, 0x4d77, 0x4c2a, { 0xb3,
> 0xdc, 0x27, 0x0b, 0x7b, 0xa9, 0xe4, 0xb0 } }
> 
>    gFspBootLoaderTolumHobGuid            = { 0x73ff4f56, 0xaa8e, 0x4451, { 0xb3,
> 0x16, 0x36, 0x35, 0x36, 0x67, 0xad, 0x44 } } # FSP EAS v1.1
> 
> 
> 
> --
> 2.28.0.windows.1


  reply	other threads:[~2021-10-01 22:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01  8:28 [PATCH] IntelFsp2Pkg: Adopt FSP 2.3 specification Chiu, Chasel
2021-10-01 22:15 ` Nate DeSimone [this message]
2021-10-04  0:53   ` Chiu, Chasel
2021-10-04  1:10 ` Chiu, Chasel

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=MWHPR1101MB21605BE47197537CB42909C8CDAB9@MWHPR1101MB2160.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

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

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