All,

There will be one topic in this week’s open design meeting discussing about storage/configuration purge design. (In case you think the meeting is cancelled as usually.)

Sunny will present and I invited Rothman Michael to help review the proposal. Michael is the expert of storage and HII.

Dandan, Hao and Eric, since you are the HII/Storage module maintainers. That would be great if you could join to provide the valuable thoughts.

NOTE:

There is one minor change: we need to use Webex for the meeting. I suggest you download Webex (www.webex.com/download) and try running it before the meeting.

Thanks,

Ray

Hi Ray,

Yes, I have tested the following:

1. SECURE_BOOT_ENABLE=true

• Key Enrollment (PK, KEK, db) via custom mode
• Execution of unit test shell application (signed one works okay, unsigned gives an Access denied)

2. SECURE_BOOT_ENABLE=false (default case)

• Secure Boot Configuration menu is not visible (Same as existing default case)
• Execution of Unit Test Application (Signed/Unsigned both works okay)

I am planning to post the script in BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2949 in a day or too.

The script generates the full key hierarchy that makes it easy to test this patch.

The patch in BZ requires modifications as per Mike’s comment, so, you can skip the patches in BZ for now.

Regards,

Divneil

Divneil,

Just want to double confirm: did you test the secure boot and non-secure boot?

Thanks,

Ray

Divneil,

Just want to double confirm: did you test the secure boot and non-secure boot?

Thanks,

Ray

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Thursday, September 17, 2020 2:32 PM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
gaoliming@...; Dong, Guo <guo.dong@...>
Cc: marcello.bauer@...; Kinney, Michael D
<michael.d.kinney@...>; 'Leif Lindholm (Nuvia address)'
<leif@...>; Doran, Mark <mark.doran@...>; 'Andrew Fish'
<afish@...>; Guptha, Soumya K <soumya.k.guptha@...>
Subject: Re: [edk2-devel] more development process failure [was:
UefiPayloadPkg: Runtime MMCONF]

Jiewen,

On 09/17/20 04:31, Yao, Jiewen wrote:

Hi Laszlo, Liming, and Mike

I appreciate your effort to setup rule and document this *complex* EDK II

Development Process.

I am thinking if we can have a way (a tool) to mandate these process and

check if there is any violation. If people makes mistake, he/she knows he/she is
making mistake and can correct immediately, instead of letting mistake happens
and getting blame later. In such way, we can prevent issue from happening.

We have old maintainer leaving, new maintainers joining. That is the reality.

We can have training for everyone. But we are still human. There are many bugs
need to be fixed in the code. How can we expect a perfect process that
everyone follows strictly without any violation?

If we only have few violation, it is OK to stay with it.
But if we continuously have violation, we need retrospect to ask, *why*? Why

there is such a process to cause so many violation?

And can we do better? A simpler process? A better tool?

while I agree that the current process is not really simple, I'd like to
point out some things:

- The current complexity exists because we are in a transition period,
and so we get to deal with both the workflow we're leaving (= the
mailing list based review) and the system we're adopting (= github).
This should not last forever. I don't know the exact schedule though.

- I think that lack of attention to detail (on the human side) takes a
relatively large chunk of the blame. The process at the moment is not
simple, but it's exercised every day, every week by some people, so if
somebody *wants*, they can get it right by following examples. Look at
recent patch series threads that have been merged, recent BZs that have
been closed, recent PRs that have been opened and merged.

It's a fallacy that adopting a 100% github.com-native patch review
workflow will solve all of these issues. There is no replacement for
human discipline and attention to detail. In the current process, I
*regularly* find pull requests (personal builds or maintainer push
attempts) on github.com that fail CI (or merging due to conflicts) and
then the submitter never bothers to close or refresh them. I have
cleaned up (closed) a *multitude* of such PRs.

I also feel sorry that Laszlo need check by his eye on every PR and catch the

violation for us. And I also feel sorry to blame some people who is contributing
his/her time to help to maintain the code, review the code, check in the code.

We both feel frustrated. We are all coming her to enable new features or fix

bugs to make EDKII better.

I would like to ask: Is that technically possible to enhance the CI to catch that

earlier, as Laszlo point out:

1) Add patch 0 to PR - can we let CI reject empty description PR?

It won't help.

See the following bug report:

https://bugzilla.tianocore.org/show_bug.cgi?id=2963#c0

While it is technically not empty (the string in comment#0 has nonzero
length), it's practically *devoid of information*.

People that are annoyed that they are required to write sensible
summaries for patch sets and bug reports, will do anything and
everything to wiggle out of that requirement. They will create
single-sentence PR descriptions, which will allow them to pass the CI
check. And the community will be *worse off*, because we will have
complicated our CI logic, but the resultant historical records will be
just as useless.

2) Send email - can we let CI send email automatically? Or remind us to send

email?

github.com *already* sends an email notification when a PR undergoes a
state change; that is, when it is merged, or else CI fails. The email is
*already* there, people just have to *act* upon it -- run a local "git
pull" again, see what the new commit range is, and send a response to
the original thread.

3) update Bugzilla - can we let CI update Bugzilla automatically? Or remind us

to update bugzilla?

Automatically closing tickets is not implemented between github.com and
Bugzilla. It is implemented within github.com (merging a PR can
auto-close issue tracker tickets, if you format the commit message
corectly).

However, auto-closing is *wrong*. It occurs that multiple patch series
relate to a single ticket. In such cases, it's possible that 10+ patches
are merged for a single ticket, and the ticket should *still* not be
closed, because more patches (for the same ticket) are necessary. Only a
human can tell when the fix or the feature is considered complete
(according to their knowledge at that point in time).

4) Unicode char - can we add check in patchchecker, to reject predefined

format violation?

There are many-many classes of unicode code points. It's not easy to
express "accept U+003A for punctuation, but do not accept U+FF1A".

It's easy to express "accept 7-bit ASCII only", but I think some people
might take issue with that, because then their names could not be placed
in subject lines in native form.

I know the new tool/CI cannot be built in one day. And we do improvement

step by step.

The *real* problem is with the attitude. If a developer cares *only*
until their patches are merged, then no tooling will *ever* fix this
issue. People need to start caring about the afterlife of their work.
When you throw a party, or join one, you stay around for the cleanup
afterwards, don't you?

When you call a contractor to fix something in or around the house, do
you expect them to clean up when they're done, or are you happy cleaning
after them?


The exact same bad attitude is the reason that

- we have botched error paths in programming languages like C,

- we have programming languages and libraries that attempt (and *fail*)
to clean up resources on errors, "on behalf" of the programmer -- I'm
referring to exceptions and destructors in C++, for example.

Both of these are symptoms that people *refuse* to deal with the
"boring" aspects of the job.


Just accept that the party isn't finished until the house and the garden
are tidied up, and the furniture is restored to original order.

Laszlo

SECURE_BOOT_ENABLE feature flag is introduced to enable Secure Boot.

The following gets enabled with this patch:

o Secure Boot Menu in "Device Manager" for enrolling keys

o Storage space for Authenticated Variables

o Authenticated execution of 3rd party images

Signed-off-by: Divneil Rai Wadhawan <divneil.r.wadhawan@...>

---

EmulatorPkg/EmulatorPkg.dsc | 37 +++++++++++++++++++++++++++++++++++--

EmulatorPkg/EmulatorPkg.fdf | 14 ++++++++++++++

2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc

index 86a6271735..c6e25c745e 100644

--- a/EmulatorPkg/EmulatorPkg.dsc

+++ b/EmulatorPkg/EmulatorPkg.dsc

@@ -32,6 +32,7 @@

   DEFINE NETWORK_TLS_ENABLE       = FALSE

   DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE

   DEFINE NETWORK_ISCSI_ENABLE     = FALSE

+  DEFINE SECURE_BOOT_ENABLE       = FALSE

 [SkuIds]

   0|DEFAULT

@@ -106,12 +107,20 @@

   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf

   CpuExceptionHandlerLib|MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.inf

   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf

-  AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf

   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf

   SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf

   ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf

   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf

+!if $(SECURE_BOOT_ENABLE) == TRUE

+  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf

+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf

+  PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf

+  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf

+!else

+  AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf

+!endif

+

[LibraryClasses.common.SEC]

   PeiServicesLib|EmulatorPkg/Library/SecPeiServicesLib/SecPeiServicesLib.inf

   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf

@@ -162,6 +171,16 @@

   TimerLib|EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.inf

  EmuThunkLib|EmulatorPkg/Library/DxeEmuLib/DxeEmuLib.inf

+[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION]

+!if $(SECURE_BOOT_ENABLE) == TRUE

+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf

+!endif

+

+[LibraryClasses.common.DXE_RUNTIME_DRIVER]

+!if $(SECURE_BOOT_ENABLE) == TRUE

+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf

+!endif

+

[LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.UEFI_APPLICATION]

   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf

   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf

@@ -190,6 +209,10 @@

   gEmulatorPkgTokenSpaceGuid.PcdEmuFirmwareFdSize|0x002a0000

   gEmulatorPkgTokenSpaceGuid.PcdEmuFirmwareBlockSize|0x10000

   gEmulatorPkgTokenSpaceGuid.PcdEmuFirmwareVolume|L"../FV/FV_RECOVERY.fd"

+!if $(SECURE_BOOT_ENABLE) == TRUE

+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800

+  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE

+!endif

   gEmulatorPkgTokenSpaceGuid.PcdEmuMemorySize|L"64!64"

@@ -306,7 +329,14 @@

   EmulatorPkg/ResetRuntimeDxe/Reset.inf

   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf

   EmulatorPkg/FvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf

-  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf

+

+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {

+    <LibraryClasses>

+!if $(SECURE_BOOT_ENABLE) == TRUE

+      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf

+!endif

+  }

+

   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

   MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf

   EmulatorPkg/EmuThunkDxe/EmuThunk.inf

@@ -315,6 +345,9 @@

   EmulatorPkg/PlatformSmbiosDxe/PlatformSmbiosDxe.inf

   EmulatorPkg/TimerDxe/Timer.inf

+!if $(SECURE_BOOT_ENABLE) == TRUE

+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf

+!endif

   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {

     <LibraryClasses>

diff --git a/EmulatorPkg/EmulatorPkg.fdf b/EmulatorPkg/EmulatorPkg.fdf

index 295f6f1db8..b256aa9397 100644

--- a/EmulatorPkg/EmulatorPkg.fdf

+++ b/EmulatorPkg/EmulatorPkg.fdf

@@ -46,10 +46,17 @@ DATA = {

   # Blockmap[1]: End

   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,

   ## This is the VARIABLE_STORE_HEADER

+!if $(SECURE_BOOT_ENABLE) == FALSE

   #Signature: gEfiVariableGuid =

   #  { 0xddcf3616, 0x3275, 0x4164, { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}

   0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,

   0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,

+!else

+  # Signature: gEfiAuthenticatedVariableGuid =

+  #  { 0xaaf32c78, 0x947b, 0x439a, { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}

+  0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,

+  0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,

+!endif

   #Size: 0xc000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0xBFB8

   # This can speed up the Variable Dispatch a bit.

   0xB8, 0xBF, 0x00, 0x00,

@@ -186,6 +193,13 @@ INF  RuleOverride = UI MdeModulePkg/Application/UiApp/UiApp.inf

INF  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf

INF  MdeModulePkg/Universal/DriverSampleDxe/DriverSampleDxe.inf

+#

+# Secure Boot Key Enroll

+#

+!if $(SECURE_BOOT_ENABLE) == TRUE

+INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf

+!endif

+

#

# Network stack drivers

#

--

2.24.1.windows.2

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Thursday, September 17, 2020 9:49 AM
To: devel@edk2.groups.io; lersek@...; Dong, Guo
<guo.dong@...>
Cc: marcello.bauer@...; Kinney, Michael D
<michael.d.kinney@...>; 'Leif Lindholm (Nuvia address)'
<leif@...>; Doran, Mark <mark.doran@...>; 'Andrew Fish'
<afish@...>; Guptha, Soumya K <soumya.k.guptha@...>
Subject: 回复: [edk2-devel] more development process failure [was:
UefiPayloadPkg: Runtime MMCONF]

Guo:
On pull request, https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
Development-Process#the-maintainer-process-for-the-edk-ii-project section 7
gives the requirement.
If <new-integration-branch> is a patch series, then copy the patch #0 summary
into the pull request description.

Laszlo:
I think we can enhance wiki page
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-
Process#the-maintainer-process-for-the-edk-ii-project to add another step to
reply the patch mail with the merged pull request or commit after PR is merged.

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+65339+4905953+8761045@groups.io
<bounce+27952+65339+4905953+8761045@groups.io> 代表 Laszlo Ersek
发送时间: 2020年9月17日 2:14
收件人: Dong, Guo <guo.dong@...>; devel@edk2.groups.io
抄送: marcello.bauer@...; Kinney, Michael D
<michael.d.kinney@...>; Leif Lindholm (Nuvia address)
<leif@...>; Doran, Mark <mark.doran@...>; Andrew Fish
<afish@...>; Guptha, Soumya K <soumya.k.guptha@...>
主题: Re: [edk2-devel] more development process failure [was:
UefiPayloadPkg: Runtime MMCONF]

On 09/16/20 19:30, Dong, Guo wrote:

Hi Laszlo,

The patchset includes 3 patches, and all of them had been reviewed by

package owners.

The patch submitter has a pull request

https://github.com/tianocore/edk2/pull/885, I rebased the patch to latest
master, and merged it by adding reviewed-by found from emails.

I also make sure it passed all the checks before I put "push" button there.

then retrigger a new build with "push" button.

I am not sure what is missing. If there is any other requirements, should

they be captured during code review or tool check?

- The description field of <https://github.com/tianocore/edk2/pull/932/>
is empty. It's difficult to tell where the patches come from -- where
they were posted and reviewed. A copy of the cover letter should have
been included here, plus preferably a link to the v5 mailing list thread
(the one that got merged in the end).

- It was not confirmed in the v5 mailing list thread that the series had
been merged. The confirmation should have included at least one of: (a)
the github PR link, (b) the git commit range. (Preferably: both.)

It's not the eventual git commits that I'm complaining about, but the
lack of communication with the community, and the lack of record for
posterity.

Myself, I used to consider github PRs a means merely for replacing our
earlier direct "git push" commands -- with a CI build + mergify. So, as
a maintainer, I would myself queue up several patch sets in a single
"batch" PR, add some links to BZs and the mailing list, and let it fly.
But then Mike told me this was really wrong, and we should clearly
associate any given PR with a specific patch set on the list.

This meant an *immense* workload increase for me, in particular because
I tend to merge patch sets for *other* people and subsystems too (after
they pass review), that is, for such subsystems that I do not
co-maintain. In particular during the feature freeze periods.

So what really rubs me the wrong way is that, if I am expected to keep
all of this meta-data nice and tidy, why aren't some other maintainers?
It's a double standard.

I can live with either *all of us* ignoring PR tidiness, or *all of us*
doing our best to keep everything nicely cross-referenced.

But right now I spend significant time and effort on keeping
communication and records complete and clean in *all three of* bugzilla,
github, and mailing list, whereas a good subset of the maintainers
couldn't care less in *either* of those communication channels.

For your reference, here's a random PR I submitted and merged for others:

https://github.com/tianocore/edk2/pull/904

Observe in PR#904:

- title carries cover letter subject
- description carries cover letter body
- description has a pointer to the BZ, and a link to the cover letter in
the mailing list archive (two links in fact, in different archives)

And then here's my report back on the list:

https://edk2.groups.io/g/devel/message/64644

And my BZ comment to the same effect (also closing the BZ as
RESOLVED|FIXED):

https://bugzilla.tianocore.org/show_bug.cgi?id=2376#c9
https://edk2.groups.io/g/bugs/message/12777


I don't insist on the particular information content of github PRs, as
-- at this stage -- they really are not more than just a way to set off
CI, before pushing/merging a series.

What I do insist on is that all of us maintainers (people with
permission to set the "push" label) be subject to the same expectations
when it comes to creating pull requests.

(Please note also that I absolutely don't need a BZ for every
contribution. My request is only that *if* there is a BZ, then handle it
thoroughly.)

Laszlo

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Wednesday, September 16, 2020 1:57 AM
To: Dong, Guo <guo.dong@...>
Cc: devel@edk2.groups.io; marcello.bauer@...; Kinney,

Michael D

<michael.d.kinney@...>; Leif Lindholm (Nuvia address)
<leif@...>; Doran, Mark <mark.doran@...>; Andrew Fish
<afish@...>; Guptha, Soumya K <soumya.k.guptha@...>
Subject: [edk2-devel] more development process failure [was:

UefiPayloadPkg:

Runtime MMCONF]

Guo,

On 08/18/20 10:24, Marcello Sylvester Bauer wrote:

Support arbitrary platforms with different or even no MMCONF space.
Fixes crash on platforms not exposing 256 buses.

Tested on:
* AMD Stoney Ridge

Branch: https://github.com/9elements/edk2-1/tree/UefiPayloadPkg-

MMCONF

PR: https://github.com/tianocore/edk2/pull/885

v5:
* MdePkg
- support variable size MMCONF in all PciExpressLibs
- use (UINTX)-1 as return values for invalid Pci addresses

Okay, so we got more of the same development process violations here, as
I've just reported at <https://edk2.groups.io/g/devel/message/65313>.

See this new pull request:

https://github.com/tianocore/edk2/pull/932/

"No description provided."

You should be embarrassed.

Laszlo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vladimir
Olovyannikov via groups.io
Sent: Wednesday, September 16, 2020 2:04 AM
To: devel@edk2.groups.io
Cc: Vladimir Olovyannikov <vladimir.olovyannikov@...>; Samer El-
Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Laszlo Ersek
<lersek@...>; Gao, Zhichao <zhichao.gao@...>; Maciej Rabeda
<maciej.rabeda@...>; Wu, Jiaxin <jiaxin.wu@...>; Fu, Siyuan
<siyuan.fu@...>; Ni, Ray <ray.ni@...>; Gao, Liming
<liming.gao@...>; Nd <nd@...>
Subject: [edk2-devel] [PATCH v12 1/1] ShellPkg/DynamicCommand: add
HttpDynamicCommand

Introduce an http client utilizing EDK2 HTTP protocol, to
allow fast image downloading from http/https servers.
HTTP download speed is usually faster than tftp.
The client is based on the same approach as tftp dynamic command, and
uses the same UEFI Shell command line parameters. This makes it easy
integrating http into existing UEFI Shell scripts.
Note that to enable HTTP download, feature Pcd
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
be set to TRUE.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860

Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@...>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Zhichao Gao <zhichao.gao@...>
Cc: Maciej Rabeda <maciej.rabeda@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Siyuan Fu <siyuan.fu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Liming Gao <liming.gao@...>
Cc: Nd <nd@...>
---
ShellPkg/ShellPkg.dec | 1 +
ShellPkg/ShellPkg.dsc | 5 +
.../HttpDynamicCommand/HttpApp.inf | 58 +
.../HttpDynamicCommand/HttpDynamicCommand.inf | 63 +
.../DynamicCommand/HttpDynamicCommand/Http.h | 90 +
ShellPkg/Include/Guid/ShellLibHiiGuid.h | 5 +
.../DynamicCommand/HttpDynamicCommand/Http.c | 1843
+++++++++++++++++
.../HttpDynamicCommand/HttpApp.c | 61 +
.../HttpDynamicCommand/HttpDynamicCommand.c | 137 ++
.../HttpDynamicCommand/Http.uni | 117 ++
10 files changed, 2380 insertions(+)
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni

diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
index d0843d338126..7b2d1230bd2c 100644
--- a/ShellPkg/ShellPkg.dec
+++ b/ShellPkg/ShellPkg.dec
@@ -53,6 +53,7 @@ [Guids]
gShellNetwork1HiiGuid = {0xf3d301bb, 0xf4a5, 0x45a8, {0xb0, 0xb7, 0xfa,
0x99, 0x9c, 0x62, 0x37, 0xae}}
gShellNetwork2HiiGuid = {0x174b2b5, 0xf505, 0x4b12, {0xaa, 0x60, 0x59,
0xdf, 0xf8, 0xd6, 0xea, 0x37}}
gShellTftpHiiGuid = {0x738a9314, 0x82c1, 0x4592, {0x8f, 0xf7, 0xc1,
0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
+ gShellHttpHiiGuid = {0x390f84b3, 0x221c, 0x4d9e, {0xb5, 0x06, 0x6d,
0xb9, 0x42, 0x3e, 0x0a, 0x7e}}
gShellBcfgHiiGuid = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 0xb2, 0xeb,
0x12, 0xda, 0xb4, 0xa2, 0xb6}}
gShellAcpiViewHiiGuid = {0xda8ccdf4, 0xed8f, 0x4ffc, {0xb5, 0xef, 0x2e,
0xf5, 0x5e, 0x24, 0x93, 0x2a}}
# FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
index 86e9f1e0040d..c42bc9464a0f 100644
--- a/ShellPkg/ShellPkg.dsc
+++ b/ShellPkg/ShellPkg.dsc
@@ -139,6 +139,11 @@ [Components]
gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
}
ShellPkg/DynamicCommand/TftpDynamicCommand/TftpApp.inf
+
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
+ <PcdsFixedAtBuild>
+ gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+ }
+ ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf {
<PcdsFixedAtBuild>
gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
new file mode 100644
index 000000000000..d08d47fb37d5
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
@@ -0,0 +1,58 @@
+## @file
+# Provides Shell 'http' standalone application.
+#
+# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved. <BR>
+# Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2020, Broadcom. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010006
+ BASE_NAME = http
+ FILE_GUID = 56B00FB7-91D2-869B-CE5C-26CD1A89C73C
+ MODULE_TYPE = UEFI_APPLICATION
+ VERSION_STRING = 1.0
+ ENTRY_POINT = HttpAppInitialize
+#
+# This flag specifies whether HII resource section is generated into PE image.
+#
+ UEFI_HII_RESOURCE_SECTION = TRUE
+
+[Sources.common]
+ Http.c
+ HttpApp.c
+ Http.h
+ Http.uni
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ NetworkPkg/NetworkPkg.dec
+ ShellPkg/ShellPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ FileHandleLib
+ HiiLib
+ HttpLib
+ MemoryAllocationLib
+ NetLib
+ ShellLib
+ UefiApplicationEntryPoint
+ UefiBootServicesTableLib
+ UefiHiiServicesLib
+ UefiLib
+ UefiRuntimeServicesTableLib
+
+[Protocols]
+ gEfiHiiPackageListProtocolGuid ## CONSUMES
+ gEfiHttpProtocolGuid ## CONSUMES
+ gEfiHttpServiceBindingProtocolGuid ## CONSUMES
+ gEfiManagedNetworkServiceBindingProtocolGuid ## CONSUMES
diff --git
a/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.in
f
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.in
f
new file mode 100644
index 000000000000..5d46ee2384d5
--- /dev/null
+++
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.in
f
@@ -0,0 +1,63 @@
+## @file
+# Provides Shell 'http' dynamic command.
+#
+# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved. <BR>
+# Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2020, Broadcom. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010006
+ BASE_NAME = httpDynamicCommand
+ FILE_GUID = 19618BCE-55AE-09C6-37E9-4CE04084C7A1
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = HttpCommandInitialize
+ UNLOAD_IMAGE = HttpUnload
+#
+# This flag specifies whether HII resource section is generated into PE image.
+#
+ UEFI_HII_RESOURCE_SECTION = TRUE
+
+[Sources.common]
+ Http.c
+ HttpDynamicCommand.c
+ Http.h
+ Http.uni
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ NetworkPkg/NetworkPkg.dec
+ ShellPkg/ShellPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ FileHandleLib
+ HiiLib
+ HttpLib
+ MemoryAllocationLib
+ NetLib
+ ShellLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+ UefiHiiServicesLib
+ UefiLib
+ UefiRuntimeServicesTableLib
+
+[Protocols]
+ gEfiHiiPackageListProtocolGuid ## CONSUMES
+ gEfiHttpProtocolGuid ## CONSUMES
+ gEfiHttpServiceBindingProtocolGuid ## CONSUMES
+ gEfiManagedNetworkServiceBindingProtocolGuid ## CONSUMES
+ gEfiShellDynamicCommandProtocolGuid ## PRODUCES
+
+[DEPEX]
+ TRUE
diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
new file mode 100644
index 000000000000..c53479b823e7
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
@@ -0,0 +1,90 @@
+/** @file
+ Header file for 'http' command functions.
+
+ Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. <BR>
+ Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2020, Broadcom. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _HTTP_H_
+#define _HTTP_H_
+
+#include <Uefi.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HiiLib.h>
+#include <Library/HttpLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/NetLib.h>
+#include <Library/PrintLib.h>
+#include <Library/ShellLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiHiiServicesLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+
+#include <Protocol/HiiPackageList.h>
+#include <Protocol/HttpUtilities.h>
+#include <Protocol/ServiceBinding.h>
+
+#define HTTP_APP_NAME L"http"
+
+#define REQ_OK 0
+#define REQ_NEED_REPEAT 1
+
+// Download Flags
+#define DL_FLAG_TIME BIT0 // Show elapsed time.
+#define DL_FLAG_KEEP_BAD BIT1 // Keep files even if download failed.
+
+extern EFI_HII_HANDLE mHttpHiiHandle;
+
+typedef struct {
+ UINTN ContentDownloaded;
+ UINTN ContentLength;
+ UINTN LastReportedNbOfBytes;
+ UINTN BufferSize;
+ UINTN Status;
+ UINTN Flags;
+ UINT8 *Buffer;
+ CHAR16 *ServerAddrAndProto;
+ CHAR16 *URI;
+ EFI_HTTP_TOKEN ResponseToken;
+ EFI_HTTP_TOKEN RequestToken;
+ EFI_HTTP_PROTOCOL *Http;
+ EFI_HTTP_CONFIG_DATA HttpConfigData;
+} HTTP_DOWNLOAD_CONTEXT;
+
+/**
+ Function for 'http' command.
+
+ @param[in] ImageHandle The image handle.
+ @param[in] SystemTable The system table.
+
+ @retval SHELL_SUCCESS Command completed successfully.
+ @retval SHELL_INVALID_PARAMETER Command usage error.
+ @retval SHELL_ABORTED The user aborts the operation.
+ @retval value Unknown error.
+**/
+SHELL_STATUS
+RunHttp (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ );
+
+/**
+ Retrive HII package list from ImageHandle and publish to HII database.
+
+ @param ImageHandle The image handle of the process.
+
+ @return HII handle.
+**/
+EFI_HII_HANDLE
+InitializeHiiPackage (
+ EFI_HANDLE ImageHandle
+ );
+#endif // _HTTP_H_
diff --git a/ShellPkg/Include/Guid/ShellLibHiiGuid.h
b/ShellPkg/Include/Guid/ShellLibHiiGuid.h
index 5da9128333a4..6e328b460d8c 100644
--- a/ShellPkg/Include/Guid/ShellLibHiiGuid.h
+++ b/ShellPkg/Include/Guid/ShellLibHiiGuid.h
@@ -59,6 +59,10 @@
0x738a9314, 0x82c1, 0x4592, { 0x8f, 0xf7, 0xc1, 0xbd, 0xf1, 0xb2, 0x0e, 0xd4 }
\
}

+#define SHELL_HTTP_HII_GUID \
+ { \
+ 0x390f84b3, 0x221c, 0x4d9e, { 0xb5, 0x06, 0x6d, 0xb9, 0x42, 0x3e, 0x0a,
0x7e } \
+ }

#define SHELL_BCFG_HII_GUID \
{ \
@@ -75,6 +79,7 @@ extern EFI_GUID gShellLevel3HiiGuid;
extern EFI_GUID gShellNetwork1HiiGuid;
extern EFI_GUID gShellNetwork2HiiGuid;
extern EFI_GUID gShellTftpHiiGuid;
+extern EFI_GUID gShellHttpHiiGuid;
extern EFI_GUID gShellBcfgHiiGuid;

#endif
diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
new file mode 100644
index 000000000000..3f11c1cd84c3
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
@@ -0,0 +1,1843 @@
+/** @file
+ The implementation for the 'http' Shell command.
+
+ Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved. <BR>
+ (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+ Copyright (c) 2020, Broadcom. All rights reserved. <BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "Http.h"
+
+#define IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH 32
+
+/*
+ Constant strings and definitions related to the message
+ indicating the amount of progress in the dowloading of a HTTP file.
+*/
+
+// Number of steps in the progression slider
+#define HTTP_PROGRESS_SLIDER_STEPS \
+ ((sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) - 3)
+
+// Size in number of characters plus one (final zero) of the message to
+// indicate the progress of an HTTP download. The format is "[(progress slider:
+// 40 characters)] (nb of KBytes downloaded so far: 7 characters) Kb". There
+// are thus the number of characters in HTTP_PROGR_FRAME[] plus 11
characters
+// (2 // spaces, "Kb" and seven characters for the number of KBytes).
+#define HTTP_PROGRESS_MESSAGE_SIZE \
+ ((sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) + 12)
+
+//
+// Buffer size. Note that larger buffer does not mean better speed!
+//
+#define DEFAULT_BUF_SIZE SIZE_32KB
+#define MAX_BUF_SIZE SIZE_4MB
+
+#define MIN_PARAM_COUNT 2
+#define MAX_PARAM_COUNT 4
+#define NEED_REDIRECTION(Code) \
+ (((Code >= HTTP_STATUS_300_MULTIPLE_CHOICES) \
+ && (Code <= HTTP_STATUS_307_TEMPORARY_REDIRECT)) \
+ || (Code == HTTP_STATUS_308_PERMANENT_REDIRECT))
+
+#define CLOSE_HTTP_HANDLE(ControllerHandle,HttpChildHandle) \
+ do { \
+ if (HttpChildHandle) { \
+ CloseProtocolAndDestroyServiceChild ( \
+ ControllerHandle, \
+ &gEfiHttpServiceBindingProtocolGuid, \
+ &gEfiHttpProtocolGuid, \
+ HttpChildHandle \
+ ); \
+ HttpChildHandle = NULL; \
+ } \
+ } while (0)
+
+typedef enum {
+ HDR_HOST,
+ HDR_CONN,
+ HDR_AGENT,
+ HDR_MAX
+} HDR_TYPE;
+
+#define USER_AGENT_HDR "Mozilla/5.0 (EDK2; Linux) Gecko/20100101
Firefox/79.0"
+
+#define TIMER_MAX_TIMEOUT_S 10
+
+// File name to use when URI ends with "/"
+#define DEFAULT_HTML_FILE L"index.html"
+#define DEFAULT_HTTP_PROTO L"http"
+
+// String to delete the HTTP progress message to be able to update it :
+// (HTTP_PROGRESS_MESSAGE_SIZE-1) '\b'
+#define HTTP_PROGRESS_DEL \
+ L"\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\
+\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b"
+
+#define HTTP_KB L"\b\b\b\b\b\b\b\b\b\b"
+// Frame for the progression slider
+#define HTTP_PROGR_FRAME L"[ ]"
+
+// Improve readability by using these macros
+#define PRINT_HII(token,...) \
+ ShellPrintHiiEx (\
+ -1, -1, NULL, token, mHttpHiiHandle, __VA_ARGS__)
+
+#define PRINT_HII_APP(token,value) \
+ PRINT_HII (token, HTTP_APP_NAME, value)
+
+//
+// TimeBaseLib.h constants.
+// TODO: remove once the library gets fixed.
+//
+
+// Define EPOCH (1970-JANUARY-01) in the Julian Date representation
+#define EPOCH_JULIAN_DATE 2440588
+
+// Seconds per unit
+#define SEC_PER_MIN ((UINTN) 60)
+#define SEC_PER_HOUR ((UINTN) 3600)
+#define SEC_PER_DAY ((UINTN) 86400)
+
+
+// String descriptions for server errors
+STATIC CONST CHAR16 *ErrStatusDesc[] =
+{
+ L"400 Bad Request",
+ L"401 Unauthorized",
+ L"402 Payment required",
+ L"403 Forbidden",
+ L"404 Not Found",
+ L"405 Method not allowed",
+ L"406 Not acceptable",
+ L"407 Proxy authentication required",
+ L"408 Request time out",
+ L"409 Conflict",
+ L"410 Gone",
+ L"411 Length required",
+ L"412 Precondition failed",
+ L"413 Request entity too large",
+ L"414 Request URI to large",
+ L"415 Unsupported media type",
+ L"416 Requested range not satisfied",
+ L"417 Expectation failed",
+ L"500 Internal server error",
+ L"501 Not implemented",
+ L"502 Bad gateway",
+ L"503 Service unavailable",
+ L"504 Gateway timeout",
+ L"505 HTTP version not supported"
+};
+
+STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
+ {L"-i", TypeValue},
+ {L"-k", TypeFlag},
+ {L"-l", TypeValue},
+ {L"-m", TypeFlag},
+ {L"-s", TypeValue},
+ {L"-t", TypeValue},
+ {NULL , TypeMax}
+};
+
+// Local File Handle
+STATIC SHELL_FILE_HANDLE mFileHandle = NULL;
+
+// Path of the local file, Unicode encoded
+STATIC CONST CHAR16 *mLocalFilePath;
+
+STATIC BOOLEAN gRequestCallbackComplete = FALSE;
+STATIC BOOLEAN gResponseCallbackComplete = FALSE;
+
+STATIC BOOLEAN gHttpError;
+
+EFI_HII_HANDLE mHttpHiiHandle;
+
+// Functions declarations
+/**
+ Check and convert the UINT16 option values of the 'http' command
+
+ @param[in] ValueStr Value as an Unicode encoded string
+ @param[out] Value UINT16 value
+
+ @return TRUE The value was returned.
+ @return FALSE A parsing error occured.
+**/
+STATIC
+BOOLEAN
+StringToUint16 (
+ IN CONST CHAR16 *ValueStr,
+ OUT UINT16 *Value
+ );
+
+/**
+ Get the name of the NIC.
+
+ @param[in] ControllerHandle The network physical device handle.
+ @param[in] NicNumber The network physical device number.
+ @param[out] NicName Address where to store the NIC name.
+ The memory area has to be at least
+ IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH
+ double byte wide.
+
+ @return EFI_SUCCESS The name of the NIC was returned.
+ @return Others The creation of the child for the Managed
+ Network Service failed or the opening of
+ the Managed Network Protocol failed or
+ the operational parameters for the
+ Managed Network Protocol could not be
+ read.
+**/
+STATIC
+EFI_STATUS
+GetNicName (
+ IN EFI_HANDLE ControllerHandle,
+ IN UINTN NicNumber,
+ OUT CHAR16 *NicName
+ );
+
+/**
+ Create a child for the service identified by its service binding protocol GUID
+ and get from the child the interface of the protocol identified by its GUID.
+
+ @param[in] ControllerHandle Controller handle.
+ @param[in] ServiceBindingProtocolGuid Service binding protocol GUID of the
+ service to be created.
+ @param[in] ProtocolGuid GUID of the protocol to be open.
+ @param[out] ChildHandle Address where the handler of the
+ created child is returned. NULL is
+ returned in case of error.
+ @param[out] Interface Address where a pointer to the
+ protocol interface is returned in
+ case of success.
+
+ @return EFI_SUCCESS The child was created and the protocol opened.
+ @return Others Either the creation of the child or the opening
+ of the protocol failed.
+**/
+STATIC
+EFI_STATUS
+CreateServiceChildAndOpenProtocol (
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_GUID *ServiceBindingProtocolGuid,
+ IN EFI_GUID *ProtocolGuid,
+ OUT EFI_HANDLE *ChildHandle,
+ OUT VOID **Interface
+ );
+
+/**
+ Close the protocol identified by its GUID on the child handle of the service
+ identified by its service binding protocol GUID, then destroy the child
+ handle.
+
+ @param[in] ControllerHandle Controller handle.
+ @param[in] ServiceBindingProtocolGuid Service binding protocol GUID of the
+ service to be destroyed.
+ @param[in] ProtocolGuid GUID of the protocol to be closed.
+ @param[in] ChildHandle Handle of the child to be destroyed.
+
+**/
+STATIC
+VOID
+CloseProtocolAndDestroyServiceChild (
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_GUID *ServiceBindingProtocolGuid,
+ IN EFI_GUID *ProtocolGuid,
+ IN EFI_HANDLE ChildHandle
+ );
+
+/**
+ Worker function that download the data of a file from an HTTP server given
+ the path of the file and its size.
+
+ @param[in] Context A pointer to the download context.
+
+ @retval EFI_SUCCESS The file was downloaded.
+ @retval EFI_OUT_OF_RESOURCES A memory allocation failed.
+ @retval Others The downloading of the file
+ from the server failed.
+
+**/
+STATIC
+EFI_STATUS
+DownloadFile (
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN EFI_HANDLE ControllerHandle,
+ IN CHAR16 *NicName
+ );
+
+/**
+ Cleans off leading and trailing spaces and tabs.
+
+ @param[in] String pointer to the string to trim them off.
+**/
+STATIC
+EFI_STATUS
+TrimSpaces (
+ IN CHAR16 *String
+ )
+{
+ CHAR16 *Str;
+ UINTN Len;
+
+ ASSERT (String != NULL);
+
+ if (!String) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Str = String;
+
+ //
+ // Remove any whitespace at the beginning of the Str.
+ //
+ while (*Str == L' ' || *Str == L'\t') {
+ Str++;
+ }
+
+ //
+ // Remove any whitespace at the end of the Str.
+ //
+ do {
+ Len = StrLen (Str);
+ if (!Len || (Str[Len - 1] != L' ' && Str[Len - 1] != '\t')) {
+ break;
+ }
+
+ Str[Len - 1] = CHAR_NULL;
+ } while (Len);
+
+ CopyMem (String, Str, StrSize (Str));
+
+ return EFI_SUCCESS;
+}
+
+
+/*
+ * Callbacks for request and response.
+ * We just acknowledge that operation has completed here.
+ */
+STATIC
+VOID
+EFIAPI
+RequestCallback (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ gRequestCallbackComplete = TRUE;
+}
+
+STATIC
+VOID
+EFIAPI
+ResponseCallback (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ gResponseCallbackComplete = TRUE;
+}
+
+//
+// Set of functions from TimeBaseLib.
+// TODO: remove once TimeBaseLib gets fixed, and enabled for ShellPkg.
+//
+
+/**
+ Calculate Epoch days
+ **/
+STATIC
+UINTN
+EfiGetEpochDays (
+ IN EFI_TIME *Time
+ )
+{
+ UINTN a;
+ UINTN y;
+ UINTN m;
+ UINTN JulianDate; // Absolute Julian Date representation of the supplied Time
+ UINTN EpochDays; // Number of days elapsed since EPOCH_JULIAN_DAY
+
+ a = (14 - Time->Month) / 12 ;
+ y = Time->Year + 4800 - a;
+ m = Time->Month + (12 * a) - 3;
+
+ JulianDate = Time->Day + ((153 * m + 2) / 5) + (365 * y) + (y / 4) -
+ (y / 100) + (y / 400) - 32045;
+
+ ASSERT (JulianDate >= EPOCH_JULIAN_DATE);
+ EpochDays = JulianDate - EPOCH_JULIAN_DATE;
+
+ return EpochDays;
+}
+
+/**
+ Converts EFI_TIME to Epoch seconds
+ (elapsed since 1970 JANUARY 01, 00:00:00 UTC)
+ **/
+STATIC
+UINTN
+EFIAPI
+EfiTimeToEpoch (
+ IN EFI_TIME *Time
+ )
+{
+ UINTN EpochDays; // Number of days elapsed since EPOCH_JULIAN_DAY
+ UINTN EpochSeconds;
+
+ EpochDays = EfiGetEpochDays (Time);
+
+ EpochSeconds = (EpochDays * SEC_PER_DAY) +
+ ((UINTN)Time->Hour * SEC_PER_HOUR) +
+ (Time->Minute * SEC_PER_MIN) + Time->Second;
+
+ return EpochSeconds;
+}
+
+/**
+ Function for 'http' command.
+
+ @param[in] ImageHandle Handle to the Image (NULL if Internal).
+ @param[in] SystemTable Pointer to the System Table (NULL if Internal).
+
+ @return SHELL_SUCCESS The 'http' command completed successfully.
+ @return SHELL_ABORTED The Shell Library initialization failed.
+ @return SHELL_INVALID_PARAMETER At least one of the command's
arguments is
+ not valid.
+ @return SHELL_OUT_OF_RESOURCES A memory allocation failed.
+ @return SHELL_NOT_FOUND Network Interface Card not found.
+ @return SHELL_UNSUPPORTED Command was valid, but the server
returned
+ a status code indicating some error.
+ Examine the file requested for error body.
+
+**/
+SHELL_STATUS
+RunHttp (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+ LIST_ENTRY *CheckPackage;
+ UINTN ParamCount;
+ UINTN HandleCount;
+ UINTN NicNumber;
+ UINTN InitialSize;
+ UINTN ParamOffset;
+ UINTN StartSize;
+ CHAR16 *ProblemParam;
+ CHAR16 NicName[IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH];
+ CHAR16 *Walker1;
+ CHAR16 *VStr;
+ CONST CHAR16 *UserNicName;
+ CONST CHAR16 *ValueStr;
+ CONST CHAR16 *RemoteFilePath;
+ CONST CHAR16 *Walker;
+ EFI_HTTPv4_ACCESS_POINT IPv4Node;
+ EFI_HANDLE *Handles;
+ EFI_HANDLE ControllerHandle;
+ HTTP_DOWNLOAD_CONTEXT Context;
+ BOOLEAN NicFound;
+
+ ProblemParam = NULL;
+ RemoteFilePath = NULL;
+ NicFound = FALSE;
+ Handles = NULL;
+
+ //
+ // Initialize the Shell library (we must be in non-auto-init...)
+ //
+ ParamOffset = 0;
+ gHttpError = FALSE;
+
+ Status = ShellInitialize ();
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return SHELL_ABORTED;
+ }
+
+ ZeroMem (&Context, sizeof (Context));
+
+ //
+ // Parse the command line.
+ //
+ Status = ShellCommandLineParse (
+ ParamList,
+ &CheckPackage,
+ &ProblemParam,
+ TRUE
+ );
+ if (EFI_ERROR (Status)) {
+ if ((Status == EFI_VOLUME_CORRUPTED)
+ && (ProblemParam != NULL))
+ {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_PROBLEM), ProblemParam);
+ SHELL_FREE_NON_NULL (ProblemParam);
+ } else {
+ ASSERT (FALSE);
+ }
+
+ goto Error;
+ }
+
+ //
+ // Check the number of parameters
+ //
+ Status = EFI_INVALID_PARAMETER;
+
+ ParamCount = ShellCommandLineGetCount (CheckPackage);
+ if (ParamCount > MAX_PARAM_COUNT) {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_TOO_MANY), NULL);
+ goto Error;
+ }
+
+ if (ParamCount < MIN_PARAM_COUNT) {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_TOO_FEW), NULL);
+ goto Error;
+ }
+
+ ZeroMem (&Context.HttpConfigData, sizeof (Context.HttpConfigData));
+ ZeroMem (&IPv4Node, sizeof (IPv4Node));
+ IPv4Node.UseDefaultAddress = TRUE;
+
+ Context.HttpConfigData.HttpVersion = HttpVersion11;
+ Context.HttpConfigData.AccessPoint.IPv4Node = &IPv4Node;
+
+ //
+ // Get the host address (not necessarily IPv4 format)
+ //
+ ValueStr = ShellCommandLineGetRawValue (CheckPackage, 1);
+ if (!ValueStr) {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_PARAM_INV), ValueStr);
+ goto Error;
+ } else {
+ StartSize = 0;
+ TrimSpaces ((CHAR16 *)ValueStr);
+ if (!StrStr (ValueStr, L"://")) {
+ Context.ServerAddrAndProto = StrnCatGrow (
+ &Context.ServerAddrAndProto,
+ &StartSize,
+ DEFAULT_HTTP_PROTO,
+ StrLen (DEFAULT_HTTP_PROTO)
+ );
+ Context.ServerAddrAndProto = StrnCatGrow (
+ &Context.ServerAddrAndProto,
+ &StartSize,
+ L"://",
+ StrLen (L"://")
+ );
+ VStr = (CHAR16 *)ValueStr;
+ } else {
+ VStr = StrStr (ValueStr, L"://") + StrLen (L"://");
+ }
+
+ for (Walker1 = VStr; *Walker1; Walker1++) {
+ if (*Walker1 == L'/') {
+ break;
+ }
+ }
+
+ if (*Walker1 == L'/') {
+ ParamOffset = 1;
+ RemoteFilePath = Walker1;
+ }
+
+ Context.ServerAddrAndProto = StrnCatGrow (
+ &Context.ServerAddrAndProto,
+ &StartSize,
+ ValueStr,
+ StrLen (ValueStr) - StrLen (Walker1)
+ );
+ if (!Context.ServerAddrAndProto) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Error;
+ }
+ }
+
+ if (!RemoteFilePath) {
+ RemoteFilePath = ShellCommandLineGetRawValue (CheckPackage, 2);
+ if (!RemoteFilePath) {
+ // If no path given, assume just "/"
+ RemoteFilePath = L"/";
+ }
+ }
+
+ TrimSpaces ((CHAR16 *)RemoteFilePath);
+
+ if (ParamCount == MAX_PARAM_COUNT - ParamOffset) {
+ mLocalFilePath = ShellCommandLineGetRawValue (
+ CheckPackage,
+ MAX_PARAM_COUNT - 1 - ParamOffset
+ );
+ } else {
+ Walker = RemoteFilePath + StrLen (RemoteFilePath);
+ while ((--Walker) >= RemoteFilePath) {
+ if ((*Walker == L'\\') ||
+ (*Walker == L'/' ) ) {
+ break;
+ }
+ }
+
+ mLocalFilePath = Walker + 1;
+ }
+
+ if (!StrLen (mLocalFilePath)) {
+ mLocalFilePath = DEFAULT_HTML_FILE;
+ }
+
+ InitialSize = 0;
+ Context.URI = StrnCatGrow (
+ &Context.URI,
+ &InitialSize,
+ RemoteFilePath,
+ StrLen (RemoteFilePath)
+ );
+ if (!Context.URI) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Error;
+ }
+
+ //
+ // Get the name of the Network Interface Card to be used if any.
+ //
+ UserNicName = ShellCommandLineGetValue (CheckPackage, L"-i");
+
+ ValueStr = ShellCommandLineGetValue (CheckPackage, L"-l");
+ if ((ValueStr != NULL)
+ && (!StringToUint16 (
+ ValueStr,
+ &Context.HttpConfigData.AccessPoint.IPv4Node->LocalPort
+ )
+ ))
+ {
+ goto Error;
+ }
+
+ Context.BufferSize = DEFAULT_BUF_SIZE;
+
+ ValueStr = ShellCommandLineGetValue (CheckPackage, L"-s");
+ if (ValueStr != NULL) {
+ Context.BufferSize = ShellStrToUintn (ValueStr);
+ if (!Context.BufferSize || Context.BufferSize > MAX_BUF_SIZE) {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_PARAM_INV), ValueStr);
+ goto Error;
+ }
+ }
+
+ ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
+ if (ValueStr != NULL) {
+ Context.HttpConfigData.TimeOutMillisec = (UINT32)ShellStrToUintn
(ValueStr);
+ }
+
+ //
+ // Locate all HTTP Service Binding protocols
+ //
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEfiManagedNetworkServiceBindingProtocolGuid,
+ NULL,
+ &HandleCount,
+ &Handles
+ );
+ if (EFI_ERROR (Status) || (HandleCount == 0)) {
+ PRINT_HII (STRING_TOKEN (STR_HTTP_ERR_NO_NIC), NULL);
+ if (!EFI_ERROR (Status)) {
+ Status = EFI_NOT_FOUND;
+ }
+
+ goto Error;
+ }
+
+ Status = EFI_NOT_FOUND;
+
+ Context.Flags = 0;
+ if (ShellCommandLineGetFlag (CheckPackage, L"-m")) {
+ Context.Flags |= DL_FLAG_TIME;
+ }
+
+ if (ShellCommandLineGetFlag (CheckPackage, L"-k")) {
+ Context.Flags |= DL_FLAG_KEEP_BAD;
+ }
+
+ for (NicNumber = 0;
+ (NicNumber < HandleCount) && (Status != EFI_SUCCESS);
+ NicNumber++)
+ {
+ ControllerHandle = Handles[NicNumber];
+
+ Status = GetNicName (ControllerHandle, NicNumber, NicName);
+ if (EFI_ERROR (Status)) {
+ PRINT_HII (STRING_TOKEN (STR_HTTP_ERR_NIC_NAME), NicNumber,
Status);
+ continue;
+ }
+
+ if (UserNicName != NULL) {
+ if (StrCmp (NicName, UserNicName) != 0) {
+ Status = EFI_NOT_FOUND;
+ continue;
+ }
+
+ NicFound = TRUE;
+ }
+
+ Status = DownloadFile (&Context, ControllerHandle, NicName);
+ PRINT_HII (STRING_TOKEN (STR_GEN_CRLF), NULL);
+
+ if (EFI_ERROR (Status)) {
+ PRINT_HII (
+ STRING_TOKEN (STR_HTTP_ERR_DOWNLOAD),
+ RemoteFilePath,
+ NicName,
+ Status
+ );
+ // If a user aborted the operation, do not try another controller.
+ if (Status == EFI_ABORTED) {
+ goto Error;
+ }
+ }
+
+ if (gHttpError) {
+ //
+ // This is not related to connection, so no need to repeat with
+ // another interface.
+ //
+ break;
+ }
+ }
+
+ if ((UserNicName != NULL) && (!NicFound)) {
+ PRINT_HII (STRING_TOKEN (STR_HTTP_ERR_NIC_NOT_FOUND),
UserNicName);
+ }
+
+Error:
+ ShellCommandLineFreeVarList (CheckPackage);
+ SHELL_FREE_NON_NULL (Handles);
+ SHELL_FREE_NON_NULL (Context.ServerAddrAndProto);
+ SHELL_FREE_NON_NULL (Context.URI);
+
+ return Status & ~MAX_BIT;
+}
+
+/**
+ Check and convert the UINT16 option values of the 'http' command
+
+ @param[in] ValueStr Value as an Unicode encoded string
+ @param[out] Value UINT16 value
+
+ @return TRUE The value was returned.
+ @return FALSE A parsing error occured.
+**/
+STATIC
+BOOLEAN
+StringToUint16 (
+ IN CONST CHAR16 *ValueStr,
+ OUT UINT16 *Value
+ )
+{
+ UINTN Val;
+
+ Val = ShellStrToUintn (ValueStr);
+ if (Val > MAX_UINT16) {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_PARAM_INV), ValueStr);
+ return FALSE;
+ }
+
+ *Value = (UINT16)Val;
+ return TRUE;
+}
+
+/**
+ Get the name of the NIC.
+
+ @param[in] ControllerHandle The network physical device handle.
+ @param[in] NicNumber The network physical device number.
+ @param[out] NicName Address where to store the NIC name.
+ The memory area has to be at least
+ IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH
+ double byte wide.
+
+ @return EFI_SUCCESS The name of the NIC was returned.
+ @return Others The creation of the child for the Managed
+ Network Service failed or the opening of
+ the Managed Network Protocol failed or
+ the operational parameters for the
+ Managed Network Protocol could not be
+ read.
+**/
+STATIC
+EFI_STATUS
+GetNicName (
+ IN EFI_HANDLE ControllerHandle,
+ IN UINTN NicNumber,
+ OUT CHAR16 *NicName
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE MnpHandle;
+ EFI_MANAGED_NETWORK_PROTOCOL *Mnp;
+ EFI_SIMPLE_NETWORK_MODE SnpMode;
+
+ Status = CreateServiceChildAndOpenProtocol (
+ ControllerHandle,
+ &gEfiManagedNetworkServiceBindingProtocolGuid,
+ &gEfiManagedNetworkProtocolGuid,
+ &MnpHandle,
+ (VOID**)&Mnp
+ );
+ if (EFI_ERROR (Status)) {
+ goto Error;
+ }
+
+ Status = Mnp->GetModeData (Mnp, NULL, &SnpMode);
+ if (EFI_ERROR (Status) && (Status != EFI_NOT_STARTED)) {
+ goto Error;
+ }
+
+ UnicodeSPrint (
+ NicName,
+ IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH,
+ SnpMode.IfType == NET_IFTYPE_ETHERNET ? L"eth%d" : L"unk%d",
+ NicNumber
+ );
+
+ Status = EFI_SUCCESS;
+
+Error:
+
+ if (MnpHandle != NULL) {
+ CloseProtocolAndDestroyServiceChild (
+ ControllerHandle,
+ &gEfiManagedNetworkServiceBindingProtocolGuid,
+ &gEfiManagedNetworkProtocolGuid,
+ MnpHandle
+ );
+ }
+
+ return Status;
+}
+
+/**
+ Create a child for the service identified by its service binding protocol GUID
+ and get from the child the interface of the protocol identified by its GUID.
+
+ @param[in] ControllerHandle Controller handle.
+ @param[in] ServiceBindingProtocolGuid Service binding protocol GUID of the
+ service to be created.
+ @param[in] ProtocolGuid GUID of the protocol to be open.
+ @param[out] ChildHandle Address where the handler of the
+ created child is returned. NULL is
+ returned in case of error.
+ @param[out] Interface Address where a pointer to the
+ protocol interface is returned in
+ case of success.
+
+ @return EFI_SUCCESS The child was created and the protocol opened.
+ @return Others Either the creation of the child or the opening
+ of the protocol failed.
+**/
+STATIC
+EFI_STATUS
+CreateServiceChildAndOpenProtocol (
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_GUID *ServiceBindingProtocolGuid,
+ IN EFI_GUID *ProtocolGuid,
+ OUT EFI_HANDLE *ChildHandle,
+ OUT VOID **Interface
+ )
+{
+ EFI_STATUS Status;
+
+ *ChildHandle = NULL;
+ Status = NetLibCreateServiceChild (
+ ControllerHandle,
+ gImageHandle,
+ ServiceBindingProtocolGuid,
+ ChildHandle
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = gBS->OpenProtocol (
+ *ChildHandle,
+ ProtocolGuid,
+ Interface,
+ gImageHandle,
+ ControllerHandle,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL
+ );
+ if (EFI_ERROR (Status)) {
+ NetLibDestroyServiceChild (
+ ControllerHandle,
+ gImageHandle,
+ ServiceBindingProtocolGuid,
+ *ChildHandle
+ );
+ *ChildHandle = NULL;
+ }
+ }
+
+ return Status;
+}
+
+/**
+ Close the protocol identified by its GUID on the child handle of the service
+ identified by its service binding protocol GUID, then destroy the child
+ handle.
+
+ @param[in] ControllerHandle Controller handle.
+ @param[in] ServiceBindingProtocolGuid Service binding protocol GUID of the
+ service to be destroyed.
+ @param[in] ProtocolGuid GUID of the protocol to be closed.
+ @param[in] ChildHandle Handle of the child to be destroyed.
+
+**/
+STATIC
+VOID
+CloseProtocolAndDestroyServiceChild (
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_GUID *ServiceBindingProtocolGuid,
+ IN EFI_GUID *ProtocolGuid,
+ IN EFI_HANDLE ChildHandle
+ )
+{
+ gBS->CloseProtocol (
+ ChildHandle,
+ ProtocolGuid,
+ gImageHandle,
+ ControllerHandle
+ );
+
+ NetLibDestroyServiceChild (
+ ControllerHandle,
+ gImageHandle,
+ ServiceBindingProtocolGuid,
+ ChildHandle
+ );
+}
+
+/**
+ Wait until operation completes. Completion is indicated by
+ setting of an appropriate variable.
+
+ @param[in] Context A pointer to the HTTP download context.
+ @param[in] CallBackComplete A pointer to the callback completion
+ variable set by the callback.
+
+ @return EFI_SUCCESS Callback signalled completion.
+ @return EFI_TIMEOUT Timed out waiting for completion.
+ @return Others Error waiting for completion.
+**/
+STATIC
+EFI_STATUS
+WaitForCompletion (
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN OUT BOOLEAN *CallBackComplete
+ )
+{
+ EFI_STATUS Status;
+ EFI_EVENT WaitEvt;
+
+ Status = EFI_SUCCESS;
+
+ // Use a timer to measure timeout. Cannot use Stall here!
+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &WaitEvt
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ if (!EFI_ERROR (Status)) {
+ Status = gBS->SetTimer (
+ WaitEvt,
+ TimerRelative,
+ EFI_TIMER_PERIOD_SECONDS (TIMER_MAX_TIMEOUT_S)
+ );
+
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ while (! *CallBackComplete
+ && (!EFI_ERROR (Status))
+ && EFI_ERROR (gBS->CheckEvent (WaitEvt)))
+ {
+ Status = Context->Http->Poll (Context->Http);
+ if (!Context->ContentDownloaded
+ && CallBackComplete == &gResponseCallbackComplete)
+ {
+ //
+ // An HTTP server may just send a response redirection header.
+ // In this case, don't wait for the event as
+ // it might never happen and we waste 10s waiting.
+ // Note that at this point Response may not has been populated,
+ // so it needs to be checked first.
+ //
+ if (Context->ResponseToken.Message
+ && Context->ResponseToken.Message->Data.Response
+ && (NEED_REDIRECTION (
+ Context->ResponseToken.Message->Data.Response->StatusCode
+ )
+ ))
+ {
+ break;
+ }
+ }
+ }
+
+ gBS->SetTimer (WaitEvt, TimerCancel, 0);
+ gBS->CloseEvent (WaitEvt);
+
+ if (*CallBackComplete) {
+ return EFI_SUCCESS;
+ }
+
+ if (!EFI_ERROR (Status)) {
+ Status = EFI_TIMEOUT;
+ }
+
+ return Status;
+}
+
+/**
+ Generate and send a request to the http server.
+
+ @param[in] Context HTTP download context.
+ @param[in] DownloadUrl Fully qualified URL to be downloaded.
+
+ @return EFI_SUCCESS Request has been sent successfully.
+ @return EFI_INVALID_PARAMETER Invalid URL.
+ @return EFI_OUT_OF_RESOURCES Out of memory.
+ @return EFI_DEVICE_ERROR If HTTPS is used, this probably
+ means that TLS support either was not
+ installed or not configured.
+ @return Others Error sending the request.
+**/
+
+STATIC
+EFI_STATUS
+SendRequest (
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN CHAR16 *DownloadUrl
+ )
+{
+ EFI_HTTP_REQUEST_DATA RequestData;
+ EFI_HTTP_HEADER RequestHeader[HDR_MAX];
+ EFI_HTTP_MESSAGE RequestMessage;
+ EFI_STATUS Status;
+ CHAR16 *Host;
+ UINTN StringSize;
+
+ ZeroMem (&RequestData, sizeof (RequestData));
+ ZeroMem (&RequestHeader, sizeof (RequestHeader));
+ ZeroMem (&RequestMessage, sizeof (RequestMessage));
+ ZeroMem (&Context->RequestToken, sizeof (Context->RequestToken));
+
+ RequestHeader[HDR_HOST].FieldName = "Host";
+ RequestHeader[HDR_CONN].FieldName = "Connection";
+ RequestHeader[HDR_AGENT].FieldName = "User-Agent";
+
+ Host = (CHAR16 *)Context->ServerAddrAndProto;
+ while (*Host != CHAR_NULL && *Host != L'/') {
+ Host++;
+ }
+
+ if (*Host == CHAR_NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Get the next slash
+ //
+ Host++;
+ //
+ // And now the host name
+ //
+ Host++;
+
+ StringSize = StrLen (Host) + 1;
+ RequestHeader[HDR_HOST].FieldValue = AllocatePool (StringSize);
+ if (!RequestHeader[HDR_HOST].FieldValue) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ UnicodeStrToAsciiStrS (
+ Host,
+ RequestHeader[HDR_HOST].FieldValue,
+ StringSize
+ );
+
+ RequestHeader[HDR_CONN].FieldValue = "close";
+ RequestHeader[HDR_AGENT].FieldValue = USER_AGENT_HDR;
+ RequestMessage.HeaderCount = HDR_MAX;
+
+ RequestData.Method = HttpMethodGet;
+ RequestData.Url = DownloadUrl;
+
+ RequestMessage.Data.Request = &RequestData;
+ RequestMessage.Headers = RequestHeader;
+ RequestMessage.BodyLength = 0;
+ RequestMessage.Body = NULL;
+ Context->RequestToken.Event = NULL;
+
+ //
+ // Completion callback event to be set when Request completes.
+ //
+ Status = gBS->CreateEvent (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ RequestCallback,
+ Context,
+ &Context->RequestToken.Event
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Context->RequestToken.Status = EFI_SUCCESS;
+ Context->RequestToken.Message = &RequestMessage;
+ gRequestCallbackComplete = FALSE;
+ Status = Context->Http->Request (Context->Http, &Context->RequestToken);
+ if (EFI_ERROR (Status)) {
+ goto Error;
+ }
+
+ Status = WaitForCompletion (Context, &gRequestCallbackComplete);
+ if (EFI_ERROR (Status)) {
+ Context->Http->Cancel (Context->Http, &Context->RequestToken);
+ }
+
+Error:
+ SHELL_FREE_NON_NULL (RequestHeader[HDR_HOST].FieldValue);
+ if (Context->RequestToken.Event) {
+ gBS->CloseEvent (Context->RequestToken.Event);
+ ZeroMem (&Context->RequestToken, sizeof (Context->RequestToken));
+ }
+
+ return Status;
+}
+
+/**
+ Update the progress of a file download
+ This procedure is called each time a new HTTP body portion is received.
+
+ @param[in] Context HTTP download context.
+ @param[in] DownloadLen Portion size, in bytes.
+ @param[in] Buffer The pointer to the parsed buffer.
+
+ @retval EFI_SUCCESS Portion saved.
+ @retval Other Error saving the portion.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+SavePortion (
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN UINTN DownloadLen,
+ IN CHAR8 *Buffer
+ )
+{
+ CHAR16 Progress[HTTP_PROGRESS_MESSAGE_SIZE];
+ UINTN NbOfKb;
+ UINTN Index;
+ UINTN LastStep;
+ UINTN Step;
+ EFI_STATUS Status;
+
+ LastStep = 0;
+ Step = 0;
+
+ ShellSetFilePosition (mFileHandle, Context->LastReportedNbOfBytes);
+ Status = ShellWriteFile (mFileHandle, &DownloadLen, Buffer);
+ if (EFI_ERROR (Status)) {
+ if (Context->ContentDownloaded > 0) {
+ PRINT_HII (STRING_TOKEN (STR_GEN_CRLF), NULL);
+ }
+
+ PRINT_HII (STRING_TOKEN (STR_HTTP_ERR_WRITE), mLocalFilePath, Status);
+ return Status;
+ }
+
+ if (Context->ContentDownloaded == 0) {
+ ShellPrintEx (-1, -1, L"%s 0 Kb", HTTP_PROGR_FRAME);
+ }
+
+ Context->ContentDownloaded += DownloadLen;
+ NbOfKb = Context->ContentDownloaded >> 10;
+
+ Progress[0] = L'\0';
+ if (Context->ContentLength) {
+ LastStep = (Context->LastReportedNbOfBytes *
HTTP_PROGRESS_SLIDER_STEPS) /
+ Context->ContentLength;
+ Step = (Context->ContentDownloaded * HTTP_PROGRESS_SLIDER_STEPS) /
+ Context->ContentLength;
+ }
+
+ Context->LastReportedNbOfBytes = Context->ContentDownloaded;
+
+ if (Step <= LastStep) {
+ if (!Context->ContentLength) {
+ //
+ // Update downloaded size, there is no length info available.
+ //
+ ShellPrintEx (-1, -1, L"%s", HTTP_KB);
+ ShellPrintEx (-1, -1, L"%7d Kb", NbOfKb);
+ }
+
+ return EFI_SUCCESS;
+ }
+
+ ShellPrintEx (-1, -1, L"%s", HTTP_PROGRESS_DEL);
+
+ Status = StrCpyS (Progress, HTTP_PROGRESS_MESSAGE_SIZE,
HTTP_PROGR_FRAME);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ for (Index = 1; Index < Step; Index++) {
+ Progress[Index] = L'=';
+ }
+
+ if (Step) {
+ Progress[Step] = L'>';
+ }
+
+ UnicodeSPrint (
+ Progress + (sizeof (HTTP_PROGR_FRAME) / sizeof (CHAR16)) - 1,
+ sizeof (Progress) - sizeof (HTTP_PROGR_FRAME),
+ L" %7d Kb",
+ NbOfKb
+ );
+
+
+ ShellPrintEx (-1, -1, L"%s", Progress);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Replace the original Host and URI with Host and URI returned by the
+ HTTP server in 'Location' header (redirection).
+
+ @param[in] Location A pointer to the 'Location' string
+ provided by HTTP server.
+ @param[in] Context A pointer to HTTP download context.
+ @param[in] DownloadUrl Fully qualified HTTP URL.
+
+ @return EFI_SUCCESS Host and URI were successfully set.
+ @return EFI_OUT_OF_RESOURCES Error setting Host or URI.
+**/
+
+STATIC
+EFI_STATUS
+SetHostURI (
+ IN CHAR8 *Location,
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN CHAR16 *DownloadUrl
+ )
+{
+ EFI_STATUS Status;
+ UINTN StringSize;
+ UINTN FirstStep;
+ UINTN Idx;
+ UINTN Step;
+ CHAR8 *Walker;
+ CHAR16 *Temp;
+ CHAR8 *Tmp;
+ CHAR16 *Url;
+ BOOLEAN IsAbEmptyUrl;
+
+ Tmp = NULL;
+ Url = NULL;
+ IsAbEmptyUrl = FALSE;
+ FirstStep = 0;
+
+ StringSize = (AsciiStrSize (Location) * sizeof (CHAR16));
+ Url = AllocateZeroPool (StringSize);
+ if (!Url) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = AsciiStrToUnicodeStrS (
+ (CONST CHAR8 *)Location,
+ Url,
+ StringSize
+ );
+
+ if (EFI_ERROR (Status)) {
+ goto Error;
+ }
+
+ //
+ // If an HTTP server redirects to the same location more than once,
+ // then stop attempts and tell it is not reachable.
+ //
+ if (!StrCmp (Url, DownloadUrl)) {
+ Status = EFI_NO_MAPPING;
+ goto Error;
+ }
+
+ if (AsciiStrLen (Location) > 2) {
+ // Some servers return 'Location: //server/resource'
+ IsAbEmptyUrl = (Location[0] == '/') && (Location[1] == '/');
+ if (IsAbEmptyUrl) {
+ // Skip first "//"
+ Location += 2;
+ FirstStep = 1;
+ }
+ }
+
+ if (AsciiStrStr (Location, "://") || IsAbEmptyUrl) {
+ Idx = 0;
+ Walker = Location;
+
+ for (Step = FirstStep; Step < 2; Step++) {
+ for (; *Walker != '/' && *Walker != '\0'; Walker++) {
+ Idx++;
+ }
+
+ if (!Step) {
+ //
+ // Skip "//"
+ //
+ Idx += 2;
+ Walker += 2;
+ }
+ }
+
+ Tmp = AllocateZeroPool (Idx + 1);
+ if (!Tmp) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Error;
+ }
+
+ CopyMem (Tmp, Location, Idx);
+
+ //
+ // Location now points to URI
+ //
+ Location += Idx;
+ StringSize = (Idx + 1) * sizeof (CHAR16);
+
+ SHELL_FREE_NON_NULL (Context->ServerAddrAndProto);
+
+ Temp = AllocateZeroPool (StringSize);
+ if (!Temp) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Error;
+ }
+
+ Status = AsciiStrToUnicodeStrS (
+ (CONST CHAR8 *)Tmp,
+ Temp,
+ StringSize
+ );
+ if (EFI_ERROR (Status)) {
+ SHELL_FREE_NON_NULL (Temp);
+ goto Error;
+ }
+
+ Idx = 0;
+ if (IsAbEmptyUrl) {
+ Context->ServerAddrAndProto = StrnCatGrow (
+ &Context->ServerAddrAndProto,
+ &Idx,
+ L"http://",
+ StrLen (L"http://")
+ );
+ }
+
+ Context->ServerAddrAndProto = StrnCatGrow (
+ &Context->ServerAddrAndProto,
+ &Idx,
+ Temp,
+ StrLen (Temp)
+ );
+ SHELL_FREE_NON_NULL (Temp);
+ if (!Context->ServerAddrAndProto) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Error;
+ }
+ }
+
+ SHELL_FREE_NON_NULL (Context->URI);
+
+ StringSize = AsciiStrSize (Location) * sizeof (CHAR16);
+ Context->URI = AllocateZeroPool (StringSize);
+ if (!Context->URI) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Error;
+ }
+
+ //
+ // Now make changes to the URI part.
+ //
+ Status = AsciiStrToUnicodeStrS (
+ (CONST CHAR8 *)Location,
+ Context->URI,
+ StringSize
+ );
+Error:
+ SHELL_FREE_NON_NULL (Tmp);
+ SHELL_FREE_NON_NULL (Url);
+
+ return Status;
+}
+
+/**
+ Message parser callback.
+ Save a portion of HTTP body.
+
+ @param[in] EventType Type of event. Can be either
+ OnComplete or OnData.
+ @param[in] Data A pointer to the buffer with data.
+ @param[in] Length Data length of this portion.
+ @param[in] Context A pointer to the HTTP download context.
+
+ @return EFI_SUCCESS The portion was processed successfully.
+ @return Other Error returned by SavePortion.
+**/
+
+STATIC
+EFI_STATUS
+EFIAPI
+ParseMsg (
+ IN HTTP_BODY_PARSE_EVENT EventType,
+ IN CHAR8 *Data,
+ IN UINTN Length,
+ IN VOID *Context
+ )
+{
+ if (!Data || (EventType == BodyParseEventOnComplete) || !Context) {
+ return EFI_SUCCESS;
+ }
+
+ return SavePortion (Context, Length, Data);
+}
+
+
+/**
+ Get HTTP server response and collect the whole body as a file.
+ Set appropriate status in Context (REQ_OK, REQ_REPEAT, REQ_ERROR).
+ Note that even if HTTP server returns an error code, it might send
+ the body as well. This body will be collected in the resultant file.
+
+ @param[in] Context A pointer to the HTTP download context.
+ @param[in] DownloadedUrl A pointer to the fully qualified URL to download.
+
+ @return EFI_SUCCESS Valid file. Body successfully collected.
+ @return EFI_HTTP_ERROR Response is a valid HTTP response, but the
+ HTTP server
+ indicated an error (HTTP code >= 400).
+ Response body MAY contain full
+ HTTP server response.
+ @return Others Error getting the reponse from the HTTP server.
+ Response body is not collected.
+**/
+STATIC
+EFI_STATUS
+GetResponse (
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN CHAR16 *DownloadUrl
+ )
+{
+ EFI_HTTP_RESPONSE_DATA ResponseData;
+ EFI_HTTP_MESSAGE ResponseMessage;
+ EFI_HTTP_HEADER *Header;
+ EFI_STATUS Status;
+ VOID *MsgParser;
+ EFI_TIME StartTime;
+ EFI_TIME EndTime;
+ CONST CHAR16 *Desc;
+ UINTN ElapsedSeconds;
+ BOOLEAN IsTrunked;
+ BOOLEAN CanMeasureTime;
+
+ ZeroMem (&ResponseData, sizeof (ResponseData));
+ ZeroMem (&ResponseMessage, sizeof (ResponseMessage));
+ ZeroMem (&Context->ResponseToken, sizeof (Context->ResponseToken));
+ IsTrunked = FALSE;
+
+ ResponseMessage.Body = Context->Buffer;
+ Context->ResponseToken.Status = EFI_SUCCESS;
+ Context->ResponseToken.Message = &ResponseMessage;
+ Context->ContentLength = 0;
+ Context->Status = REQ_OK;
+ MsgParser = NULL;
+ ResponseData.StatusCode = HTTP_STATUS_UNSUPPORTED_STATUS;
+ ResponseMessage.Data.Response = &ResponseData;
+ Context->ResponseToken.Event = NULL;
+ CanMeasureTime = FALSE;
+ if (Context->Flags & DL_FLAG_TIME) {
+ ZeroMem (&StartTime, sizeof (StartTime));
+ CanMeasureTime = !EFI_ERROR (gRT->GetTime (&StartTime, NULL));
+ }
+
+ do {
+ SHELL_FREE_NON_NULL (ResponseMessage.Headers);
+ ResponseMessage.HeaderCount = 0;
+ gResponseCallbackComplete = FALSE;
+ ResponseMessage.BodyLength = Context->BufferSize;
+
+ if (ShellGetExecutionBreakFlag ()) {
+ Status = EFI_ABORTED;
+ break;
+ }
+
+ if (!Context->ContentDownloaded && !Context->ResponseToken.Event) {
+ Status = gBS->CreateEvent (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ ResponseCallback,
+ Context,
+ &Context->ResponseToken.Event
+ );
+ ASSERT_EFI_ERROR (Status);
+ } else {
+ ResponseMessage.Data.Response = NULL;
+ }
+
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ Status = Context->Http->Response (Context->Http, &Context-

ResponseToken);

+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ Status = WaitForCompletion (Context, &gResponseCallbackComplete);
+ if (EFI_ERROR (Status) && ResponseMessage.HeaderCount) {
+ Status = EFI_SUCCESS;
+ }
+
+ if (EFI_ERROR (Status)) {
+ Context->Http->Cancel (Context->Http, &Context->ResponseToken);
+ break;
+ }
+
+ if (!Context->ContentDownloaded) {
+ if (NEED_REDIRECTION (ResponseData.StatusCode)) {
+ //
+ // Need to repeat the request with new Location (server redirected).
+ //
+ Context->Status = REQ_NEED_REPEAT;
+
+ Header = HttpFindHeader (
+ ResponseMessage.HeaderCount,
+ ResponseMessage.Headers,
+ "Location"
+ );
+ if (Header) {
+ Status = SetHostURI (Header->FieldValue, Context, DownloadUrl);
+ if (Status == EFI_NO_MAPPING) {
+ PRINT_HII (
+ STRING_TOKEN (STR_HTTP_ERR_STATUSCODE),
+ Context->ServerAddrAndProto,
+ L"Recursive HTTP server relocation",
+ Context->URI
+ );
+ }
+ } else {
+ //
+ // Bad reply from the server. Server must specify the location.
+ // Indicate that resource was not found, and no body collected.
+ //
+ Status = EFI_NOT_FOUND;
+ }
+
+ Context->Http->Cancel (Context->Http, &Context->ResponseToken);
+ break;
+ }
+
+ //
+ // Init message-body parser by header information.
+ //
+ if (!MsgParser) {
+ Status = HttpInitMsgParser (
+ ResponseMessage.Data.Request->Method,
+ ResponseData.StatusCode,
+ ResponseMessage.HeaderCount,
+ ResponseMessage.Headers,
+ ParseMsg,
+ Context,
+ &MsgParser
+ );
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+ }
+
+ //
+ // If it is a trunked message, rely on the parser.
+ //
+ Header = HttpFindHeader (
+ ResponseMessage.HeaderCount,
+ ResponseMessage.Headers,
+ "Transfer-Encoding"
+ );
+ IsTrunked = (Header && !AsciiStrCmp (Header->FieldValue, "chunked"));
+
+ HttpGetEntityLength (MsgParser, &Context->ContentLength);
+
+ if (ResponseData.StatusCode >= HTTP_STATUS_400_BAD_REQUEST
+ && (ResponseData.StatusCode !=
HTTP_STATUS_308_PERMANENT_REDIRECT))
+ {
+ //
+ // Server reported an error via Response code.
+ // Collect the body if any.
+ //
+ if (!gHttpError) {
+ gHttpError = TRUE;
+
+ Desc = ErrStatusDesc[ResponseData.StatusCode -
+ HTTP_STATUS_400_BAD_REQUEST];
+ PRINT_HII (
+ STRING_TOKEN (STR_HTTP_ERR_STATUSCODE),
+ Context->ServerAddrAndProto,
+ Desc,
+ Context->URI
+ );
+
+ //
+ // This gives an RFC HTTP error.
+ //
+ Context->Status = ShellStrToUintn (Desc);
+ Status = ENCODE_ERROR (Context->Status);
+ }
+ }
+ }
+
+ // Do NOT try to parse an empty body.
+ if (ResponseMessage.BodyLength || IsTrunked) {
+ Status = HttpParseMessageBody (
+ MsgParser,
+ ResponseMessage.BodyLength,
+ ResponseMessage.Body
+ );
+ }
+ } while (!HttpIsMessageComplete (MsgParser)
+ && !EFI_ERROR (Status)
+ && ResponseMessage.BodyLength);
+
+ if (Context->Status != REQ_NEED_REPEAT
+ && Status == EFI_SUCCESS
+ && CanMeasureTime)
+ {
+ if (!EFI_ERROR (gRT->GetTime (&EndTime, NULL))) {
+ ElapsedSeconds = EfiTimeToEpoch (&EndTime) - EfiTimeToEpoch
(&StartTime);
+ Print (
+ L",%a%Lus\n",
+ ElapsedSeconds ? " " : " < ",
+ ElapsedSeconds > 1 ? (UINT64)ElapsedSeconds : 1
+ );
+ }
+ }
+
+ SHELL_FREE_NON_NULL (MsgParser);
+ if (Context->ResponseToken.Event) {
+ gBS->CloseEvent (Context->ResponseToken.Event);
+ ZeroMem (&Context->ResponseToken, sizeof (Context->ResponseToken));
+ }
+
+ return Status;
+}
+
+/**
+ Worker function that downloads the data of a file from an HTTP server given
+ the path of the file and its size.
+
+ @param[in] Context A pointer to the HTTP download context.
+ @param[in] Controllerhandle The handle of the network interface controller
+ @param[in] NicName NIC name
+
+ @retval EFI_SUCCESS The file was downloaded.
+ @retval EFI_OUT_OF_RESOURCES A memory allocation failed.
+ #retval EFI_HTTP_ERROR The server returned a valid HTTP error.
+ Examine the mLocalFilePath file
+ to get error body.
+ @retval Others The downloading of the file from the server
+ failed.
+
+**/
+STATIC
+EFI_STATUS
+DownloadFile (
+ IN HTTP_DOWNLOAD_CONTEXT *Context,
+ IN EFI_HANDLE ControllerHandle,
+ IN CHAR16 *NicName
+ )
+{
+ EFI_STATUS Status;
+ CHAR16 *DownloadUrl;
+ UINTN UrlSize;
+ EFI_HANDLE HttpChildHandle;
+
+ ASSERT (Context);
+ if (!Context) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DownloadUrl = NULL;
+ HttpChildHandle = NULL;
+
+ Context->Buffer = AllocatePool (Context->BufferSize);
+ if (!Context->Buffer) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto ON_EXIT;
+ }
+
+ //
+ // OPEN FILE
+ //
+ if (!EFI_ERROR (ShellFileExists (mLocalFilePath))) {
+ ShellDeleteFileByName (mLocalFilePath);
+ }
+
+ Status = ShellOpenFileByName (
+ mLocalFilePath,
+ &mFileHandle,
+ EFI_FILE_MODE_CREATE |
+ EFI_FILE_MODE_WRITE |
+ EFI_FILE_MODE_READ,
+ 0
+ );
+ if (EFI_ERROR (Status)) {
+ PRINT_HII_APP (STRING_TOKEN (STR_GEN_FILE_OPEN_FAIL),
mLocalFilePath);
+ goto ON_EXIT;
+ }
+
+ do {
+ SHELL_FREE_NON_NULL (DownloadUrl);
+
+ CLOSE_HTTP_HANDLE (ControllerHandle, HttpChildHandle);
+
+ Status = CreateServiceChildAndOpenProtocol (
+ ControllerHandle,
+ &gEfiHttpServiceBindingProtocolGuid,
+ &gEfiHttpProtocolGuid,
+ &HttpChildHandle,
+ (VOID**)&Context->Http
+ );
+
+ if (EFI_ERROR (Status)) {
+ PRINT_HII (STRING_TOKEN (STR_HTTP_ERR_OPEN_PROTOCOL), NicName,
Status);
+ goto ON_EXIT;
+ }
+
+ Status = Context->Http->Configure (Context->Http, &Context-

HttpConfigData);

+ if (EFI_ERROR (Status)) {
+ PRINT_HII (STRING_TOKEN (STR_HTTP_ERR_CONFIGURE), NicName,
Status);
+ goto ON_EXIT;
+ }
+
+ UrlSize = 0;
+ DownloadUrl = StrnCatGrow (
+ &DownloadUrl,
+ &UrlSize,
+ Context->ServerAddrAndProto,
+ StrLen (Context->ServerAddrAndProto)
+ );
+ if (Context->URI[0] != L'/') {
+ DownloadUrl = StrnCatGrow (
+ &DownloadUrl,
+ &UrlSize,
+ L"/",
+ StrLen (Context->ServerAddrAndProto)
+ );
+ }
+
+ DownloadUrl = StrnCatGrow (
+ &DownloadUrl,
+ &UrlSize,
+ Context->URI,
+ StrLen (Context->URI));
+
+ PRINT_HII (STRING_TOKEN (STR_HTTP_DOWNLOADING), DownloadUrl);
+
+ Status = SendRequest (Context, DownloadUrl);
+ if (Status) {
+ goto ON_EXIT;
+ }
+
+ Status = GetResponse (Context, DownloadUrl);
+
+ if (Status) {
+ goto ON_EXIT;
+ }
+
+ } while (Context->Status == REQ_NEED_REPEAT);
+
+ if (Context->Status) {
+ Status = ENCODE_ERROR (Context->Status);
+ }
+
+ON_EXIT:
+ //
+ // CLOSE FILE
+ //
+ if (mFileHandle) {
+ if (EFI_ERROR (Status) && !(Context->Flags & DL_FLAG_KEEP_BAD)) {
+ ShellDeleteFile (&mFileHandle);
+ } else {
+ ShellCloseFile (&mFileHandle);
+ }
+ }
+
+ SHELL_FREE_NON_NULL (DownloadUrl);
+ SHELL_FREE_NON_NULL (Context->Buffer);
+
+ CLOSE_HTTP_HANDLE (ControllerHandle, HttpChildHandle);
+
+ return Status;
+}
+
+/**
+ Retrive HII package list from ImageHandle and publish to HII database.
+
+ @param ImageHandle The image handle of the process.
+
+ @return HII handle.
+**/
+EFI_HII_HANDLE
+InitializeHiiPackage (
+ EFI_HANDLE ImageHandle
+ )
+{
+ EFI_STATUS Status;
+ EFI_HII_PACKAGE_LIST_HEADER *PackageList;
+ EFI_HII_HANDLE HiiHandle;
+
+ //
+ // Retrieve HII package list from ImageHandle
+ //
+ Status = gBS->OpenProtocol (
+ ImageHandle,
+ &gEfiHiiPackageListProtocolGuid,
+ (VOID **)&PackageList,
+ ImageHandle,
+ NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ return NULL;
+ }
+
+ //
+ // Publish HII package list to HII Database.
+ //
+ Status = gHiiDatabase->NewPackageList (
+ gHiiDatabase,
+ PackageList,
+ NULL,
+ &HiiHandle
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ return NULL;
+ }
+
+ return HiiHandle;
+}
diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
new file mode 100644
index 000000000000..a7d2c27191a2
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
@@ -0,0 +1,61 @@
+/** @file
+ Entrypoint of "http" shell standalone application.
+
+ Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. <BR>
+ Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2020, Broadcom. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include "Http.h"
+
+/*
+ * String token ID of help message text.
+ * Shell supports to find help message in the resource section of an
+ * application image if * .MAN file is not found.
+ * This global variable is added to make build tool recognizes
+ * that the help string is consumed by user and then build tool will
+ * add the string into the resource section.
+ * Thus the application can use '-?' option to show help message in Shell.
+ */
+GLOBAL_REMOVE_IF_UNREFERENCED
+EFI_STRING_ID mStringHelpTokenId = STRING_TOKEN (STR_GET_HELP_HTTP);
+
+/**
+ Entry point of Http standalone application.
+
+ @param ImageHandle The image handle of the process.
+ @param SystemTable The EFI System Table pointer.
+
+ @retval EFI_SUCCESS Http command is executed sucessfully.
+ @retval EFI_ABORTED HII package was failed to initialize.
+ @retval others Other errors when executing http command.
+**/
+EFI_STATUS
+EFIAPI
+HttpAppInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+ SHELL_STATUS ShellStatus;
+
+ mHttpHiiHandle = InitializeHiiPackage (ImageHandle);
+ if (mHttpHiiHandle == NULL) {
+ return EFI_ABORTED;
+ }
+
+ Status = EFI_SUCCESS;
+
+ ShellStatus = RunHttp (ImageHandle, SystemTable);
+
+ HiiRemovePackages (mHttpHiiHandle);
+
+ if (Status != SHELL_SUCCESS) {
+ Status = ENCODE_ERROR (ShellStatus);
+ }
+
+ return Status;
+}
diff --git
a/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
new file mode 100644
index 000000000000..7f59cc74d2a7
--- /dev/null
+++
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
@@ -0,0 +1,137 @@
+/** @file
+ Produce "http" shell dynamic command.
+
+ Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. <BR>
+ Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2020, Broadcom. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include <Protocol/ShellDynamicCommand.h>
+#include "Http.h"
+
+/**
+ This is the shell command handler function pointer callback type. This
+ function handles the command when it is invoked in the shell.
+
+ @param[in] This The instance of the
+ EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL.
+ @param[in] SystemTable The pointer to the system table.
+ @param[in] ShellParameters The parameters associated with the command.
+ @param[in] Shell The instance of the shell protocol used in
+ the context of processing this command.
+
+ @return EFI_SUCCESS the operation was sucessful
+ @return other the operation failed.
+**/
+SHELL_STATUS
+EFIAPI
+HttpCommandHandler (
+ IN EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL *This,
+ IN EFI_SYSTEM_TABLE *SystemTable,
+ IN EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters,
+ IN EFI_SHELL_PROTOCOL *Shell
+ )
+{
+ gEfiShellParametersProtocol = ShellParameters;
+ gEfiShellProtocol = Shell;
+
+ return RunHttp (gImageHandle, SystemTable);
+}
+
+/**
+ This is the command help handler function pointer callback type. This
+ function is responsible for displaying help information for the associated
+ command.
+
+ @param[in] This The instance of the
EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL.
+ @param[in] Language The pointer to the language string to use.
+
+ @return string Pool allocated help string, must be freed by caller
+**/
+CHAR16 *
+EFIAPI
+HttpCommandGetHelp (
+ IN EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL *This,
+ IN CONST CHAR8 *Language
+ )
+{
+ return HiiGetString (
+ mHttpHiiHandle,
+ STRING_TOKEN (STR_GET_HELP_HTTP),
+ Language
+ );
+}
+
+EFI_SHELL_DYNAMIC_COMMAND_PROTOCOL mHttpDynamicCommand = {
+ HTTP_APP_NAME,
+ HttpCommandHandler,
+ HttpCommandGetHelp
+};
+
+/**
+ Entry point of Http Dynamic Command.
+
+ Produce the DynamicCommand protocol to handle "http" command.
+
+ @param ImageHandle The image handle of the process.
+ @param SystemTable The EFI System Table pointer.
+
+ @retval EFI_SUCCESS Http command is executed sucessfully.
+ @retval EFI_ABORTED HII package was failed to initialize.
+ @retval others Other errors when executing http command.
+**/
+EFI_STATUS
+EFIAPI
+HttpCommandInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ mHttpHiiHandle = InitializeHiiPackage (ImageHandle);
+ if (mHttpHiiHandle == NULL) {
+ return EFI_ABORTED;
+ }
+
+ Status = gBS->InstallProtocolInterface (
+ &ImageHandle,
+ &gEfiShellDynamicCommandProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mHttpDynamicCommand
+ );
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
+
+/**
+ Http driver unload handler.
+
+ @param ImageHandle The image handle of the process.
+
+ @retval EFI_SUCCESS The image is unloaded.
+ @retval Others Failed to unload the image.
+**/
+EFI_STATUS
+EFIAPI
+HttpUnload (
+ IN EFI_HANDLE ImageHandle
+)
+{
+ EFI_STATUS Status;
+
+ Status = gBS->UninstallProtocolInterface (
+ ImageHandle,
+ &gEfiShellDynamicCommandProtocolGuid,
+ &mHttpDynamicCommand
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ HiiRemovePackages (mHttpHiiHandle);
+
+ return EFI_SUCCESS;
+}
diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
new file mode 100644
index 000000000000..00cf05deeb5c
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
@@ -0,0 +1,117 @@
+// /**
+//
+// (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
+// Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. <BR>
+// Copyright (c) 2020, Broadcom. All rights reserved.<BR>
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// Module Name:
+//
+// Http.uni
+//
+// Abstract:
+//
+// String definitions for UEFI Shell HTTP command
+//
+//
+// **/
+
+/=#
+
+#langdef en-US "english"
+
+#string STR_GEN_TOO_MANY #language en-US "%H%s%N: Too many
arguments. Try help http.\r\n"
+#string STR_GEN_TOO_FEW #language en-US "%H%s%N: Too few
arguments. Try help http.\r\n"
+#string STR_GEN_PARAM_INV #language en-US "%H%s%N: Invalid
argument - '%H%s%N'. Try help http.\r\n"
+#string STR_GEN_PROBLEM #language en-US "%H%s%N: Unknown flag -
'%H%s%N'. Try help http.\r\n"
+#string STR_GEN_FILE_OPEN_FAIL #language en-US "%H%s%N: Cannot open
file - '%H%s%N'\r\n"
+#string STR_GEN_CRLF #language en-US "\r\n"
+
+#string STR_HTTP_ERR_NO_NIC #language en-US "No network interface
card found.\r\n"
+#string STR_HTTP_ERR_NIC_NAME #language en-US "Failed to get the name
of the network interface card number %d - %r\r\n"
+#string STR_HTTP_ERR_OPEN_PROTOCOL #language en-US "Unable to open
HTTP protocol on '%H%s%N' - %r\r\n"
+#string STR_HTTP_ERR_CONFIGURE #language en-US "Unable to configure
HTTP protocol on '%H%s%N' - %r\r\n"
+#string STR_HTTP_ERR_DOWNLOAD #language en-US "Unable to download
the file '%H%s%N' on '%H%s%N' - %r\r\n"
+#string STR_HTTP_ERR_WRITE #language en-US "Unable to write into file
'%H%s%N' - %r\r\n"
+#string STR_HTTP_ERR_NIC_NOT_FOUND #language en-US "Network Interface
Card '%H%s%N' not found.\r\n"
+#string STR_HTTP_ERR_STATUSCODE #language en-US "\r'%H%s%N' reports
'%s' for '%H%s%N' \r\n"
+#string STR_HTTP_DOWNLOADING #language en-US "Downloading
'%H%s%N'\r\n"
+
+#string STR_GET_HELP_HTTP #language en-US ""
+".TH http 0 "Download a file from HTTP server."\r\n"
+".SH NAME\r\n"
+"Download a file from HTTP server.\r\n"
+".SH SYNOPSIS\r\n"
+" \r\n"
+"HTTP [-i interface] [-l port] [-t timeout] [-s size] [-m] [-k]\r\n"
+" <URL> [localfilepath]\r\n"
+".SH OPTIONS\r\n"
+" \r\n"
+" -i interface - Specifies an adapter name, i.e., eth0.\r\n"
+" -k Keep the downloaded file even if there was an error.\r\n"
+" If this parameter is not used, the file will be deleted.\r\n"
+" -l port - Specifies the local port number. Default value is 0\r\n"
+" and the port number is automatically assigned.\r\n"
+" -m Measure and report download time (in seconds). \r\n"
+" -s size The size of the download buffer for a chunk, in bytes.\r\n"
+" Default is 32K. Note that larger buffer does not imply\r\n"
+" better speed.\r\n"
+" -t timeout - The number of seconds to wait for completion of\r\n"
+" requests and responses. Default is 0 which is 'automatic'.\r\n"
+" %HURL%N\r\n"
+" Two types of providing of URLs are supported:\r\n"
+" 1. tftp-like, where host and http_uri are separate parameters\r\n"
+" (example: host /host_uri), and\r\n\"
+" 2. wget-like, where host and host_uri is one parameter.\r\n"
+" (example: host/host_uri)\r\n"
+"\r\n"
+" host - Specifies HTTP Server address.\r\n
+ Can be either IPv4 address or 'http (or https)://addr'\r\n
+ Can use addresses resolvable by DNS as well. \r\n
+ Port can be specified after ':' if needed. \r\n
+ By default port 80 is used.\r\n"
+" http_uri - HTTP server URI to download the file.\r\n"
+"\r\n"
+" localfilepath - Local destination file path.\r\n"
+".SH DESCRIPTION\r\n"
+" \r\n"
+"NOTES:\r\n"
+" 1. The HTTP command allows geting of the file specified by its 'http_uri'\r\n"
+" path from the HTTP server specified by its 'host' IPv4 address. If the\r\n"
+" optional 'localfilepath' parameter is provided, the downloaded file is\r\n"
+" stored locally using the provided file path. If the local file path is\r\n"
+" not specified, the file is stored in the current directory using the file\r\n"
+" server's name.\r\n"
+" 2. Before using the HTTP command, the network interface intended to
be\r\n"
+" used to retrieve the file must be configured. This configuration may be\r\n"
+" done by means of the 'ifconfig' command.\r\n"
+" 3. If a network interface is defined with the '-i' option then only this\r\n"
+" interface will be used to retrieve the remote file. Otherwise, all
network\r\n"
+" interfaces are tried in the order they have been discovered during the\r\n"
+" DXE phase.\r\n"
+".SH EXAMPLES\r\n"
+" \r\n"
+"EXAMPLES:\r\n"
+" * To get the file "dir1/file1.dat" from the HTTP server 192.168.1.1, port 8080,
and\r\n"
+" store it as file2.dat in the current directory (use tftp-like URL format) :\r\n"
+" fs0:\> http 192.168.1.1:8080 dir1/file1.dat file2.dat\r\n"
+" * To get the file /image.bin via HTTPS from server 192.168.1.1 at port 443
\r\n"
+" (default HTTPS port), and store it in the current directory: \r\n"
+" fs0:\> http https://192.168.1.1 image.bin\r\n"
+" To get an index file from http://google.com and place it into the \r\n"
+" current directory:\r\n"
+" fs0:\> http google.com index.html\r\n"
+".SH RETURNVALUES\r\n"
+" \r\n"
+"RETURN VALUES:\r\n"
+" SHELL_SUCCESS The action was completed as requested.\r\n"
+" SHELL_INVALID_PARAMETER One of the passed-in parameters was
incorrectly\r\n"
+" formatted or its value was out of bounds.\r\n"
+" HTTP_ERROR No EFI errors, but the server reported a status
code\r\n"
+" which should be treated as an error. If an error body sent\r\n"
+" by the server, and -k parameter is on command line,
+" the file wil be saved either as localfilepath filename,\r\n"
+" or as an URI name in the current directory.\r\n"
+" If '/' is at the end of the URL, and no locafilepath filename\r\n"
+" is given on the command line, the file will be retrieved as\r\n"
+" index.html.\r\n"
--
2.28.0.394.ge197136389

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami
Mujawar
Sent: Tuesday, August 25, 2020 7:07 PM
To: devel@edk2.groups.io
Cc: Sami Mujawar <sami.mujawar@...>; Ni, Ray <ray.ni@...>; Gao,
Zhichao <zhichao.gao@...>; marc.moisson-franckhauser@...;
Guillaume.Letellier@...; Matteo.Carlini@...;
Ben.Adderson@...; nd@...
Subject: [edk2-devel] [PATCH v2 1/1] ShellPkg/AcpiView: HMAT Parser

From: Marc Moisson-Franckhauser <marc.moisson-franckhauser@...>

Add a new parser for the Heterogeneous Memory Attribute Table. The parser
also validates some fields for this table.

The HMAT table is used to describe the memory attributes such as memory side
cache attributes and bandwidth and latency details related to memory proximity
domains. The info in the HMAT table can be used by an operating system for
optimisation.

Signed-off-by: Marc Moisson-Franckhauser <marc.moisson-
franckhauser@...>
Signed-off-by: Sami Mujawar <sami.mujawar@...>
---

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/833_hmat_parser_v2

Notes:
v2:
- Fixed minor output formatting in DumpCacheAttributes() [SAMI]

ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 28 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h | 4
+-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
| 641 ++++++++++++++++++++

ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
| 3 +-

ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.i
nf | 3 +-

ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.
uni | 3 +-
6 files changed, 676 insertions(+), 6 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index
f81ccac7e118378aa185db4b625e5bcd75f78347..969cc0b371852f01f30c88dc506
374a459c9c19e 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -1,7 +1,7 @@
/** @file
Header file for ACPI parser

- Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
+ Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent **/

@@ -592,6 +592,32 @@ ParseAcpiGtdt (
);

/**
+ This function parses the ACPI HMAT table.
+ When trace is enabled this function parses the HMAT table and traces
+ the ACPI table fields.
+
+ This function parses the following HMAT structures:
+ - Memory Proximity Domain Attributes Structure (Type 0)
+ - System Locality Latency and Bandwidth Info Structure (Type 1)
+ - Memory Side Cache Info structure (Type 2)
+
+ This function also performs validation of the ACPI table fields.
+
+ @param [in] Trace If TRUE, trace the ACPI fields.
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] AcpiTableLength Length of the ACPI table.
+ @param [in] AcpiTableRevision Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiHmat (
+ IN BOOLEAN Trace,
+ IN UINT8* Ptr,
+ IN UINT32 AcpiTableLength,
+ IN UINT8 AcpiTableRevision
+ );
+
+/**
This function parses the ACPI IORT table.
When trace is enabled this function parses the IORT table and
traces the ACPI fields.
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
index
4f92596b90a6ee422d8d0959881015ffd3de4da0..f8e8b5979f3be041bbc8d17042
b2db8e0b73f205 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
@@ -1,7 +1,7 @@
/** @file
Header file for ACPI table parser

- Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2016 - 2018, Arm Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent **/

@@ -11,7 +11,7 @@
/**
The maximum number of ACPI table parsers.
*/
-#define MAX_ACPI_TABLE_PARSERS 16
+#define MAX_ACPI_TABLE_PARSERS 32

/** An invalid/NULL signature value.
*/
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
new file mode 100644
index
0000000000000000000000000000000000000000..57b93cca6ba24ed77f9dcd7bf
2f45ba9f0cb9f26
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatPars
+++ er.c
@@ -0,0 +1,641 @@
+/** @file
+ HMAT table parser
+
+ Copyright (c) 2020, Arm Limited.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - ACPI 6.3 Specification - January 2019
+
+ @par Glossary:
+ - MPDA - Memory Proximity Domain Attributes
+ - SLLBI - System Locality Latency and Bandwidth Information
+ - MSCI - Memory Side Cache Information
+ - Dom - Domain
+**/
+
+#include <Library/PrintLib.h>
+#include <Library/BaseLib.h>
+#include <Library/UefiLib.h>
+#include "AcpiParser.h"
+#include "AcpiView.h"
+
+// Maximum Memory Domain matrix print size.
+#define MAX_MEMORY_DOMAIN_TARGET_PRINT_MATRIX 12
+
+// Local variables
+STATIC CONST UINT16* HmatStructureType; STATIC CONST UINT32*
+HmatStructureLength;
+
+STATIC CONST UINT32* NumberInitiatorProximityDomain; STATIC CONST
+UINT32* NumberTargetProximityDomain; STATIC CONST
+EFI_ACPI_6_3_HMAT_STRUCTURE_SYSTEM_LOCALITY_LATENCY_AND_BAND
WIDTH_INFO_
+FLAGS*
+SllbiFlags;
+
+STATIC CONST UINT8* SllbiDataType;
+STATIC CONST UINT32* NumberSMBIOSHandles;
+
+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+ Names of System Locality Latency Bandwidth Information (SLLBI) data
+types **/ STATIC CONST CHAR16* SllbiNames[] = {
+ L"Access %sLatency%s",
+ L"Read %sLatency%s",
+ L"Write %sLatency%s",
+ L"Access %sBandwidth%s",
+ L"Read %sBandwidth%s",
+ L"Write %sBandwidth%s"
+};
+
+/**
+ This function validates the Cache Attributes field.
+
+ @param [in] Ptr Pointer to the start of the field data.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateCacheAttributes (
+ IN UINT8* Ptr,
+ IN VOID* Context
+ )
+{
+
EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTR
IBUTES*
+ Attributes;
+
+ Attributes =
+
+
(EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATT
RIBUTES*)
+ Ptr;
+
+ if (Attributes->TotalCacheLevels > 0x3) {
+ IncrementErrorCount ();
+ Print (
+ L"\nERROR: Attributes bits [3:0] have invalid value: 0x%x",
+ Attributes->TotalCacheLevels
+ );
+ }
+ if (Attributes->CacheLevel > 0x3) {
+ IncrementErrorCount ();
+ Print (
+ L"\nERROR: Attributes bits [7:4] have invalid value: 0x%x",
+ Attributes->CacheLevel
+ );
+ }
+ if (Attributes->CacheAssociativity > 0x2) {
+ IncrementErrorCount ();
+ Print (
+ L"\nERROR: Attributes bits [11:8] have invalid value: 0x%x",
+ Attributes->CacheAssociativity
+ );
+ }
+ if (Attributes->WritePolicy > 0x2) {
+ IncrementErrorCount ();
+ Print (
+ L"\nERROR: Attributes bits [15:12] have invalid value: 0x%x",
+ Attributes->WritePolicy
+ );
+ }
+}
+
+/**
+ Dumps the cache attributes field
+
+ @param [in] Format Optional format string for tracing the data.
+ @param [in] Ptr Pointer to the start of the buffer.
+**/
+STATIC
+VOID
+EFIAPI
+DumpCacheAttributes (
+ IN CONST CHAR16* Format OPTIONAL,
+ IN UINT8* Ptr
+ )
+{
+
EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTR
IBUTES*
+ Attributes;
+
+ Attributes =
+
+
(EFI_ACPI_6_3_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATT
RIBUTES*)
+ Ptr;
+
+ Print (L"\n");
+ PrintFieldName (4, L"Total Cache Levels");
+ Print (L"%d\n", Attributes->TotalCacheLevels);
+ PrintFieldName (4, L"Cache Level");
+ Print (L"%d\n", Attributes->CacheLevel);
+ PrintFieldName (4, L"Cache Associativity");
+ Print (L"%d\n", Attributes->CacheAssociativity);
+ PrintFieldName (4, L"Write Policy");
+ Print (L"%d\n", Attributes->WritePolicy);
+ PrintFieldName (4, L"Cache Line Size");
+ Print (L"%d\n", Attributes->CacheLineSize); }
+
+/**
+ An ACPI_PARSER array describing the ACPI HMAT Table.
+*/
+STATIC CONST ACPI_PARSER HmatParser[] = {
+ PARSE_ACPI_HEADER (&AcpiHdrInfo),
+ {L"Reserved", 4, 36, NULL, NULL, NULL, NULL, NULL} };
+
+/**
+ An ACPI_PARSER array describing the HMAT structure header.
+*/
+STATIC CONST ACPI_PARSER HmatStructureHeaderParser[] = {
+ {L"Type", 2, 0, NULL, NULL, (VOID**)&HmatStructureType, NULL, NULL},
+ {L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL},
+ {L"Length", 4, 4, NULL, NULL, (VOID**)&HmatStructureLength, NULL,
+NULL} };
+
+/**
+ An ACPI PARSER array describing the Memory Proximity Domain
+Attributes
+ Structure - Type 0.
+*/
+STATIC CONST ACPI_PARSER MemProximityDomainAttributeParser[] = {
+ {L"Type", 2, 0, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL},
+ {L"Flags", 2, 8, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Reserved", 2, 10, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Proximity Dom for initiator", 4, 12, L"0x%x", NULL, NULL, NULL,
+NULL},
+ {L"Proximity Dom for memory", 4, 16, L"0x%x", NULL, NULL, NULL,
+NULL},
+ {L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Reserved", 8, 24, L"0x%lx", NULL, NULL, NULL, NULL},
+ {L"Reserved", 8, 32, L"0x%lx", NULL, NULL, NULL, NULL} };
+
+/**
+ An ACPI PARSER array describing the System Locality Latency and
+Bandwidth
+ Information Structure - Type 1.
+*/
+STATIC CONST ACPI_PARSER
+SllbiParser[] = {
+ {L"Type", 2, 0, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL},
+ {L"Flags", 1, 8, L"0x%x", NULL, (VOID**)&SllbiFlags, NULL, NULL},
+ {L"Data type", 1, 9, L"0x%x", NULL, (VOID**)&SllbiDataType, NULL,
+NULL},
+ {L"Reserved", 2, 10, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Initiator Proximity Dom Count", 4, 12, L"%d", NULL,
+ (VOID**)&NumberInitiatorProximityDomain, NULL, NULL},
+ {L"Target Proximity Dom Count", 4, 16, L"%d", NULL,
+ (VOID**)&NumberTargetProximityDomain, NULL, NULL},
+ {L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Entry Base Unit", 8, 24, L"0x%lx", NULL, NULL, NULL, NULL}
+ // initiator Proximity Domain list ...
+ // target Proximity Domain list ...
+ // Latency/Bandwidth matrix ...
+};
+
+/**
+ An ACPI PARSER array describing the Memory Side Cache Information
+ Structure - Type 2.
+*/
+STATIC CONST ACPI_PARSER MemSideCacheInfoParser[] = {
+ {L"Type", 2, 0, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Reserved", 2, 2, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL},
+ {L"Proximity Dom for memory", 4, 8, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Reserved", 4, 12, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Memory Side Cache Size", 8, 16, L"0x%lx", NULL, NULL, NULL, NULL},
+ {L"Cache Attributes", 4, 24, NULL, DumpCacheAttributes, NULL,
+ ValidateCacheAttributes, NULL},
+ {L"Reserved", 2, 28, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"SMBIOS Handle Count", 2, 30, L"%d", NULL,
+ (VOID**)&NumberSMBIOSHandles, NULL, NULL}
+ // SMBIOS handles List ...
+};
+
+/**
+ This function parses the Memory Proximity Domain Attributes
+ Structure (Type 0).
+
+ @param [in] Ptr Pointer to the start of the Memory Proximity Domain
+ Attributes Structure data.
+ @param [in] Length Length of the Memory Proximity Domain Attributes
+ Structure.
+**/
+STATIC
+VOID
+DumpMpda (
+ IN UINT8* Ptr,
+ IN UINT8 Length
+ )
+{
+ ParseAcpi (
+ TRUE,
+ 2,
+ "Memory Proximity Domain Attributes Structure",
+ Ptr,
+ Length,
+ PARSER_PARAMS (MemProximityDomainAttributeParser)
+ );
+}
+
+/**
+ This function parses the System Locality Latency and Bandwidth
+Information
+ Structure (Type 1).
+
+ @param [in] Ptr Pointer to the start of the System Locality Latency and
+ Bandwidth Information Structure data.
+ @param [in] Length Length of the System Locality Latency and Bandwidth
+ Information Structure.
+**/
+STATIC
+VOID
+DumpSllbi (
+ IN UINT8* Ptr,
+ IN UINT8 Length
+ )
+{
+ CONST UINT32* InitiatorProximityDomainList;
+ CONST UINT32* TargetProximityDomainList;
+ CONST UINT16* LatencyBandwidthMatrix;
+ UINT32 Offset;
+ CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
+ CHAR16 SecondBuffer[OUTPUT_FIELD_COLUMN_WIDTH];
+ UINT32 RequiredTableSize;
+ UINT32 Index;
+ UINT32 IndexInitiator;
+ UINT32 IndexTarget;
+
+ Offset = ParseAcpi (
+ TRUE,
+ 2,
+ "System Locality Latency and Bandwidth Information Structure",
+ Ptr,
+ Length,
+ PARSER_PARAMS (SllbiParser)
+ );
+
+ // Check if the values used to control the parsing logic have been
+ // successfully read.
+ if ((SllbiFlags == NULL) ||
+ (SllbiDataType == NULL) ||
+ (NumberInitiatorProximityDomain == NULL) ||
+ (NumberTargetProximityDomain == NULL)) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Insufficient remaining table buffer length to read the " \
+ L"SLLBI structure header. Length = %d.\n",
+ Length
+ );
+ return;
+ }
+
+ RequiredTableSize = (*NumberInitiatorProximityDomain * sizeof (UINT32)) +
+ (*NumberTargetProximityDomain * sizeof (UINT32)) +
+ (*NumberInitiatorProximityDomain *
+ *NumberTargetProximityDomain * sizeof (UINT16)) +
+ Offset;
+
+ if (RequiredTableSize > Length) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Insufficient System Locality Latency and Bandwidth" \
+ L"Information Structure length. TableLength = %d. " \
+ L"RequiredTableLength = %d.\n",
+ Length,
+ RequiredTableSize
+ );
+ return;
+ }
+
+ InitiatorProximityDomainList = (UINT32*) (Ptr + Offset);
+ TargetProximityDomainList = InitiatorProximityDomainList +
+ *NumberInitiatorProximityDomain;
+ LatencyBandwidthMatrix = (UINT16*) (TargetProximityDomainList +
+ *NumberTargetProximityDomain);
+
+ // Display each element of the Initiator Proximity Domain list for
+ (Index = 0; Index < *NumberInitiatorProximityDomain; Index++) {
+ UnicodeSPrint (
+ Buffer,
+ sizeof (Buffer),
+ L"Initiator Proximity Dom [%d]",
+ Index
+ );
+
+ PrintFieldName (4, Buffer);
+ Print (
+ L"0x%x\n",
+ InitiatorProximityDomainList[Index]
+ );
+ }
+
+ // Display each element of the Target Proximity Domain list for
+ (Index = 0; Index < *NumberTargetProximityDomain; Index++) {
+ UnicodeSPrint (
+ Buffer,
+ sizeof (Buffer),
+ L"Target Proximity Dom [%d]",
+ Index
+ );
+
+ PrintFieldName (4, Buffer);
+ Print (
+ L"0x%x\n",
+ TargetProximityDomainList[Index]
+ );
+ }
+
+ // Create base name depending on Data Type in this Structure if
+ (*SllbiDataType >= ARRAY_SIZE (SllbiNames)) {
+ IncrementErrorCount ();
+ Print (L"Error: Unkown Data Type. DataType = 0x%x.\n", *SllbiDataType);
+ return;
+ }
+ StrCpyS (Buffer, sizeof (Buffer), SllbiNames[*SllbiDataType]);
+
+ // Adjust base name depending on Memory Hierarchy in this Structure
+ switch (SllbiFlags->MemoryHierarchy) {
+ case 0: {
+ UnicodeSPrint (
+ SecondBuffer,
+ sizeof (SecondBuffer),
+ Buffer,
+ L"",
+ L"%s"
+ );
+ break;
+ }
+
+ case 1:
+ case 2:
+ case 3: {
+ UnicodeSPrint (
+ SecondBuffer,
+ sizeof (SecondBuffer),
+ Buffer,
+ L"Hit ",
+ L"%s"
+ );
+ break;
+ }
+
+ default: {
+ IncrementErrorCount ();
+ Print (
+ L"Error: Invalid Memory Hierarchy. MemoryHierarchy = %d.\n",
+ SllbiFlags->MemoryHierarchy
+ );
+ return;
+ }

+ } // switch
+
+ if (IndexInitiator <= MAX_MEMORY_DOMAIN_TARGET_PRINT_MATRIX) {

+ // Display the latency/bandwidth matrix as a matrix
+ UnicodeSPrint (
+ Buffer,
+ sizeof (Buffer),
+ SecondBuffer,
+ L""
+ );
+ PrintFieldName (4, Buffer);
+ Print (L"\n\n Initiator ->\nTarget\n|\nV\n");

+ Print (L" |");
+
+ for (IndexTarget = 0;
+ IndexTarget < *NumberTargetProximityDomain;
+ IndexTarget++) {
+ Print (L" %2d", IndexTarget);
+ }
+
+ Print (L"\n---+");
+ for (IndexTarget = 0;
+ IndexTarget < *NumberTargetProximityDomain;
+ IndexTarget++) {
+ Print (L"-------");
+ }
+ Print (L"\n");
+
+ for (IndexInitiator = 0;
+ IndexInitiator < *NumberInitiatorProximityDomain;
+ IndexInitiator++) {
+ Print (L"%2d |", IndexInitiator);
+ for (IndexTarget = 0;
+ IndexTarget < *NumberTargetProximityDomain;
+ IndexTarget++) {
+ Print (
+ L" %5d",
+ LatencyBandwidthMatrix[IndexInitiator *
+ (*NumberTargetProximityDomain + IndexTarget)]

+ );
+ } // for Target
+ Print (L"\n");
+ } // for Initiator
+ Print (L"\n");
+ } else {
+ // Display the latency/bandwidth matrix as a list
+ UnicodeSPrint (
+ Buffer,
+ sizeof (Buffer),
+ SecondBuffer,
+ L" [%d][%d]"
+ );
+ for (IndexInitiator = 0;
+ IndexInitiator < *NumberInitiatorProximityDomain;
+ IndexInitiator++) {
+ for (IndexTarget = 0;
+ IndexTarget < *NumberTargetProximityDomain;
+ IndexTarget++) {
+ UnicodeSPrint (
+ Buffer,
+ sizeof (Buffer),
+ SecondBuffer,
+ IndexInitiator,
+ IndexTarget
+ );
+
+ PrintFieldName (4, Buffer);
+ Print (
+ L"%d\n",
+ LatencyBandwidthMatrix[IndexInitiator *
+ (*NumberTargetProximityDomain + IndexTarget)]
+ );
+ } // for Target
+ } // for Initiator
+ }
+}
+
+/**
+ This function parses the Memory Side Cache Information Structure (Type 2).
+
+ @param [in] Ptr Pointer to the start of the Memory Side Cache Information
+ Structure data.
+ @param [in] Length Length of the Memory Side Cache Information Structure.
+**/
+STATIC
+VOID
+DumpMsci (
+ IN UINT8* Ptr,
+ IN UINT8 Length
+ )
+{
+ CONST UINT16* SMBIOSHandlesList;
+ CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
+ UINT32 Offset;
+ UINT16 Index;
+
+ Offset = ParseAcpi (
+ TRUE,
+ 2,
+ "Memory Side Cache Information Structure",
+ Ptr,
+ Length,
+ PARSER_PARAMS (MemSideCacheInfoParser)
+ );
+
+ // Check if the values used to control the parsing logic have been
+ // successfully read.
+ if (NumberSMBIOSHandles == NULL) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Insufficient remaining table buffer length to read the " \
+ L"MSCI structure header. Length = %d.\n",
+ Length
+ );
+ return;
+ }
+
+ if ((*NumberSMBIOSHandles * sizeof (UINT16)) > (Length - Offset)) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Invalid Number of SMBIOS Handles. SMBIOSHandlesCount = %d."
\
+ L"RemainingBufferLength = %d.\n",
+ *NumberSMBIOSHandles,
+ Length - Offset
+ );
+ return;
+ }
+
+ SMBIOSHandlesList = (UINT16*) (Ptr + Offset);
+
+ for (Index = 0; Index < *NumberSMBIOSHandles; Index++) {
+ UnicodeSPrint (
+ Buffer,
+ sizeof (Buffer),
+ L"SMBIOS Handles [%d]",
+ Index
+ );
+
+ PrintFieldName (4, Buffer);
+ Print (
+ L"0x%x\n",
+ SMBIOSHandlesList[Index]
+ );
+ }
+}

+
+/**
+ This function parses the ACPI HMAT table.
+ When trace is enabled this function parses the HMAT table and
+ traces the ACPI table fields.
+
+ This function parses the following HMAT structures:
+ - Memory Proximity Domain Attributes Structure (Type 0)
+ - System Locality Latency and Bandwidth Info Structure (Type 1)
+ - Memory Side Cache Info structure (Type 2)
+
+ This function also performs validation of the ACPI table fields.
+
+ @param [in] Trace If TRUE, trace the ACPI fields.
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] AcpiTableLength Length of the ACPI table.
+ @param [in] AcpiTableRevision Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiHmat (
+ IN BOOLEAN Trace,
+ IN UINT8* Ptr,
+ IN UINT32 AcpiTableLength,
+ IN UINT8 AcpiTableRevision
+ )
+{
+ UINT32 Offset;
+ UINT8* HmatStructurePtr;
+
+ if (!Trace) {
+ return;
+ }
+
+ Offset = ParseAcpi (
+ Trace,
+ 0,
+ "HMAT",
+ Ptr,
+ AcpiTableLength,
+ PARSER_PARAMS (HmatParser)
+ );
+
+ HmatStructurePtr = Ptr + Offset;
+
+ while (Offset < AcpiTableLength) {
+ // Parse HMAT Structure Header to obtain Type and Length.
+ ParseAcpi (
+ FALSE,
+ 0,
+ NULL,
+ HmatStructurePtr,
+ AcpiTableLength - Offset,
+ PARSER_PARAMS (HmatStructureHeaderParser)
+ );
+
+ // Check if the values used to control the parsing logic have been
+ // successfully read.
+ if ((HmatStructureType == NULL) ||
+ (HmatStructureLength == NULL)) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Insufficient remaining table buffer length to read the " \
+ L"HMAT structure header. Length = %d.\n",
+ AcpiTableLength - Offset
+ );
+ return;
+ }
+
+ switch (*HmatStructureType) {
+ case
EFI_ACPI_6_3_HMAT_TYPE_MEMORY_PROXIMITY_DOMAIN_ATTRIBUTES: {
+ DumpMpda (
+ HmatStructurePtr,
+ *HmatStructureLength
+ );
+ break;
+ }
+
+ case
EFI_ACPI_6_3_HMAT_TYPE_SYSTEM_LOCALITY_LATENCY_AND_BANDWIDTH_I
NFO: {
+ DumpSllbi (
+ HmatStructurePtr,
+ *HmatStructureLength
+ );
+ break;
+ }
+
+ case EFI_ACPI_6_3_HMAT_TYPE_MEMORY_SIDE_CACHE_INFO: {
+ DumpMsci (
+ HmatStructurePtr,
+ *HmatStructureLength
+ );
+ break;
+ }
+
+ default: {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Unknown HMAT structure:"
+ L" Type = %d, Length = %d\n",
+ *HmatStructureType,
+ *HmatStructureLength
+ );
+ }

+ }
+
+ HmatStructurePtr += *HmatStructureLength;
+ Offset += *HmatStructureLength;
+ } // while
+}
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.c
index
d2f26ff89f12e596702281c38ab0de3729aa68e4..c9e3054a5c413ba95e6420ce3d
02f0d954e99d90 100644
---
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
+++ andLib.c
@@ -1,7 +1,7 @@
/** @file
Main file for 'acpiview' Shell command function.

- Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.<BR>
+ Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent **/

@@ -53,6 +53,7 @@ ACPI_TABLE_PARSER ParserList[] = {
{EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE,
ParseAcpiFacs},
{EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiFadt},
{EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
ParseAcpiGtdt},
+ {EFI_ACPI_6_3_HETEROGENEOUS_MEMORY_ATTRIBUTE_TABLE_SIGNATURE,
+ ParseAcpiHmat},
{EFI_ACPI_6_2_IO_REMAPPING_TABLE_SIGNATURE, ParseAcpiIort},
{EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
ParseAcpiMadt},

{EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BA
SE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.inf
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.inf
index
91459f9ec632635ee453c5ef46f67445cd9eee0c..e970c4259c143b5b06fb1a759d5
819e7cb93e749 100644
---
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.inf
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
+++ andLib.inf
@@ -1,7 +1,7 @@
## @file
# Provides Shell 'acpiview' command functions # -# Copyright (c) 2016 - 2020,
ARM Limited. All rights reserved.<BR>
+# Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -33,6 +33,7 @@
[Sources.common]
Parsers/Facs/FacsParser.c
Parsers/Fadt/FadtParser.c
Parsers/Gtdt/GtdtParser.c
+ Parsers/Hmat/HmatParser.c
Parsers/Iort/IortParser.c
Parsers/Madt/MadtParser.c
Parsers/Madt/MadtParser.h
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.uni
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.uni
index
7cd43d0518fd0a23dc547a5cab0d08b62602a113..6b0537b47a751184e8a68a653
0aa85eb28ea33ba 100644
---
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
b.uni
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
+++ andLib.uni
@@ -1,6 +1,6 @@
// /**
//
-// Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.<BR>
+// Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.<BR>
// SPDX-License-Identifier: BSD-2-Clause-Patent // // Module Name:
@@ -84,6 +84,7 @@
" DSDT - Differentiated System Description Table\r\n"
" FACP - Fixed ACPI Description Table (FADT)\r\n"
" GTDT - Generic Timer Description Table\r\n"
+" HMAT - Heterogeneous Memory Attributes Table\r\n"
" IORT - IO Remapping Table\r\n"
" MCFG - Memory Mapped Config Space Base Address Description
Table\r\n"
" PPTT - Processor Properties Topology Table\r\n"
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

-----邮件原件-----
发件人: bounce+27952+65339+4905953+8761045@groups.io
<bounce+27952+65339+4905953+8761045@groups.io> 代表 Laszlo Ersek
发送时间: 2020年9月17日 2:14
收件人: Dong, Guo <guo.dong@...>; devel@edk2.groups.io
抄送: marcello.bauer@...; Kinney, Michael D
<michael.d.kinney@...>; Leif Lindholm (Nuvia address)
<leif@...>; Doran, Mark <mark.doran@...>; Andrew Fish
<afish@...>; Guptha, Soumya K <soumya.k.guptha@...>
主题: Re: [edk2-devel] more development process failure [was:
UefiPayloadPkg: Runtime MMCONF]

On 09/16/20 19:30, Dong, Guo wrote:

Hi Laszlo,

The patchset includes 3 patches, and all of them had been reviewed by

package owners.

The patch submitter has a pull request

https://github.com/tianocore/edk2/pull/885, I rebased the patch to latest
master, and merged it by adding reviewed-by found from emails.

I also make sure it passed all the checks before I put "push" button there.

then retrigger a new build with "push" button.

I am not sure what is missing. If there is any other requirements, should

they be captured during code review or tool check?

- The description field of <https://github.com/tianocore/edk2/pull/932/>
is empty. It's difficult to tell where the patches come from -- where
they were posted and reviewed. A copy of the cover letter should have
been included here, plus preferably a link to the v5 mailing list thread
(the one that got merged in the end).

- It was not confirmed in the v5 mailing list thread that the series had
been merged. The confirmation should have included at least one of: (a)
the github PR link, (b) the git commit range. (Preferably: both.)

It's not the eventual git commits that I'm complaining about, but the
lack of communication with the community, and the lack of record for
posterity.

Myself, I used to consider github PRs a means merely for replacing our
earlier direct "git push" commands -- with a CI build + mergify. So, as
a maintainer, I would myself queue up several patch sets in a single
"batch" PR, add some links to BZs and the mailing list, and let it fly.
But then Mike told me this was really wrong, and we should clearly
associate any given PR with a specific patch set on the list.

This meant an *immense* workload increase for me, in particular because
I tend to merge patch sets for *other* people and subsystems too (after
they pass review), that is, for such subsystems that I do not
co-maintain. In particular during the feature freeze periods.

So what really rubs me the wrong way is that, if I am expected to keep
all of this meta-data nice and tidy, why aren't some other maintainers?
It's a double standard.

I can live with either *all of us* ignoring PR tidiness, or *all of us*
doing our best to keep everything nicely cross-referenced.

But right now I spend significant time and effort on keeping
communication and records complete and clean in *all three of* bugzilla,
github, and mailing list, whereas a good subset of the maintainers
couldn't care less in *either* of those communication channels.

For your reference, here's a random PR I submitted and merged for others:

https://github.com/tianocore/edk2/pull/904

Observe in PR#904:

- title carries cover letter subject
- description carries cover letter body
- description has a pointer to the BZ, and a link to the cover letter in
the mailing list archive (two links in fact, in different archives)

And then here's my report back on the list:

https://edk2.groups.io/g/devel/message/64644

And my BZ comment to the same effect (also closing the BZ as
RESOLVED|FIXED):

https://bugzilla.tianocore.org/show_bug.cgi?id=2376#c9
https://edk2.groups.io/g/bugs/message/12777


I don't insist on the particular information content of github PRs, as
-- at this stage -- they really are not more than just a way to set off
CI, before pushing/merging a series.

What I do insist on is that all of us maintainers (people with
permission to set the "push" label) be subject to the same expectations
when it comes to creating pull requests.

(Please note also that I absolutely don't need a BZ for every
contribution. My request is only that *if* there is a BZ, then handle it
thoroughly.)

Laszlo

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Wednesday, September 16, 2020 1:57 AM
To: Dong, Guo <guo.dong@...>
Cc: devel@edk2.groups.io; marcello.bauer@...; Kinney,

Michael D

<michael.d.kinney@...>; Leif Lindholm (Nuvia address)
<leif@...>; Doran, Mark <mark.doran@...>; Andrew Fish
<afish@...>; Guptha, Soumya K <soumya.k.guptha@...>
Subject: [edk2-devel] more development process failure [was:

UefiPayloadPkg:

Runtime MMCONF]

Guo,

On 08/18/20 10:24, Marcello Sylvester Bauer wrote:

Support arbitrary platforms with different or even no MMCONF space.
Fixes crash on platforms not exposing 256 buses.

Tested on:
* AMD Stoney Ridge

Branch: https://github.com/9elements/edk2-1/tree/UefiPayloadPkg-

MMCONF

PR: https://github.com/tianocore/edk2/pull/885

v5:
* MdePkg
- support variable size MMCONF in all PciExpressLibs
- use (UINTX)-1 as return values for invalid Pci addresses

Okay, so we got more of the same development process violations here, as
I've just reported at <https://edk2.groups.io/g/devel/message/65313>.

See this new pull request:

https://github.com/tianocore/edk2/pull/932/

"No description provided."

You should be embarrassed.

Laszlo

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@...>
Sent: Wednesday, September 16, 2020 18:34
To: Zurcher, Christopher J <christopher.j.zurcher@...>; Laszlo Ersek <lersek@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Subject: RE: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface

Thanks to ask this question Christopher.

When we create the CryptoPkg, our goal is to pick openssl as implementation choice. We don’t want to mandate any
EDKII consumer use openssl.

The EDKII consumer shall have freedom to choose whatever crypto library they want to use. Even Intel IPP and mebdTLS
are options. A vendor may choose a GPL wolfssl, or vendor proprietary close source implementation. We choose openssl
just because it is the BSD license and it is used widely when we make decision.

That is why we spend effort to design BaseCryptoLib API to abstract the crypto action, instead of calling openssl
function directly in our code.
Whenever we add new API, we need evaluate if it is applicable for any other crypto implantation and have negative
impact.

That assumption that EDKII must link openssl is NOT our original intent.
Feel free to drop the patch or reject the Bugzilla, if you want.

Thank you
Yao Jiewen

-----Original Message-----
From: Zurcher, Christopher J <christopher.j.zurcher@...>
Sent: Thursday, September 17, 2020 9:21 AM
To: Laszlo Ersek <lersek@...>; Yao, Jiewen <jiewen.yao@...>;
devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Subject: RE: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP
(Envelope) Digest interface

Jiewen,

This patch set assumes that EDK2 is linked with OpenSSL for the foreseeable
future. The end goal would be to move all applicable Crypto access to the EVP
interface and remove the individual modules we maintain for each algorithm.
The primary benefit here is reducing complexity and code duplication. Without
this end goal, this patch set will not be particularly useful.

If the design goal of BaseCryptLib is to allow OpenSSL to be replaced by other
crypto providers, I should abandon this patch set, and we can save the EVP
transition for when moving to OpenSSL 3 becomes mandatory. At that time, the
CryptX.c family can be modified to call EVP functions.
(This could even be done transparently, by returning UINTN from GetContextSize
and using the user-supplied "context" to store the OpenSSL-supplied context
pointer.)
Delaying EVP adoption in this case would also allow more time for platforms to
adopt the Crypto Services model, which should help offset the size impact.

Please let me know which path should be taken.

Thanks,
Christopher Zurcher

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Wednesday, September 16, 2020 09:05
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io; Zurcher,

Christopher J

<christopher.j.zurcher@...>
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP

(Envelope) Digest interface

On 09/16/20 16:17, Yao, Jiewen wrote:

Thank you Laszlo.
You forced me to read the code again and more carefully. :-)

I believe I understand why you use NULL to free the context now - to support

error path.

First, I did have some new thought for EVP API. Let’s consider 3 cases, 1)

Hash all data one time, 2) Hash update

data multiple times, 3) Error during update.

A. In current design, the API sequence is:
1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR, EvpMdFinal (NULL)

B. We can auto free context in EvpMdUpdate in error path - the API

sequence is:

1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR

I don't like "B" because in the loop body where you call EvpMdUpdate(),
you may encounter an error from a different source than EvpMdUpdate()
itself. For example, you may be reading data from the network or a disk
file. If fetching the next chunk fails, we'd still want to clean up the
EVP MD context. Therefore, if EvpMdUpdate() would itself invalidate the
context, if it failed, then that would *complicate* the error path. (=
Clean up the context after the loop body *only* if something different
from EvpMdUpdate() failed.)

C: We can use New/Free style - similar to HMAC
1) EvpMdHashAll (Digest)
2) EvpMdNew, EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal

(Digest), EvpMdFree

3) EvpMdNew, EvpMdInit, EvpMdUpdate->ERROR, EvpMdFree

Yes, this was the first pattern, the one that caused me to say "please
don't do this". In this pattern, the context may exist between "New" and
"Init", and also between "Final" and "Free". Those two states are
useless and only good for causing confusion.

For example, are you allowed to Duplicate a context in those states?
It's best to prevent even the *asking* of that question.

I checked below APIs:
1) openssl (EVP_MD_CTX_new, EVP_DigestInit_ex, EVP_DigestUpdate,

EVP_DigestFinal_ex, EVP_MD_CTX_free)

2) mbedtls (mbedtls_sha256_init, mbedtls_sha256_starts_ret,

mbedtls_sha256_update_ret, mbedtls_sha256_finish_ret,

mbedtls_sha256_free)

3) BaseCryptoLib HMAC (HmacSha256New, HmacSha256SetKey,

HmacSha256Update, HmacSha256Final, HmacSha256Free)

All APIs include free function to free the context, I don’t think it is a bad idea

to have EvpMdFree() API.

I would like to propose option - C.

- I cannot comment on mbedtls (2).

- I think the current BaseCryptLib HMAC APIs (3) are not great, and we
should use this opportunity to improve upon them.

- Regarding openssl (1), as I understand it, the situation is different
from edk2. In OpenSSL, EVP_MD_CTX is *not* an opaque type (expressed in
C language terms, it is not an "incomplete" type, but a "complete"
type). Therefore, an expression such as sizeof(EVP_MD_CTX) works.

The consequence is that OpenSSL code itself can use the following style:

void func(void)
{
EVP_MD_CTX ctx;

EVP_DigestInit_ex(&ctx, ...);
...
EVP_DigestFinal_ex(&ctx, ...)
}

In other words, OpenSSL-native code is permitted to know the internals
of EVP_MD_CTX, therefore the OpenSSL-internal code may allocate
EVP_MD_CTX with means that are *different* from EVP_MD_CTX_new() and
EVP_MD_CTX_free().

The caller of (Init, Final) may use (New, Free) for memory management,
or may use something completely different (for example, local variables).

Therefore, offering the (Init, Final) APIs separately from (New, Free)
is meaningful.

But the situation in edk2 -- or any other OpenSSL *consumer* -- is
different. In edk2, EVP_MD_CTX is an opaque type -- in C language terms:
an "incomplete" type. OpenSSL deliberately hides the internals of
EVP_MD_CTX.

See "CryptoPkg/Library/OpensslLib/openssl/include/openssl/ossl_typ.h":

typedef struct evp_md_ctx_st EVP_MD_CTX;

and the "evp_md_ctx_st" structure type is only defined in
"CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_local.h", whose
contents -- I think! -- OpenSSL client code cannot, or *should not*,
refer to.

This means that OpenSSL consumer code can *only* rely on
EVP_MD_CTX_new() and EVP_MD_CTX_free(), for allocating and releasing the
context, respectively. "sizeof(EVP_MD_CTX)" is not permitted, and
defining a local or global variable of type EVP_MD_CTX is also not valid.

This means that the only reason for separating (Init, Final) from (New,
Free) falls away, in OpenSSL consumer code. In OpenSSL consumer code,
there is no reason to keep *both* pairs of APIs.


Please note that, this (very prudent) information hiding / encapsulation
in OpenSSL used to be violated by edk2 in the past, intentionally.
That's what the HmacXxxGetContextSize() APIs were about -- those APIs
forcefully leaked the context sizes to client code, so that client code
in edk2 could perform its own allocation.

But in <https://bugzilla.tianocore.org/show_bug.cgi?id=1792>, we finally
eliminated HmacXxxGetContextSize(). As a part of that, we also
simplified the HMAC API -- note that Init was replaced by SetKey. And
because we were reworking an existent API, I didn't propose very
intrusive changes -- I didn't propose fusing Final and Free, for example.

Now, the EVP MD APIs are just being introduced to edk2. So we have a
chance at getting them right, and making them minimal.

Second, I believe EVP is just something in openssl. It does not mean that we

*have to* design API to follow

openssl.

After rethink the size impact, I do have concern to merge all Hash

implementation together in one function.

It might means nothing if the crypto library is based upon openssl.
But if the cryptolib implementation is based upon other crypto, such as Intel

IPP

(https://github.com/slimbootloader/slimbootloader/tree/master/BootloaderCo
mmonPkg/Library/IppCryptoLib) or mbedTls

(https://github.com/jyao1/edk2/tree/DeviceSecurity/CryptoMbedTlsPkg), then

we can NOT get size benefit by separating

the hash algorithm.

I would like to propose separate EvpMdxxx.
EvpMdNew -> Sha256New, Sha384New
EvpMdInit -> Sha256Init, Sha384Init
EvpMdUpdate -> Sha256Update, Sha384Update
EvpMdFinal -> Sha256Final, Sha384Final
EvpMdFree -> Sha256Free, Sha384Free

I have no comment on this.

We can do similar thing with Hmac, to deprecate Sha256GetContextSize()

API,

Yes, good idea.

and replace caller with Sha256New() / Sha384Free()

Indeed.

But then -- there's no reason left for keeping (New, Free) separate from
(Init, Final).

Thanks
Laszlo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Wednesday, September 16, 2020 4:29 PM
To: devel@edk2.groups.io; marcello.bauer@...
Subject: Re: [edk2-devel] [PATCH v2 0/2] Add support for scanning Option ROMs

Why running it will disable the ability of PciPlatform code to scan for ROMs?

I guess it is because the PciIoDevice->AllOpRomProcessed is set which causes GetPciRom() is skipped.

Can you explain more in the code comment?

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marcello
Sylvester Bauer
Sent: Tuesday, September 15, 2020 8:26 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH v2 0/2] Add support for scanning Option ROMs

Fix Option ROM enumeration and support scanning.

v2:
* add correct Maintainer and Reviewer to Cc
* PciPlatformDxe:
- Update description
- add function description

Branch: https://github.com/9elements/edk2-1/tree/UefiPayloadPkg-
Option_ROMs
PR: https://github.com/tianocore/edk2/pull/926

Patrick Rudolph (2):
MdeModulePkg: Fix OptionROM scanning
UefiPayloadPkg: Scan for Option ROMs

UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 1 +
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 1 +
UefiPayloadPkg/UefiPayloadPkg.fdf | 1 +
UefiPayloadPkg/PciPlatformDxe/PciPlatformDxe.inf | 46 +++
UefiPayloadPkg/PciPlatformDxe/PciPlatformDxe.h | 19 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 10 +-
UefiPayloadPkg/PciPlatformDxe/PciPlatformDxe.c | 426
++++++++++++++++++++
7 files changed, 500 insertions(+), 4 deletions(-)
create mode 100644 UefiPayloadPkg/PciPlatformDxe/PciPlatformDxe.inf
create mode 100644 UefiPayloadPkg/PciPlatformDxe/PciPlatformDxe.h
create mode 100644 UefiPayloadPkg/PciPlatformDxe/PciPlatformDxe.c

--
2.28.0

-----Original Message-----
From: Zurcher, Christopher J <christopher.j.zurcher@...>
Sent: Thursday, September 17, 2020 9:21 AM
To: Laszlo Ersek <lersek@...>; Yao, Jiewen <jiewen.yao@...>;
devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Subject: RE: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP
(Envelope) Digest interface

Jiewen,

This patch set assumes that EDK2 is linked with OpenSSL for the foreseeable
future. The end goal would be to move all applicable Crypto access to the EVP
interface and remove the individual modules we maintain for each algorithm.
The primary benefit here is reducing complexity and code duplication. Without
this end goal, this patch set will not be particularly useful.

If the design goal of BaseCryptLib is to allow OpenSSL to be replaced by other
crypto providers, I should abandon this patch set, and we can save the EVP
transition for when moving to OpenSSL 3 becomes mandatory. At that time, the
CryptX.c family can be modified to call EVP functions.
(This could even be done transparently, by returning UINTN from GetContextSize
and using the user-supplied "context" to store the OpenSSL-supplied context
pointer.)
Delaying EVP adoption in this case would also allow more time for platforms to
adopt the Crypto Services model, which should help offset the size impact.

Please let me know which path should be taken.

Thanks,
Christopher Zurcher

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Wednesday, September 16, 2020 09:05
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io; Zurcher,

Christopher J

<christopher.j.zurcher@...>
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP

(Envelope) Digest interface

On 09/16/20 16:17, Yao, Jiewen wrote:

Thank you Laszlo.
You forced me to read the code again and more carefully. :-)

I believe I understand why you use NULL to free the context now - to support

error path.

First, I did have some new thought for EVP API. Let’s consider 3 cases, 1)

Hash all data one time, 2) Hash update

data multiple times, 3) Error during update.

A. In current design, the API sequence is:
1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR, EvpMdFinal (NULL)

B. We can auto free context in EvpMdUpdate in error path - the API

sequence is:

1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR

I don't like "B" because in the loop body where you call EvpMdUpdate(),
you may encounter an error from a different source than EvpMdUpdate()
itself. For example, you may be reading data from the network or a disk
file. If fetching the next chunk fails, we'd still want to clean up the
EVP MD context. Therefore, if EvpMdUpdate() would itself invalidate the
context, if it failed, then that would *complicate* the error path. (=
Clean up the context after the loop body *only* if something different
from EvpMdUpdate() failed.)

C: We can use New/Free style - similar to HMAC
1) EvpMdHashAll (Digest)
2) EvpMdNew, EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal

(Digest), EvpMdFree

3) EvpMdNew, EvpMdInit, EvpMdUpdate->ERROR, EvpMdFree

Yes, this was the first pattern, the one that caused me to say "please
don't do this". In this pattern, the context may exist between "New" and
"Init", and also between "Final" and "Free". Those two states are
useless and only good for causing confusion.

For example, are you allowed to Duplicate a context in those states?
It's best to prevent even the *asking* of that question.

I checked below APIs:
1) openssl (EVP_MD_CTX_new, EVP_DigestInit_ex, EVP_DigestUpdate,

EVP_DigestFinal_ex, EVP_MD_CTX_free)

2) mbedtls (mbedtls_sha256_init, mbedtls_sha256_starts_ret,

mbedtls_sha256_update_ret, mbedtls_sha256_finish_ret,

mbedtls_sha256_free)

3) BaseCryptoLib HMAC (HmacSha256New, HmacSha256SetKey,

HmacSha256Update, HmacSha256Final, HmacSha256Free)

All APIs include free function to free the context, I don’t think it is a bad idea

to have EvpMdFree() API.

I would like to propose option - C.

- I cannot comment on mbedtls (2).

- I think the current BaseCryptLib HMAC APIs (3) are not great, and we
should use this opportunity to improve upon them.

- Regarding openssl (1), as I understand it, the situation is different
from edk2. In OpenSSL, EVP_MD_CTX is *not* an opaque type (expressed in
C language terms, it is not an "incomplete" type, but a "complete"
type). Therefore, an expression such as sizeof(EVP_MD_CTX) works.

The consequence is that OpenSSL code itself can use the following style:

void func(void)
{
EVP_MD_CTX ctx;

EVP_DigestInit_ex(&ctx, ...);
...
EVP_DigestFinal_ex(&ctx, ...)
}

In other words, OpenSSL-native code is permitted to know the internals
of EVP_MD_CTX, therefore the OpenSSL-internal code may allocate
EVP_MD_CTX with means that are *different* from EVP_MD_CTX_new() and
EVP_MD_CTX_free().

The caller of (Init, Final) may use (New, Free) for memory management,
or may use something completely different (for example, local variables).

Therefore, offering the (Init, Final) APIs separately from (New, Free)
is meaningful.

But the situation in edk2 -- or any other OpenSSL *consumer* -- is
different. In edk2, EVP_MD_CTX is an opaque type -- in C language terms:
an "incomplete" type. OpenSSL deliberately hides the internals of
EVP_MD_CTX.

See "CryptoPkg/Library/OpensslLib/openssl/include/openssl/ossl_typ.h":

typedef struct evp_md_ctx_st EVP_MD_CTX;

and the "evp_md_ctx_st" structure type is only defined in
"CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_local.h", whose
contents -- I think! -- OpenSSL client code cannot, or *should not*,
refer to.

This means that OpenSSL consumer code can *only* rely on
EVP_MD_CTX_new() and EVP_MD_CTX_free(), for allocating and releasing the
context, respectively. "sizeof(EVP_MD_CTX)" is not permitted, and
defining a local or global variable of type EVP_MD_CTX is also not valid.

This means that the only reason for separating (Init, Final) from (New,
Free) falls away, in OpenSSL consumer code. In OpenSSL consumer code,
there is no reason to keep *both* pairs of APIs.


Please note that, this (very prudent) information hiding / encapsulation
in OpenSSL used to be violated by edk2 in the past, intentionally.
That's what the HmacXxxGetContextSize() APIs were about -- those APIs
forcefully leaked the context sizes to client code, so that client code
in edk2 could perform its own allocation.

But in <https://bugzilla.tianocore.org/show_bug.cgi?id=1792>, we finally
eliminated HmacXxxGetContextSize(). As a part of that, we also
simplified the HMAC API -- note that Init was replaced by SetKey. And
because we were reworking an existent API, I didn't propose very
intrusive changes -- I didn't propose fusing Final and Free, for example.

Now, the EVP MD APIs are just being introduced to edk2. So we have a
chance at getting them right, and making them minimal.

Second, I believe EVP is just something in openssl. It does not mean that we

*have to* design API to follow

openssl.

After rethink the size impact, I do have concern to merge all Hash

implementation together in one function.

It might means nothing if the crypto library is based upon openssl.
But if the cryptolib implementation is based upon other crypto, such as Intel

IPP

(https://github.com/slimbootloader/slimbootloader/tree/master/BootloaderCo
mmonPkg/Library/IppCryptoLib) or mbedTls

(https://github.com/jyao1/edk2/tree/DeviceSecurity/CryptoMbedTlsPkg), then

we can NOT get size benefit by separating

the hash algorithm.

I would like to propose separate EvpMdxxx.
EvpMdNew -> Sha256New, Sha384New
EvpMdInit -> Sha256Init, Sha384Init
EvpMdUpdate -> Sha256Update, Sha384Update
EvpMdFinal -> Sha256Final, Sha384Final
EvpMdFree -> Sha256Free, Sha384Free

I have no comment on this.

We can do similar thing with Hmac, to deprecate Sha256GetContextSize()

API,

Yes, good idea.

and replace caller with Sha256New() / Sha384Free()

Indeed.

But then -- there's no reason left for keeping (New, Free) separate from
(Init, Final).

Thanks
Laszlo

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Wednesday, September 16, 2020 09:05
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io; Zurcher, Christopher J
<christopher.j.zurcher@...>
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>
Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface

On 09/16/20 16:17, Yao, Jiewen wrote:

Thank you Laszlo.
You forced me to read the code again and more carefully. :-)

I believe I understand why you use NULL to free the context now - to support error path.

First, I did have some new thought for EVP API. Let’s consider 3 cases, 1) Hash all data one time, 2) Hash update

data multiple times, 3) Error during update.

A. In current design, the API sequence is:
1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR, EvpMdFinal (NULL)

B. We can auto free context in EvpMdUpdate in error path - the API sequence is:
1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR

I don't like "B" because in the loop body where you call EvpMdUpdate(),
you may encounter an error from a different source than EvpMdUpdate()
itself. For example, you may be reading data from the network or a disk
file. If fetching the next chunk fails, we'd still want to clean up the
EVP MD context. Therefore, if EvpMdUpdate() would itself invalidate the
context, if it failed, then that would *complicate* the error path. (=
Clean up the context after the loop body *only* if something different
from EvpMdUpdate() failed.)

C: We can use New/Free style - similar to HMAC
1) EvpMdHashAll (Digest)
2) EvpMdNew, EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest), EvpMdFree
3) EvpMdNew, EvpMdInit, EvpMdUpdate->ERROR, EvpMdFree

Yes, this was the first pattern, the one that caused me to say "please
don't do this". In this pattern, the context may exist between "New" and
"Init", and also between "Final" and "Free". Those two states are
useless and only good for causing confusion.

For example, are you allowed to Duplicate a context in those states?
It's best to prevent even the *asking* of that question.

I checked below APIs:
1) openssl (EVP_MD_CTX_new, EVP_DigestInit_ex, EVP_DigestUpdate, EVP_DigestFinal_ex, EVP_MD_CTX_free)
2) mbedtls (mbedtls_sha256_init, mbedtls_sha256_starts_ret, mbedtls_sha256_update_ret, mbedtls_sha256_finish_ret,

mbedtls_sha256_free)

3) BaseCryptoLib HMAC (HmacSha256New, HmacSha256SetKey, HmacSha256Update, HmacSha256Final, HmacSha256Free)

All APIs include free function to free the context, I don’t think it is a bad idea to have EvpMdFree() API.
I would like to propose option - C.

- I cannot comment on mbedtls (2).

- I think the current BaseCryptLib HMAC APIs (3) are not great, and we
should use this opportunity to improve upon them.

- Regarding openssl (1), as I understand it, the situation is different
from edk2. In OpenSSL, EVP_MD_CTX is *not* an opaque type (expressed in
C language terms, it is not an "incomplete" type, but a "complete"
type). Therefore, an expression such as sizeof(EVP_MD_CTX) works.

The consequence is that OpenSSL code itself can use the following style:

void func(void)
{
EVP_MD_CTX ctx;

EVP_DigestInit_ex(&ctx, ...);
...
EVP_DigestFinal_ex(&ctx, ...)
}

In other words, OpenSSL-native code is permitted to know the internals
of EVP_MD_CTX, therefore the OpenSSL-internal code may allocate
EVP_MD_CTX with means that are *different* from EVP_MD_CTX_new() and
EVP_MD_CTX_free().

The caller of (Init, Final) may use (New, Free) for memory management,
or may use something completely different (for example, local variables).

Therefore, offering the (Init, Final) APIs separately from (New, Free)
is meaningful.

But the situation in edk2 -- or any other OpenSSL *consumer* -- is
different. In edk2, EVP_MD_CTX is an opaque type -- in C language terms:
an "incomplete" type. OpenSSL deliberately hides the internals of
EVP_MD_CTX.

See "CryptoPkg/Library/OpensslLib/openssl/include/openssl/ossl_typ.h":

typedef struct evp_md_ctx_st EVP_MD_CTX;

and the "evp_md_ctx_st" structure type is only defined in
"CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_local.h", whose
contents -- I think! -- OpenSSL client code cannot, or *should not*,
refer to.

This means that OpenSSL consumer code can *only* rely on
EVP_MD_CTX_new() and EVP_MD_CTX_free(), for allocating and releasing the
context, respectively. "sizeof(EVP_MD_CTX)" is not permitted, and
defining a local or global variable of type EVP_MD_CTX is also not valid.

This means that the only reason for separating (Init, Final) from (New,
Free) falls away, in OpenSSL consumer code. In OpenSSL consumer code,
there is no reason to keep *both* pairs of APIs.


Please note that, this (very prudent) information hiding / encapsulation
in OpenSSL used to be violated by edk2 in the past, intentionally.
That's what the HmacXxxGetContextSize() APIs were about -- those APIs
forcefully leaked the context sizes to client code, so that client code
in edk2 could perform its own allocation.

But in <https://bugzilla.tianocore.org/show_bug.cgi?id=1792>, we finally
eliminated HmacXxxGetContextSize(). As a part of that, we also
simplified the HMAC API -- note that Init was replaced by SetKey. And
because we were reworking an existent API, I didn't propose very
intrusive changes -- I didn't propose fusing Final and Free, for example.

Now, the EVP MD APIs are just being introduced to edk2. So we have a
chance at getting them right, and making them minimal.

Second, I believe EVP is just something in openssl. It does not mean that we *have to* design API to follow

openssl.

After rethink the size impact, I do have concern to merge all Hash implementation together in one function.
It might means nothing if the crypto library is based upon openssl.
But if the cryptolib implementation is based upon other crypto, such as Intel IPP

(https://github.com/slimbootloader/slimbootloader/tree/master/BootloaderCommonPkg/Library/IppCryptoLib) or mbedTls
(https://github.com/jyao1/edk2/tree/DeviceSecurity/CryptoMbedTlsPkg), then we can NOT get size benefit by separating
the hash algorithm.

I would like to propose separate EvpMdxxx.
EvpMdNew -> Sha256New, Sha384New
EvpMdInit -> Sha256Init, Sha384Init
EvpMdUpdate -> Sha256Update, Sha384Update
EvpMdFinal -> Sha256Final, Sha384Final
EvpMdFree -> Sha256Free, Sha384Free

I have no comment on this.

We can do similar thing with Hmac, to deprecate Sha256GetContextSize() API,

Yes, good idea.

and replace caller with Sha256New() / Sha384Free()

Indeed.

But then -- there's no reason left for keeping (New, Free) separate from
(Init, Final).

Thanks
Laszlo