Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
On 02/20/19 18:19, Doran, Mark wrote: >> -Original Message- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Wednesday, February 20, 2019 1:25 AM >> To: Ni, Ray ; Bi, Dandan >> Cc: edk2-devel@lists.01.org; Wu, Hao A ; Doran, Mark >> >> Subject: Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when >> fail to load/start boot option >> >> +Mark, for my comments on the process > > Thank you :) > >> On 02/20/19 03:21, Ni, Ray wrote: >>> Laszlo, >>> Thanks for catching this. >>> >>> GenPerformMemoryTest() in >>> MdeModulePkg\Universal\MemoryTest\GenericMemoryTestDxe\LightMemoryTest >>> .c uses the same technics as you suggested. >>> >>> I give up to propose another option: having pack(1) for the new status >> structure. >> >> I think that byte-packing EFI_RETURN_STATUS_EXTENDED_DATA in the PI-1.7 >> spec would have been viable, but we should have thought about that in >> advance. The PI and UEFI specs require natural alignment for structure >> members, unless stated otherwise. There *are* a number of examples of >> "stated otherwise" (the term that the specs use is frequently "byte-packed >> structure"). Again, we should have thought of that in advance, before PI- >> 1.7 was released, so now we can only fix the way the object is populated. > > Agreed. > >> More generally, I think this demonstrates a flaw in the standardization >> process. PIWG and USWG should only standardize existing practice; that is, >> features that have at least one functional implementation. (Not necessarily >> open source, but the interfaces should be battle-hardened >> *first*.) The restriction where we can't even propose patches for edk2 >> before the standards are updated is very counter-productive. There's >> practically zero chance for getting an actual implementation peer-reviewed >> and peer-tested before proposing the API for the standard. >> >> Personally I have suggested in the past to implement some new features as >> edk2 extensions first, and once they work, to upstream them to the >> standards. This idea is usually rejected by maintainers, because >> maintainers worry about becoming stuck with more and more edk2 extensions >> to core code (i.e., they worry about the feature proponents not making good >> on their promise to standardize). Unfortunately, the alternative (= the >> current practice) is that we standardize speculative interfaces, which then >> prove suboptimal once we implement them. > > So personally I'm a big believer in having working code for things we write > down in the standards. When EFI was first drafted that's how we did it: what > was then known as "the sample implementation" and the actual spec content > were more or less developed in parallel and the spec wasn't done until the > code was working satisfactorily. We got away from that largely because when > the UEFI Forum was formed, there was nominal interest in promoting more than > one implementation and keeping spec and code requirements separate was a part > of that. > > Recent changes, particularly inside Intel, present an opportunity for us to > revisit this issue. I'm advocating for the approach of spec-follows-code for > new Contributions that Intel will make to the open source project going > forward. I'll have more to say about this at upcoming industry events. > Sounds fantastic! Thank you! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
> -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, February 20, 2019 1:25 AM > To: Ni, Ray ; Bi, Dandan > Cc: edk2-devel@lists.01.org; Wu, Hao A ; Doran, Mark > > Subject: Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when > fail to load/start boot option > > +Mark, for my comments on the process Thank you :) > On 02/20/19 03:21, Ni, Ray wrote: > > Laszlo, > > Thanks for catching this. > > > > GenPerformMemoryTest() in > > MdeModulePkg\Universal\MemoryTest\GenericMemoryTestDxe\LightMemoryTest > > .c uses the same technics as you suggested. > > > > I give up to propose another option: having pack(1) for the new status > structure. > > I think that byte-packing EFI_RETURN_STATUS_EXTENDED_DATA in the PI-1.7 > spec would have been viable, but we should have thought about that in > advance. The PI and UEFI specs require natural alignment for structure > members, unless stated otherwise. There *are* a number of examples of > "stated otherwise" (the term that the specs use is frequently "byte-packed > structure"). Again, we should have thought of that in advance, before PI- > 1.7 was released, so now we can only fix the way the object is populated. Agreed. > More generally, I think this demonstrates a flaw in the standardization > process. PIWG and USWG should only standardize existing practice; that is, > features that have at least one functional implementation. (Not necessarily > open source, but the interfaces should be battle-hardened > *first*.) The restriction where we can't even propose patches for edk2 > before the standards are updated is very counter-productive. There's > practically zero chance for getting an actual implementation peer-reviewed > and peer-tested before proposing the API for the standard. > > Personally I have suggested in the past to implement some new features as > edk2 extensions first, and once they work, to upstream them to the > standards. This idea is usually rejected by maintainers, because > maintainers worry about becoming stuck with more and more edk2 extensions > to core code (i.e., they worry about the feature proponents not making good > on their promise to standardize). Unfortunately, the alternative (= the > current practice) is that we standardize speculative interfaces, which then > prove suboptimal once we implement them. So personally I'm a big believer in having working code for things we write down in the standards. When EFI was first drafted that's how we did it: what was then known as "the sample implementation" and the actual spec content were more or less developed in parallel and the spec wasn't done until the code was working satisfactorily. We got away from that largely because when the UEFI Forum was formed, there was nominal interest in promoting more than one implementation and keeping spec and code requirements separate was a part of that. Recent changes, particularly inside Intel, present an opportunity for us to revisit this issue. I'm advocating for the approach of spec-follows-code for new Contributions that Intel will make to the open source project going forward. I'll have more to say about this at upcoming industry events. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
+Mark, for my comments on the process On 02/20/19 03:21, Ni, Ray wrote: > Laszlo, > Thanks for catching this. > > GenPerformMemoryTest() in > MdeModulePkg\Universal\MemoryTest\GenericMemoryTestDxe\LightMemoryTest.c > uses the same technics as you suggested. > > I give up to propose another option: having pack(1) for the new status > structure. I think that byte-packing EFI_RETURN_STATUS_EXTENDED_DATA in the PI-1.7 spec would have been viable, but we should have thought about that in advance. The PI and UEFI specs require natural alignment for structure members, unless stated otherwise. There *are* a number of examples of "stated otherwise" (the term that the specs use is frequently "byte-packed structure"). Again, we should have thought of that in advance, before PI-1.7 was released, so now we can only fix the way the object is populated. More generally, I think this demonstrates a flaw in the standardization process. PIWG and USWG should only standardize existing practice; that is, features that have at least one functional implementation. (Not necessarily open source, but the interfaces should be battle-hardened *first*.) The restriction where we can't even propose patches for edk2 before the standards are updated is very counter-productive. There's practically zero chance for getting an actual implementation peer-reviewed and peer-tested before proposing the API for the standard. Personally I have suggested in the past to implement some new features as edk2 extensions first, and once they work, to upstream them to the standards. This idea is usually rejected by maintainers, because maintainers worry about becoming stuck with more and more edk2 extensions to core code (i.e., they worry about the feature proponents not making good on their promise to standardize). Unfortunately, the alternative (= the current practice) is that we standardize speculative interfaces, which then prove suboptimal once we implement them. Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
Hi Laszlo,
Thanks for catching this issue.
I am sorry that I didn't consider the alignment issue when working on this
patch.
Thanks,
Dandan
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, February 20, 2019 9:19 AM
> To: Bi, Dandan
> Cc: Wu, Hao A ; Ni, Ray ; edk2-
> de...@lists.01.org
> Subject: Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when
> fail to load/start boot option
>
> Hi Dandan,
>
> On 02/15/19 09:51, Dandan Bi wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1398
> >
> > According to PI1.7 Spec, report extended data describing an EFI_STATUS
> > return value along with EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR
> and
> > EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status code when fail to load
> or
> > start boot option image.
> >
> > Cc: Jian J Wang
> > Cc: Hao Wu
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Cc: Sean Brogan
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Dandan Bi
> > ---
> > .../Library/UefiBootManagerLib/BmBoot.c | 22 ++-
> > 1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > index 6444fb43eb..9be1633b74 100644
> > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > @@ -1818,15 +1818,20 @@ EfiBootManagerBoot (
> >FreePool (FilePath);
> > }
> >
> > if (EFI_ERROR (Status)) {
> >//
> > - // Report Status Code to indicate that the failure to load boot
> > option
> > + // Report Status Code with the failure status to indicate that
> > + the failure to load boot option
> >//
> > - REPORT_STATUS_CODE (
> > + REPORT_STATUS_CODE_EX (
> > EFI_ERROR_CODE | EFI_ERROR_MINOR,
> > -(EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
> > +(EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR),
> > +0,
> > +NULL,
> > +NULL,
> > +,
> > +sizeof (EFI_STATUS)
> > );
> >BootOption->Status = Status;
> >//
> >// Destroy the RAM disk
> >//
> > @@ -1902,15 +1907,20 @@ EfiBootManagerBoot (
> >Status = gBS->StartImage (ImageHandle, >ExitDataSize,
> >ExitData);
> >DEBUG ((DEBUG_INFO | DEBUG_LOAD, "Image Return Status = %r\n",
> Status));
> >BootOption->Status = Status;
> >if (EFI_ERROR (Status)) {
> > //
> > -// Report Status Code to indicate that boot failure
> > +// Report Status Code with the failure status to indicate that
> > + boot failure
> > //
> > -REPORT_STATUS_CODE (
> > +REPORT_STATUS_CODE_EX (
> >EFI_ERROR_CODE | EFI_ERROR_MINOR,
> > - (EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
> > + (EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED),
> > + 0,
> > + NULL,
> > + NULL,
> > + ,
> > + sizeof (EFI_STATUS)
> >);
> >}
> >PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32)
> > OptionNumber);
> >
> >//
> >
>
> Unfortunately, this patch is not good; we made a mistake here.
>
> Consider the EFI_RETURN_STATUS_EXTENDED_DATA structure, added in
> patch
> #1:
>
> > typedef struct {
> > ///
> > /// The data header identifying the data:
> > /// DataHeader.HeaderSize should be sizeof(EFI_STATUS_CODE_DATA),
> > /// DataHeader.Size should be
> sizeof(EFI_RETURN_STATUS_EXTENDED_DATA) - HeaderSize,
> > /// DataHeader.Type should be EFI_STATUS_CODE_SPECIFIC_DATA_GUID.
> > ///
> > EFI_STATUS_CODE_DATA DataHeader;
> > ///
> > /// The EFI_STATUS return value of the service or function whose failure
> triggered the
> > /// reporting of the status code (generally an error code or a debug
> > code).
> > ///
> > EFI_STATUS ReturnStatus;
> > } EFI_RETURN_STATUS_EXTENDED_DATA;
>
> According to the UEFI spec, unless specified otherwise, structure members
> are aligned naturally.
>
> And, the PI spec references the UEFI spec with regard to data types.
>
> Accordingly, when this structure is
Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
Laszlo,
Thanks for catching this.
GenPerformMemoryTest() in
MdeModulePkg\Universal\MemoryTest\GenericMemoryTestDxe\LightMemoryTest.c
uses the same technics as you suggested.
I give up to propose another option: having pack(1) for the new status
structure.
> -Original Message-
> From: Laszlo Ersek
> Sent: Wednesday, February 20, 2019 9:19 AM
> To: Bi, Dandan
> Cc: edk2-devel@lists.01.org; Wu, Hao A ; Ni, Ray
>
> Subject: Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when
> fail to load/start boot option
>
> Hi Dandan,
>
> On 02/15/19 09:51, Dandan Bi wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1398
> >
> > According to PI1.7 Spec, report extended data describing an EFI_STATUS
> > return value along with EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR
> and
> > EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status code when fail to load
> or
> > start boot option image.
> >
> > Cc: Jian J Wang
> > Cc: Hao Wu
> > Cc: Ruiyu Ni
> > Cc: Laszlo Ersek
> > Cc: Sean Brogan
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Dandan Bi
> > ---
> > .../Library/UefiBootManagerLib/BmBoot.c | 22 ++-
> > 1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > index 6444fb43eb..9be1633b74 100644
> > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > @@ -1818,15 +1818,20 @@ EfiBootManagerBoot (
> >FreePool (FilePath);
> > }
> >
> > if (EFI_ERROR (Status)) {
> >//
> > - // Report Status Code to indicate that the failure to load boot
> > option
> > + // Report Status Code with the failure status to indicate that
> > + the failure to load boot option
> >//
> > - REPORT_STATUS_CODE (
> > + REPORT_STATUS_CODE_EX (
> > EFI_ERROR_CODE | EFI_ERROR_MINOR,
> > -(EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
> > +(EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR),
> > +0,
> > +NULL,
> > +NULL,
> > +,
> > +sizeof (EFI_STATUS)
> > );
> >BootOption->Status = Status;
> >//
> >// Destroy the RAM disk
> >//
> > @@ -1902,15 +1907,20 @@ EfiBootManagerBoot (
> >Status = gBS->StartImage (ImageHandle, >ExitDataSize,
> >ExitData);
> >DEBUG ((DEBUG_INFO | DEBUG_LOAD, "Image Return Status = %r\n",
> Status));
> >BootOption->Status = Status;
> >if (EFI_ERROR (Status)) {
> > //
> > -// Report Status Code to indicate that boot failure
> > +// Report Status Code with the failure status to indicate that
> > + boot failure
> > //
> > -REPORT_STATUS_CODE (
> > +REPORT_STATUS_CODE_EX (
> >EFI_ERROR_CODE | EFI_ERROR_MINOR,
> > - (EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
> > + (EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED),
> > + 0,
> > + NULL,
> > + NULL,
> > + ,
> > + sizeof (EFI_STATUS)
> >);
> >}
> >PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32)
> > OptionNumber);
> >
> >//
> >
>
> Unfortunately, this patch is not good; we made a mistake here.
>
> Consider the EFI_RETURN_STATUS_EXTENDED_DATA structure, added in
> patch
> #1:
>
> > typedef struct {
> > ///
> > /// The data header identifying the data:
> > /// DataHeader.HeaderSize should be sizeof(EFI_STATUS_CODE_DATA),
> > /// DataHeader.Size should be
> sizeof(EFI_RETURN_STATUS_EXTENDED_DATA) - HeaderSize,
> > /// DataHeader.Type should be EFI_STATUS_CODE_SPECIFIC_DATA_GUID.
> > ///
> > EFI_STATUS_CODE_DATA DataHeader;
> > ///
> > /// The EFI_STATUS return value of the service or function whose failure
> triggered the
> > /// reporting of the status code (generally an error code or a debug
> > code).
> > ///
> > EFI_STATUS ReturnStatus;
> > } EFI_RETURN_STATUS_EXTENDED_DATA;
>
> According to the UEFI spec, unless specified otherwise, structure members
> are aligned naturally.
>
> And, the PI spec references the UEFI spec with regard to data types.
Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
On 02/20/19 02:19, Laszlo Ersek wrote: > Hi Dandan, > > On 02/15/19 09:51, Dandan Bi wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1398 >> >> According to PI1.7 Spec, report extended data describing an >> EFI_STATUS return value along with >> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and >> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status code >> when fail to load or start boot option image. > Unfortunately, this patch is not good; we made a mistake here. > I'll report the same in a TianoCore BZ, and will try to submit a patch > as well. https://bugzilla.tianocore.org/show_bug.cgi?id=1539 Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
Hi Dandan,
On 02/15/19 09:51, Dandan Bi wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1398
>
> According to PI1.7 Spec, report extended data describing an
> EFI_STATUS return value along with
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR and
> EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED status code
> when fail to load or start boot option image.
>
> Cc: Jian J Wang
> Cc: Hao Wu
> Cc: Ruiyu Ni
> Cc: Laszlo Ersek
> Cc: Sean Brogan
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi
> ---
> .../Library/UefiBootManagerLib/BmBoot.c | 22 ++-
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 6444fb43eb..9be1633b74 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1818,15 +1818,20 @@ EfiBootManagerBoot (
>FreePool (FilePath);
> }
>
> if (EFI_ERROR (Status)) {
>//
> - // Report Status Code to indicate that the failure to load boot option
> + // Report Status Code with the failure status to indicate that the
> failure to load boot option
>//
> - REPORT_STATUS_CODE (
> + REPORT_STATUS_CODE_EX (
> EFI_ERROR_CODE | EFI_ERROR_MINOR,
> -(EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
> +(EFI_SOFTWARE_DXE_BS_DRIVER |
> EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR),
> +0,
> +NULL,
> +NULL,
> +,
> +sizeof (EFI_STATUS)
> );
>BootOption->Status = Status;
>//
>// Destroy the RAM disk
>//
> @@ -1902,15 +1907,20 @@ EfiBootManagerBoot (
>Status = gBS->StartImage (ImageHandle, >ExitDataSize,
> >ExitData);
>DEBUG ((DEBUG_INFO | DEBUG_LOAD, "Image Return Status = %r\n", Status));
>BootOption->Status = Status;
>if (EFI_ERROR (Status)) {
> //
> -// Report Status Code to indicate that boot failure
> +// Report Status Code with the failure status to indicate that boot
> failure
> //
> -REPORT_STATUS_CODE (
> +REPORT_STATUS_CODE_EX (
>EFI_ERROR_CODE | EFI_ERROR_MINOR,
> - (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
> + (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED),
> + 0,
> + NULL,
> + NULL,
> + ,
> + sizeof (EFI_STATUS)
>);
>}
>PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);
>
>//
>
Unfortunately, this patch is not good; we made a mistake here.
Consider the EFI_RETURN_STATUS_EXTENDED_DATA structure, added in patch
#1:
> typedef struct {
> ///
> /// The data header identifying the data:
> /// DataHeader.HeaderSize should be sizeof(EFI_STATUS_CODE_DATA),
> /// DataHeader.Size should be sizeof(EFI_RETURN_STATUS_EXTENDED_DATA) -
> HeaderSize,
> /// DataHeader.Type should be EFI_STATUS_CODE_SPECIFIC_DATA_GUID.
> ///
> EFI_STATUS_CODE_DATA DataHeader;
> ///
> /// The EFI_STATUS return value of the service or function whose failure
> triggered the
> /// reporting of the status code (generally an error code or a debug code).
> ///
> EFI_STATUS ReturnStatus;
> } EFI_RETURN_STATUS_EXTENDED_DATA;
According to the UEFI spec, unless specified otherwise, structure
members are aligned naturally.
And, the PI spec references the UEFI spec with regard to data types.
Accordingly, when this structure is built for X64, the size of this
structure is 32 bytes, and the offset of ReturnStatus is 24. There is a
4-byte padding between DataHeader (which is 20 bytes in size) and the
ReturnStatus field. DataHeader has type
> typedef struct {
> ///
> /// The size of the structure. This is specified to enable future expansion.
> ///
> UINT16HeaderSize;
> ///
> /// The size of the data in bytes. This does not include the size of the
> header structure.
> ///
> UINT16Size;
> ///
> /// The GUID defining the type of the data.
> ///
> EFI_GUID Type;
> } EFI_STATUS_CODE_DATA;
which extends to 20 bytes.
I'm working on patches that capture / process
EFI_RETURN_STATUS_EXTENDED_DATA. The fields I'm seeing in DataHeader are
(on X64):
- HeaderSize = 0x14 (20 decimal)
- Size = 0x8,
- Type = {
Data1 = 0x335984bd,
Data2 = 0xe805,
Data3 = 0x409a,
Data4 = {0xb8, 0xf8, 0xd2, 0x7e, 0xce, 0x5f, 0xf7, 0xa6}
}
The "DataHeader.Size" field is incorrect. It should be 12 (that is,
32-20), according to the documentation:
> /// DataHeader.Size should be sizeof(EFI_RETURN_STATUS_EXTENDED_DATA) -
> HeaderSize,
I think in the code above, we should use a temporary
EFI_RETURN_STATUS_EXTENDED_DATA structure, zero it out, then set the
ReturnStatus field in it. Finally, call the REPORT_STATUS_CODE_EX ()
macro with the trailing portion of this temporary object.
I'll report the
