Date   

Re: [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Laszlo Ersek
 

Hi Jiewen,

(+Marvin, +Vitaly)

On 08/18/20 01:23, Yao, Jiewen wrote:
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Tuesday, August 18, 2020 12:53 AM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
xiewenyi2@...; Wang, Jian J <jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
[...]

However, I do think the producer is mandatory for a fix or at least a
security fix.
The owner to fix the issue should guarantee the patch is good.
The owner shall never rely on the code reviewer to figure out if the
patch is good and complete.

I have some bad experience that bug owner just wrote a patch and tried
to fix a problem, without any test.
And it happened passed code review from someone who does not well
understand the problem, but give rb based upon the time pressure.
Later, the fix was approved to be useless.

In my memory, at least 3 cases were security fix. They are found, just
because they are sensitive, more people took a look later.
It was simple. It was one-line change.
But it has not test, and it was wrong.
"It was ridiculous" -- commented by the people who find the so-called
security fix does not fix the issue.
Just because sloppy/rushed reviews exist, and just because reviewers
operate under time pressure, we should not automatically reject security
fixes that come without a reproducer.

Some organizations do develop reproducers, but they never share them
publicly (for fear of abuse by others).

But more importantly, in an open development project, a developer could
have time and expertise to contribute a fix, but not to create a
reproducer.

- If we make contributing harder, fewer people will upstream their
fixes.

- If we make contributing harder, then contributions that do make it to
the tree will be of higher quality.

Both statements ring true to me -- so it's a tradeoff.

(By "we", I mean the edk2 community.)

Additionally, the exact statement that the bug report does make,
namely

it's possible to overflow Offset back to 0 causing an endless loop

is wrong (as far as I can tell anyway). It is not "OffSet" that can
be overflowed to zero, but the *addend* that is added to OffSet can
be overflowed to zero. Therefore the infinite loop will arise because
OffSet remains stuck at its present value, and not because OffSet
will be re-set to zero.

For the reasons, we can only speculate as to what the actual problem
is, unless Jian decides to join the discussion and clarifies what he
had in mind originally.
[Jiewen] Would you please clarify what do you mean "we" here?
If "we" means the bug dispatcher, it is totally OK. The dispatcher
just assign the bug.
If "we" means the developer assigned to fix the bug, it is NOT OK. The
developer should take the responsibility to understand the problem.
By "we", I mean the edk2 community.

We can write a patch based on code analysis. It's possible to
identify integer overflows based on code analysis, and it's possible
to verify the correctness of fixes by code review. Obviously testing
is always good, but many times, constructing reproducers for such
issues that were found by code review, is difficult and time
consuming. We can say that we don't fix vulnerabilities without
reproducers, or we can say that we make an effort to fix them even if
all we have is code analysis (and not a reproducer).
[Jiewen] I would say: yes and no.
Yes, I agree with you that it might be difficult and time consuming to
construct the reproducer.
However, "obviously" is a subject term. Someone may think something is
obvious, but other people does not.
We should be clear the responsibility of the patch provider is to
provide high quality patch.
Having basic unit test is the best way to prove that the fix is good.

I have seen bad cases when I ask for the test for patch, then the
answer I got is: "I test the windows boot".
But the test - windows boot - has nothing related to the patch. It
only proves no regression, but cannot prove the issue described is
resolved.
Right. It would be ideal if every patch came with a unit test. But that
also means some folks will contribute less.

Consider normal (not security) patches. We require that all function
return values be checked (unless it really doesn't matter if a function
call fails). If a function call fails, we need to roll back the actions
taken thus far. Release resources and so on. This is why we have the
"cascade of error handling labels" pattern.

But, of course, we don't test every possible error path in the code. So
what's the solution there:

- reject such patches that carefully construct the error paths, but do
not provide unit tests with complete error path coverage?

- say that we don't care about thorough error paths, so let's just hang,
or leak resources, whenever something fails?

Personally I prefer the detailed error paths. They need to be written
and reviewed carefully. And they can be accepted even if they are not
tested with complete coverage.

Some people think otherwise; they say no untested (untestable) code
should ever be merged.

Back to security patches -- creating reproducers usually requires a
setup (tools, expertise, time allocation etc) that is different from a
"normal" setup. It may require specialized binary format editors,
expertise in "penetration testing", and so on.

I mostly know the C language rules that pertain to integer and buffer
overflows, so I can usually spot their violations in C code, and propose
fixes for them too. But I'm not a security researcher, so I don't write
exploits as a norm -- I don't even investigate, generally speaking, the
potential practical impact of "undefined behavior". When there's a
buffer overflow or integer overflow in the code, that's the *end* of the
story for me, while it's the *start* of the work for a security
researcher.

When you require reproducers for all security patches, you restrict the
potential contributor pool to security researchers.

Let's think again in this case, if the patch provider does some basic
unit test, he/she may find out the problem by himself/herself.
That can save other people's time to review.

I don't prefer to move the responsibility from patch provider to the
code reviewer to check if the fix is good.
Otherwise, the code reviewer may be overwhelmed.

We may clarify and document the role and responsibility in EDKII
clearly. Once that is ready, we can follow the rule.
Before that is ready, in this particular case, I still prefer we have
producer to prove the patch is good.
OK, thanks for explaining.

Given that I'm unable to create such a PE file (from scratch or by
modifying another one), I won't post the patches stand-alone.

So the above paragraph concerns "correctness". Regarding
"completeness", I guarantee you that this patch does not fix *all*
problems related to PE parsing. (See the other BZ tickets.) It does
fix *one* issue with PE parsing. We can say that we try to fix such
issues gradually (give different CVE numbers to different issues, and
address them one at a time), or we can say that we rewrite PE parsing
from the ground up. (BTW: I have seriously attempted that in the
past, and I gave up, because the PE format is FUBAR.)
[Jiewen] Maybe there is misunderstanding.
I do not mean to let patch provider to fix all issue in PE parsing.
Just like we cannot file one Bugzilla to fix all issue in EDKII - it
is unfair.

What I mean is that the patch provider should guarantee the
correctness and completeness of the issue in the bug report.

One faked bad example of correctness is:
A bug report is file to say: the code has overflow class A.
The factor is: the code has overflow class A at line X and line Y.
The patch only modified some code at line X, but the overflow
class A at line X still exists.

One faked bad example of completeness is:
A bug report is file to say: the code has overflow class A.
The factor is: the code has overflow class A at line X and line Y.
The patch only fixed the overflow class A at line X but not line
Y.

The patch provider should take responsibility to do that work
seriously to find out issue in line X and line Y and fix them.
He/she may choose to just fix line X and line Y. Rewrite is whole
module is NOT required.
I agree completely.

My point was that we need the bug report to be precise, in the first
place. If the bug report doesn't clearly identify lines X and Y, we will
likely not get the completeness part right.

"Clearly identify" may mean spelling out lines X and Y specifically. Or
it may mean defining "class A" sufficiently clearly that someone else
reading the affected function can find X and Y themselves.

If I can give some comment, I would think about the provide the fix in
BasePeCoffLib.
From a software design perspective, you are 100% right.

Unfortunately, I can't do it.

That's what I mentioned before -- I had tried rewriting BasePeCoffLib,
because in my opinion, BasePeCoffLib is unsalvageable in its current
form. And I gave up on the rewrite.

Please see the following email. I sent it to some people off-list, on
2020-Feb-14:

There are currently four (4) TianoCore security BZs (1957, 1990, 1993,
2215), embargoed, that describe various ways in which cunningly
crafted PE images can evade Secure Boot verification.

[...]

Primarily, I just couldn't find my peace with the idea that fixing
such PE/COFF parsing mistakes (integer overflows, buffer overflows)
*must* be a one-by-one fixing game. I wanted an approach that would
fix these *classes* of vulnerabilities, in PE/COFF parsing.

So, beginnning of this February I returned to this topic, and spent
two days on prototyping / researching a container / interval based
approach. Here's one of the commit messages, as a way of explaining:

OvmfPkg/DxePeCoffValidatorLib: introduce CONTAINER type and helper funcs

For validating the well-formedness of a PE/COFF file, introduce the
CONTAINER type, and some workhorse functions. (The functions added in this
patch will not be called directly from the code that will process PE/COFF
structures.)

The CONTAINER type describes a contiguous non-empty interval in a PE/COFF
file (on-disk representation, or in-memory representation). Containers can
be nested. The data from scalar-sized containers can be read out, as part
of their creation. For on-disk representations of PE/COFF files, scalar
reads are permitted; for in-memory representations, no data access is
permitted (only CONTAINER tracking / nesting).

The goals of CONTAINER are the following:

- enforce the proper nesting of PE/COFF structures (structure boundaries
must not be crossed by runs of data);

- prevent integer overflows and buffer overflows;

- prevent zero-size structures;

- prevent infinite nesting by requiring proper sub-intervals;

- prevent internal PE/COFF pointers from aliasing each other (unless they
point at container and containee structures);

- terminate nesting at scalar-sized containers;

- assuming an array of pointers is processed in increasing element order,
enforce that the pointed-to objects are located at increasing offsets
too;

- assign human-readable names to PE/COFF structures and fields, for
debugging PE/COFF malformations.

Because, several of the vulnerabilities exploited cross-directed and
aliased internal pointers in PE/COFF files.

Two days of delirious spec reading and coding later, and 2000+ lines
later, I decided that my idea was unviable. The PE/COFF spec was so
incredibly mis-designed and crufty that enforcing the *internal*
consistency of all the size fields and the internal pointers would
inevitably fall into one of the following categories:

- the checks wouldn't be strict enough, and some nasty images would
slip through,

- the checks would be too strict, and some quirky, but valid, images
would be unjustifiedly caught.

So I gave up and I've accepted that it remains a whack-a-mole game.
[...]

(NB: I don't claim that ELF is not similarly brain-damaged.)
So now, I've only considered contributing patches for bug#2215 because
the code in question resides in DxeImageVerificationLib, and *not* in
BasePeCoffLib. I'm not going to touch BasePeCoffLib -- in my opinion,
BasePeCoffLib is unfixable without a complete rewrite.

I would *like* if BasePeCoffLib were fixable incrementally, but I just
don't see how that's possible.

In support of my opinion, please open the following bugzilla ticket:

https://bugzilla.tianocore.org/show_bug.cgi?id=2643

and search the comments (with the browser's in-page search feature, such
as Ctrl+F) for the following expression:

new PE loader

I understand exactly what Vitaly and Marvin meant in those comments. :(

Thanks,
Laszlo


Re: reg: Wifi Connection Manager Setup Page Issues

Sivaraman Nainar
 

Hello Dandan:

Attached patch resolve the Issue. Can you please provide your comments.

Left base folder: E:\Work\NWStack\EDK2Tickets\WifiPassword\MOD

Right base folder: E:\Work\NWStack\EDK2Tickets\WifiPassword\ORG

*** WifiConnectionMgrHiiConfigAccess.c            2020-08-18 10:21:27.000000000 +05-30

--- WifiConnectionMgrHiiConfigAccess.c               2020-05-07 22:54:46.000000000 +05-30

***************

*** 365,378 ****

                                   Hii form.

 

  **/

  EFI_STATUS

  WifiMgrRefreshNetworkList (

    IN    WIFI_MGR_PRIVATE_DATA      *Private,

!   OUT   WIFI_MANAGER_IFR_NVDATA    *IfrNvData,

!   OUT   WIFI_MGR_NETWORK_PROFILE   *ProfileData

    )

  {

    EFI_STATUS                         Status;

    EFI_TPL                            OldTpl;

    UINT32                             AvailableCount;

    EFI_STRING_ID                      PortPromptToken;

--- 365,377 ----

                                   Hii form.

 

  **/

  EFI_STATUS

  WifiMgrRefreshNetworkList (

    IN    WIFI_MGR_PRIVATE_DATA      *Private,

!   OUT   WIFI_MANAGER_IFR_NVDATA    *IfrNvData

    )

  {

    EFI_STATUS                         Status;

    EFI_TPL                            OldTpl;

    UINT32                             AvailableCount;

    EFI_STRING_ID                      PortPromptToken;

***************

*** 663,675 ****

      FreePool (CipherListDisplay);

    }

 

    HiiFreeOpCodeHandle (StartOpCodeHandle);

    HiiFreeOpCodeHandle (EndOpCodeHandle);

 

-   ProfileData->SecurityType= Profile->SecurityType;

    DEBUG ((DEBUG_INFO, "[WiFi Connection Manager] Network List is Refreshed!\n"));

    return Status;

  }

 

  /**

    Refresh the hidden network list configured by user.

--- 662,673 ----

***************

*** 1330,1342 ****

    EFI_INPUT_KEY                      Key;

    UINTN                              BufferSize;

    WIFI_MGR_PRIVATE_DATA              *Private;

    WIFI_MANAGER_IFR_NVDATA            *IfrNvData;

    EFI_DEVICE_PATH_PROTOCOL           *FilePath;

    WIFI_MGR_NETWORK_PROFILE           *Profile;

-   WIFI_MGR_NETWORK_PROFILE                               *ProfileData;

    WIFI_MGR_NETWORK_PROFILE           *ProfileToConnect;

    WIFI_HIDDEN_NETWORK_DATA           *HiddenNetwork;

    UINTN                              TempDataSize;

    VOID                               *TempData;

    LIST_ENTRY                         *Entry;

    UINT32                             Index;

--- 1328,1339 ----

***************

*** 1877,1891 ****

    } else if (Action == EFI_BROWSER_ACTION_RETRIEVE) {

 

      switch (QuestionId) {

 

      case KEY_REFRESH_NETWORK_LIST:

 

!        ProfileData = Private->CurrentNic->CurrentOperateNetwork;

!        WifiMgrRefreshNetworkList (Private, IfrNvData,ProfileData);

!        IfrNvData->SecurityType = ProfileData->SecurityType;

        break;

 

      default:

        break;

      }

    }

--- 1874,1886 ----

    } else if (Action == EFI_BROWSER_ACTION_RETRIEVE) {

 

      switch (QuestionId) {

 

      case KEY_REFRESH_NETWORK_LIST:

 

!       WifiMgrRefreshNetworkList (Private, IfrNvData);

        break;

 

      default:

        break;

      }

    }

*** WifiConnectionMgrHiiConfigAccess.h            2020-08-18 10:19:37.000000000 +05-30

--- WifiConnectionMgrHiiConfigAccess.h               2020-05-07 22:54:46.000000000 +05-30

***************

*** 232,242 ****

                                    Hii form.

 

  **/

  EFI_STATUS

  WifiMgrRefreshNetworkList (

    IN    WIFI_MGR_PRIVATE_DATA        *Private,

!   OUT   WIFI_MANAGER_IFR_NVDATA      *IfrNvData,

!   OUT   WIFI_MGR_NETWORK_PROFILE     *ProfileData

    );

 

  #endif

--- 232,241 ----

                                    Hii form.

 

  **/

  EFI_STATUS

  WifiMgrRefreshNetworkList (

    IN    WIFI_MGR_PRIVATE_DATA        *Private,

!   OUT   WIFI_MANAGER_IFR_NVDATA      *IfrNvData

    );

 

  #endif

 

 

-Siva

From: Bi, Dandan [mailto:dandan.bi@...]
Sent: Tuesday, May 19, 2020 2:42 PM
To: Sivaraman Nainar; Wu, Jiaxin; Rabeda, Maciej
Cc: devel@edk2.groups.io
Subject: RE: reg: Wifi Connection Manager Setup Page Issues

 

Hi

Could you try to call WifiMgrDxeHiiConfigAccessCallback() in EFI_BROWSER_ACTION_RETRIEVE?

 

Thanks/Dandan

From: Sivaraman Nainar <sivaramann@...>
Sent: Friday, May 15, 2020 7:55 PM
To: Wu, Jiaxin <jiaxin.wu@...>; Rabeda, Maciej <maciej.rabeda@...>; Bi, Dandan <dandan.bi@...>
Cc: devel@edk2.groups.io; Sivaraman Nainar <sivaramann@...>
Subject: RE: reg: Wifi Connection Manager Setup Page Issues

 

Hello Jiaxin / Dandan:

Could you please provide your comments.

-Siva

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Sivaraman Nainar
Sent: Friday, May 8, 2020 8:34 AM
To: devel@edk2.groups.io
Cc: jiaxin.wu@...; Rabeda, Maciej
Subject: [edk2-devel] reg: Wifi Connection Manager Setup Page Issues

 

Hello,

Getting an issue in Wifi Connection Manager Setup page when UEFI Spec 2.8 BrowserCallback specific changes integrated in Setup Browser.

According to UEFI Spec 2.8, Browser Callback is allowed to call only from Driver's ConfigAccess protocol and via ConfigAccess->Callback() it allowed for all type of actions except EFI_BROWSER_ACTION_FORM_OPEN.

WifiMgrDxeHiiConfigAccessCallback() is the callback function of WifiConnectionMgr. In this callback Security Type IfrData is updated on action EFI_BROWSER_ACTION_FORM_OPEN.  When the setup browser is not invoking the WifiMgrDxeHiiConfigAccessCallback in EFI_BROWSER_ACTION_FORM_OPEN action the control value not updated hence the browser does not show the Security Password control. The VFR suppress the Password control and shows the control only when SecurityType is SECURITY_TYPE_WPA2_PERSONAL.

Would you please feedback how this can be addressed?

-Siva

This e-mail is intended for the use of the addressee only and may contain privileged, confidential, or proprietary information that is exempt from disclosure under law. If you have received this message in error, please inform us promptly by reply e-mail, then delete the e-mail and destroy any printed copy. Thank you.


Re: TianoCore Bug Triage - APAC / NAMO - Tue, 08/11/2020 6:30pm-7:30pm #cal-reminder

Liming Gao
 

Below issues will be discussed this week. Welcome the submitter to attend the meeting and provide the detail background.

 

2903

EDK2 Pla

Placehol

unassigned@...

UNCO

Platform\ARM\VExpressPkg\DeviceTree: some files still use BSD license

liming.gao@...

23:00:10

2902

EDK2 Pla

Placehol

unassigned@...

UNCO

Platform/ARM/SgiPkg: some files still use BSD license

liming.gao@...

22:49:39

2876

EDK2

Code

unassigned@...

UNCO

SVN#15047

Seven.ding@...

Mon 14:02

2893

EDK2

Code

unassigned@...

UNCO

Acpiview: Standardise error log output format

sami.mujawar@...

Mon 00:44

2444

Tianocor

Code

michael.d.kinney@...

UNCO

Add support for USB4

dick_wilkins@...

Fri 08:35

2897

EDK2

Code

unassigned@...

UNCO

RDSEED concerns

macarl@...

Thu 19:05

2895

EDK2

Code

zhiguang.liu@...

UNCO

MdePkgHostTest.dsc build fail with LLVM/CLANG

jiewen.yao@...

Thu 02:30

2894

EDK2

Code

unassigned@...

UNCO

enable Intel CET shadow stack (control flow) in DXE phase

jiewen.yao@...

Wed 19:09

2892

EDK2

Code

unassigned@...

UNCO

MdeModulePkg: AhciPei doesn't convert port multiplier value

anbazhagan@...

Wed 11:18

2891

EDK2 Pla

IntelSil

unassigned@...

UNCO

Add support for shadowing all microcode patch to memory.

aaron.li@...

2020-08-12

2885

EDK2

Code

unassigned@...

UNCO

CD command failed with Trailing period end of folder name

DEBDIPTA.ECE@...

2020-08-11

2875

EDK2

Code

unassigned@...

UNCO

Digest Algorithm update for Authenticated Variables

divneil.r.wadhawan@...

2020-08-11

2883

Tianocor

Code

unassigned@...

UNCO

UefiCpuPkg: MpServices2Ppi and MpServicesPpi compatibility support.

chasel.chiu@...

2020-08-09

2882

EDK2

Tools

bob.c.feng@...

UNCO

Incremental build error triggered by changing name of source code file

yuwei.chen@...

2020-08-09

2831

EDK2

Code

zhichao.gao@...

UNCO

UefiBootManagerLib is not specs compliant with regards to EFI_EVENT_GROUP_READY_TO_BOOT

pete@...

2020-08-07

2881

EDK2

Tools

bob.c.feng@...

UNCO

Incremental build issue as macro value change in Makefile

yuwei.chen@...

2020-08-07

2880

EDK2

Tools

bob.c.feng@...

UNCO

Incremental build issue as directory macros extended in Makefile

yuwei.chen@...

2020-08-07

2879

EDK2

Code

unassigned@...

UNCO

Hardware error record variable quota calculation update

kun.qin@...

2020-08-06

2878

EDK2

Code

michael.d.kinney@...

UNCO

Update FMP function descriptions to match UEFI 2.8B spec

michael.kubacki@...

2020-08-06

2424

Tianocor

Code

yonghong.zhu@...

UNCO

TLSv1.3 support

prarthanasv@...

2020-08-06

2877

EDK2

Code

unassigned@...

UNCO

Bug Capsule date error

bo90geek@...

2020-08-06

2828

EDK2

Code

deric.cole@...

UNCO

Option to make DSC [Packages] section limit what INFs can include

deric.cole@...

2020-08-05

2874

EDK2

Code

unassigned@...

UNCO

QuarkPlatformPkg compilation problem

bo90geek@...

2020-08-05

2872

EDK2 Pla

SimicsOp

unassigned@...

UNCO

GCC 5 build error: unused value

prince.agyeman@...

2020-08-04

2238

EDK2 Tes

UEFI-SCT

edhaya.chandran@...

UNCO

NULL dereference in BBTestQueryFunctionAutoTest()

xypron.glpk@...

2020-07-30

2868

EDK2

Code

unassigned@...

UNCO

Request to add PatchCheck as CI Plugin

liming.gao@...

2020-07-29

 

Thanks

Liming

From: devel@edk2.groups.io <devel@edk2.groups.io>
Sent: Wednesday, August 12, 2020 9:15 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 08/11/2020 6:30pm-7:30pm #cal-reminder

 

Reminder: TianoCore Bug Triage - APAC / NAMO

When: Tuesday, 11 August 2020, 6:30pm to 7:30pm, (GMT-07:00) America/Los Angeles

Where:https://bluejeans.com/889357567?src=join_info

View Event

Organizer: Brian Richardson brian.richardson@...

Description:

https://www.tianocore.org/bug-triage

 

Meeting URL

https://bluejeans.com/889357567?src=join_info

 

Meeting ID

889 357 567

 

Want to dial in from a phone?

Dial one of the following numbers:

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

 

(see all numbers - https://www.bluejeans.com/numbers)

Enter the meeting ID and passcode followed by #


Re: [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Laszlo Ersek
 

On 08/18/20 04:10, Wang, Jian J wrote:
Laszlo,

My apologies for the slow response. I'm not the original reporter but just the BZ
submitter. And I didn't do deep analysis to this issue. The issues was reported from
one internal team. Add John in loop to see if he knows more about it or not.

My superficial understanding on such issue is that, if there's "potential" issue in
theory and hard to reproduce, it's still worthy of using an alternative way to replace
the original implementation with no "potential" issue at all. Maybe we don't have
to prove old way is something wrong but must prove that the new way is really safe.
I agree, thanks.

It would be nice to hear more from the internal team about the
originally reported (even if hard-to-trigger) issue.

Thanks!
Laszlo


Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, August 18, 2020 12:53 AM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
xiewenyi2@...; Wang, Jian J <jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Hi Jiewen,

On 08/14/20 10:53, Yao, Jiewen wrote:
To Jiewen,
Sorry, I don't have environment to reproduce the issue.
Please help me understand, if you don’t have environment to reproduce the
issue, how do you guarantee that your patch does fix the problem and we don’t
have any other vulnerabilities?

The original bug report in
<https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is seriously
lacking. It does not go into detail about the alleged integer overflow.
It does not quote the code, does not explain the control flow, does not
identify the exact edk2 commit at which the vulnerability exists.

The bug report also does not offer a reproducer.

Additionally, the exact statement that the bug report does make, namely

it's possible to overflow Offset back to 0 causing an endless loop

is wrong (as far as I can tell anyway). It is not "OffSet" that can be
overflowed to zero, but the *addend* that is added to OffSet can be
overflowed to zero. Therefore the infinite loop will arise because
OffSet remains stuck at its present value, and not because OffSet will
be re-set to zero.

For the reasons, we can only speculate as to what the actual problem is,
unless Jian decides to join the discussion and clarifies what he had in
mind originally.

My understanding (or even "reconstruction") of the vulnerability is
described above, and in the patches that I proposed.

We can write a patch based on code analysis. It's possible to identify
integer overflows based on code analysis, and it's possible to verify
the correctness of fixes by code review. Obviously testing is always
good, but many times, constructing reproducers for such issues that were
found by code review, is difficult and time consuming. We can say that
we don't fix vulnerabilities without reproducers, or we can say that we
make an effort to fix them even if all we have is code analysis (and not
a reproducer).

So the above paragraph concerns "correctness". Regarding "completeness",
I guarantee you that this patch does not fix *all* problems related to
PE parsing. (See the other BZ tickets.) It does fix *one* issue with PE
parsing. We can say that we try to fix such issues gradually (give
different CVE numbers to different issues, and address them one at a
time), or we can say that we rewrite PE parsing from the ground up.
(BTW: I have seriously attempted that in the past, and I gave up,
because the PE format is FUBAR.)

In summary:

- the problem statement is unclear,

- it seems like there is indeed an integer overflow problem in the
SecDataDir parsing loop, but it's uncertain whether the bug reporter had
exactly that in mind

- PE parsing is guaranteed to have other vulnerabilities elsewhere in
edk2, but I'm currently unaware of other such issues in
DxeImageVerificationLib specifically

- even if there are other such problems (in DxeImageVerificationLib or
elswehere), fixing this bug that we know about is likely worthwhile

- for many such bugs, constructing a reproducer is difficult and time
consuming; code analysis, and *regression-testing* are frequently the
only tools we have. That doesn't mean we should ignore this class of bugs.

(Fixing integer overflows retro-actively is more difficult than writing
overflow-free code in the first place, but that ship has sailed; so we
can only fight these bugs incrementally now, unless we can rewrite PE
parsing with a new data structure from the ground up. Again I tried that
and gave up, because the spec is not public, and what I did manage to
learn about PE, showed that it was insanely over-engineered. I'm not
saying that other binary / executable formats are better, of course.)

Please check out my patches (inlined elsewhere in this thread), and
comment whether you'd like me to post them to the list as a standalone
series.

Jian: it wouldn't hurt if you commented as well.

Thanks
Laszlo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
wenyi,xie
via groups.io
Sent: Friday, August 14, 2020 3:54 PM
To: Laszlo Ersek <lersek@...>; devel@edk2.groups.io; Yao, Jiewen
<jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

To Laszlo,
Thank you for your detailed description, I agree with what you analyzed and
I'm
OK with your patches, it's
correct and much simpler.

To Jiewen,
Sorry, I don't have environment to reproduce the issue.

Thanks
Wenyi

On 2020/8/14 2:50, Laszlo Ersek wrote:
On 08/13/20 13:55, Wenyi Xie wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2215

There is an integer overflow vulnerability in DxeImageVerificationHandler
function when parsing the PE files attribute certificate table. In cases
where WinCertificate->dwLength is sufficiently large, it's possible to
overflow Offset back to 0 causing an endless loop.

Check offset inbetween VirtualAddress and VirtualAddress + Size.
Using SafeintLib to do offset addition with result check.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Wenyi Xie <xiewenyi2@...>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
|
1 +
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h
|
1 +
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
111 +++++++++++---------
3 files changed, 63 insertions(+), 50 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
index 1e1a639857e0..a7ac4830b3d4 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
@@ -53,6 +53,7 @@ [LibraryClasses]
SecurityManagementLib
PeCoffLib
TpmMeasurementLib
+ SafeIntLib

[Protocols]
gEfiFirmwareVolume2ProtocolGuid ## SOMETIMES_CONSUMES
diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h
index 17955ff9774c..060273917d5d 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h
@@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/DevicePathLib.h>
#include <Library/SecurityManagementLib.h>
#include <Library/PeCoffLib.h>
+#include <Library/SafeIntLib.h>
#include <Protocol/FirmwareVolume2.h>
#include <Protocol/DevicePath.h>
#include <Protocol/BlockIo.h>
diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 36b87e16d53d..dbc03e28c05b 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1658,6 +1658,10 @@ DxeImageVerificationHandler (
EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
BOOLEAN IsFound;
+ UINT32 AlignedLength;
+ UINT32 Result;
+ EFI_STATUS AddStatus;
+ BOOLEAN IsAuthDataAssigned;

SignatureList = NULL;
SignatureListSize = 0;
@@ -1667,6 +1671,7 @@ DxeImageVerificationHandler (
Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
IsVerified = FALSE;
IsFound = FALSE;
+ Result = 0;

//
// Check the image type and get policy setting.
@@ -1850,9 +1855,10 @@ DxeImageVerificationHandler (
// The first certificate starts at offset (SecDataDir->VirtualAddress) from
the
start of the file.
//
for (OffSet = SecDataDir->VirtualAddress;
- OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
- OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
dwLength))) {
+ (OffSet >= SecDataDir->VirtualAddress) && (OffSet < (SecDataDir-
VirtualAddress + SecDataDir->Size));) {
+ IsAuthDataAssigned = FALSE;
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+ AlignedLength = WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength);

I disagree with this patch.

The primary reason for my disagreement is that the bug report
<https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is inexact, and
so this patch tries to fix the wrong thing.

With edk2 master at commit 65904cdbb33c, it is *not* possible to
overflow the OffSet variable to zero with "WinCertificate->dwLength"
*purely*, and cause an endless loop. Note that we have (at commit
65904cdbb33c):

for (OffSet = SecDataDir->VirtualAddress;
OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
dwLength))) {
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate-
dwLength) {
break;
}

The last sub-condition checks whether the Security Data Directory has
enough room left for "WinCertificate->dwLength". If not, then we break
out of the loop.

If we *do* have enough room, that is:

(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) >=
WinCertificate-
dwLength

then we have (by adding OffSet to both sides):

SecDataDir->VirtualAddress + SecDataDir->Size >= OffSet + WinCertificate-
dwLength

The left hand side is a known-good UINT32, and so incrementing OffSet (a
UINT32) *solely* by "WinCertificate->dwLength" (also a UINT32) does not
cause an overflow.

Instead, the problem is with the alignment. The "if" statement checks
whether we have enough room for "dwLength", but then "OffSet" is
advanced by "dwLength" *aligned up* to the next multiple of 8. And that
may indeed cause various overflows.

Now, the main problem with the present patch is that it does not fix one
of those overflows. Namely, consider that "dwLength" is very close to
MAX_UINT32 (or even think it's exactly MAX_UINT32). Then aligning it up
to the next multiple of 8 will yield 0. In other words, "AlignedLength"
will be zero.

And when that happens, there's going to be an infinite loop just the
same: "OffSet" will not be zero, but it will be *stuck*. The
SafeUint32Add() call at the bottom will succeed, but it will not change
the value of "OffSet".

More at the bottom.


if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate->dwLength) {
break;
@@ -1872,6 +1878,8 @@ DxeImageVerificationHandler (
}
AuthData = PkcsCertData->CertData;
AuthDataSize = PkcsCertData->Hdr.dwLength - sizeof(PkcsCertData-
Hdr);
+ IsAuthDataAssigned = TRUE;
+ HashStatus = HashPeImageByType (AuthData, AuthDataSize);
} else if (WinCertificate->wCertificateType ==
WIN_CERT_TYPE_EFI_GUID)
{
//
// The certificate is formatted as WIN_CERTIFICATE_UEFI_GUID which
is
described in UEFI Spec.
@@ -1880,72 +1888,75 @@ DxeImageVerificationHandler (
if (WinCertUefiGuid->Hdr.dwLength <=
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)) {
break;
}
- if (!CompareGuid (&WinCertUefiGuid->CertType, &gEfiCertPkcs7Guid))
{
- continue;
+ if (CompareGuid (&WinCertUefiGuid->CertType, &gEfiCertPkcs7Guid))
{
+ AuthData = WinCertUefiGuid->CertData;
+ AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
+ IsAuthDataAssigned = TRUE;
+ HashStatus = HashPeImageByType (AuthData, AuthDataSize);
}
- AuthData = WinCertUefiGuid->CertData;
- AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
} else {
if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) {
break;
}
- continue;
}

- HashStatus = HashPeImageByType (AuthData, AuthDataSize);
- if (EFI_ERROR (HashStatus)) {
- continue;
- }
-
- //
- // Check the digital signature against the revoked certificate in
forbidden
database (dbx).
- //
- if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
- IsVerified = FALSE;
- break;
- }
-
- //
- // Check the digital signature against the valid certificate in allowed
database (db).
- //
- if (!IsVerified) {
- if (IsAllowedByDb (AuthData, AuthDataSize)) {
- IsVerified = TRUE;
+ if (IsAuthDataAssigned && !EFI_ERROR (HashStatus)) {
+ //
+ // Check the digital signature against the revoked certificate in
forbidden
database (dbx).
+ //
+ if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
+ IsVerified = FALSE;
+ break;
}
- }

- //
- // Check the image's hash value.
- //
- DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE1,
- mImageDigest,
- &mCertType,
- mImageDigestSize,
- &IsFound
- );
- if (EFI_ERROR (DbStatus) || IsFound) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed
but %s
hash of image is found in DBX.\n", mHashTypeStr));
- IsVerified = FALSE;
- break;
- }
+ //
+ // Check the digital signature against the valid certificate in allowed
database (db).
+ //
+ if (!IsVerified) {
+ if (IsAllowedByDb (AuthData, AuthDataSize)) {
+ IsVerified = TRUE;
+ }
+ }

- if (!IsVerified) {
+ //
+ // Check the image's hash value.
+ //
DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE,
+ EFI_IMAGE_SECURITY_DATABASE1,
mImageDigest,
&mCertType,
mImageDigestSize,
&IsFound
);
- if (!EFI_ERROR (DbStatus) && IsFound) {
- IsVerified = TRUE;
- } else {
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed
but
signature is not allowed by DB and %s hash of image is not found in
DB/DBX.\n",
mHashTypeStr));
+ if (EFI_ERROR (DbStatus) || IsFound) {
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed
but %s hash of image is found in DBX.\n", mHashTypeStr));
+ IsVerified = FALSE;
+ break;
}
+
+ if (!IsVerified) {
+ DbStatus = IsSignatureFoundInDatabase (
+ EFI_IMAGE_SECURITY_DATABASE,
+ mImageDigest,
+ &mCertType,
+ mImageDigestSize,
+ &IsFound
+ );
+ if (!EFI_ERROR (DbStatus) && IsFound) {
+ IsVerified = TRUE;
+ } else {
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed
but
signature is not allowed by DB and %s hash of image is not found in
DB/DBX.\n",
mHashTypeStr));
+ }
+ }
+ }
+
+ AddStatus = SafeUint32Add (OffSet, AlignedLength, &Result);
+ if (EFI_ERROR (AddStatus)) {
+ break;
}
+ OffSet = Result;
}

if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
There are other (smaller) reasons why I dislike this patch:

- The "IsAuthDataAssigned" variable is superfluous; we could use the
existent "AuthData" variable (with a NULL-check and a NULL-assignment)
similarly.

- The patch complicates / reorganizes the control flow needlessly. This
complication originates from placing the checked "OffSet" increment at
the bottom of the loop, which then requires the removal of all the
"continue" statements. But we don't need to check-and-increment at the
bottom. We can keep the increment inside the "for" statement, only
extend the *existent* room check (which I've quoted) to take the
alignment into account as well. If there is enough room for the
alignment in the security data directory, then that guarantees there
won't be a UINT32 overflow either.

All in all, I'm proposing the following three patches instead. The first
two patches are preparation, the last patch is the fix.

Patch#1:

From 11af0a104d34d39bf1b1aab256428ae4edbddd77 Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:11:39 +0200
Subject: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract
SecDataDirEnd, SecDataDirLeft

The following two quantities:

SecDataDir->VirtualAddress + SecDataDir->Size
SecDataDir->VirtualAddress + SecDataDir->Size - OffSet

are used multiple times in DxeImageVerificationHandler(). Introduce helper
variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively.
This saves us multiple calculations and significantly simplifies the code.

Note that all three summands above have type UINT32, therefore the new
variables are also of type UINT32.

This patch does not change behavior.

(Note that the code already handles the case when the

SecDataDir->VirtualAddress + SecDataDir->Size

UINT32 addition overflows -- namely, in that case, the certificate loop is
never entered, and the corruption check right after the loop fires.)

Signed-off-by: Laszlo Ersek <lersek@...>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c |
12
++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 36b87e16d53d..8761980c88aa 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
UINT8 *AuthData;
UINTN AuthDataSize;
EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
+ UINT32 SecDataDirEnd;
+ UINT32 SecDataDirLeft;
UINT32 OffSet;
CHAR16 *NameStr;
RETURN_STATUS PeCoffStatus;
@@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
// "Attribute Certificate Table".
// The first certificate starts at offset (SecDataDir->VirtualAddress) from
the
start of the file.
//
+ SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
for (OffSet = SecDataDir->VirtualAddress;
- OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
+ OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
dwLength))) {
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
- if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
(WIN_CERTIFICATE) ||
- (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate->dwLength) {
+ SecDataDirLeft = SecDataDirEnd - OffSet;
+ if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
+ SecDataDirLeft < WinCertificate->dwLength) {
break;
}

@@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
}
}

- if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
+ if (OffSet != SecDataDirEnd) {
//
// The Size in Certificate Table or the attribute certificate table is
corrupted.
//
--
2.19.1.3.g30247aa5d201
Patch#2:

From 72012c065a53582f7df695e7b9730c45f49226c6 Mon Sep 17 00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:19:06 +0200
Subject: [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign
WinCertificate after size check

Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only
guards the de-referencing of the "WinCertificate" pointer. It does not
guard the calculation of hte pointer itself:

WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);

This is wrong; if we don't know for sure that we have enough room for a
WIN_CERTIFICATE, then even creating such a pointer, not just
de-referencing it, may invoke undefined behavior.

Move the pointer calculation after the size check.

Signed-off-by: Laszlo Ersek <lersek@...>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c |
8
+++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 8761980c88aa..461ed7cfb5ac 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
for (OffSet = SecDataDir->VirtualAddress;
OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
dwLength))) {
- WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
SecDataDirLeft = SecDataDirEnd - OffSet;
- if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
- SecDataDirLeft < WinCertificate->dwLength) {
+ if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
+ break;
+ }
+ WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+ if (SecDataDirLeft < WinCertificate->dwLength) {
break;
}

--
2.19.1.3.g30247aa5d201
Patch#3:

From 0bbba15b84f8f9f2cdc770a89f418aaec6cfb31e Mon Sep 17 00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:34:33 +0200
Subject: [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch
alignment
overflow (CVE-2019-14562)

The DxeImageVerificationHandler() function currently checks whether
"SecDataDir" has enough room for "WinCertificate->dwLength". However,
for
advancing "OffSet", "WinCertificate->dwLength" is aligned to the next
multiple of 8. If "WinCertificate->dwLength" is large enough, the
alignment will return 0, and "OffSet" will be stuck at the same value.

Check whether "SecDataDir" has room left for both
"WinCertificate->dwLength" and the alignment.

Signed-off-by: Laszlo Ersek <lersek@...>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c |
4
+++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 461ed7cfb5ac..e38eb981b7a0 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
break;
}
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
- if (SecDataDirLeft < WinCertificate->dwLength) {
+ if (SecDataDirLeft < WinCertificate->dwLength ||
+ (SecDataDirLeft - WinCertificate->dwLength <
+ ALIGN_SIZE (WinCertificate->dwLength))) {
break;
}

--
2.19.1.3.g30247aa5d201
If Wenyi and the reviewers are OK with these patches, I can submit them
as a standalone patch series.

Note that I do not have any reproducer for the issue; the best testing
that I could offer would be some light-weight Secure Boot regression
tests.

Thanks
Laszlo


.



Re: [PATCH 1/1] OvmfPkg/Bhyve: rename files to remove 'Pkg' infix

Laszlo Ersek
 

On 08/18/20 04:21, Peter Grehan wrote:
Reviewed-by: Peter Grehan <grehan@...>
Reviewed-by: Laszlo Ersek <lersek@...>

I'll merge this after the stable tag (it's neither a feature nor a
bugfix -- I'd say it is a cleanup, and so I'd not like it to add any
noise to the feature freezes).

Thanks!
Laszlo


OvmfPkg is the package, so while there are files to build bhyve
separately, they shouldn't have 'Pkg' in the name.

Signed-off-by: Rebecca Cran <rebecca@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Peter Grehan <grehan@...>
---
  OvmfPkg/Bhyve/{BhyvePkgX64.dsc => BhyveX64.dsc}                 | 2 +-
  OvmfPkg/Bhyve/{BhyvePkgX64.fdf => BhyveX64.fdf}                 | 2 +-
  OvmfPkg/Bhyve/{BhyvePkgDefines.fdf.inc => BhyveDefines.fdf.inc} | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename OvmfPkg/Bhyve/{BhyvePkgX64.dsc => BhyveX64.dsc} (97%)
  rename OvmfPkg/Bhyve/{BhyvePkgX64.fdf => BhyveX64.fdf} (96%)
  rename OvmfPkg/Bhyve/{BhyvePkgDefines.fdf.inc =>
BhyveDefines.fdf.inc} (100%)

diff --git a/OvmfPkg/Bhyve/BhyvePkgX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
similarity index 97%
rename from OvmfPkg/Bhyve/BhyvePkgX64.dsc
rename to OvmfPkg/Bhyve/BhyveX64.dsc
index 99e214619be0..d2e9edfaa6b8 100644
--- a/OvmfPkg/Bhyve/BhyvePkgX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -22,7 +22,7 @@ [Defines]
    SUPPORTED_ARCHITECTURES        = X64
    BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
    SKUID_IDENTIFIER               = DEFAULT
-  FLASH_DEFINITION               = OvmfPkg/Bhyve/BhyvePkgX64.fdf
+  FLASH_DEFINITION               = OvmfPkg/Bhyve/BhyveX64.fdf
      #
    # Defines for default states.  These can be changed on the command
line.
diff --git a/OvmfPkg/Bhyve/BhyvePkgX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf
similarity index 96%
rename from OvmfPkg/Bhyve/BhyvePkgX64.fdf
rename to OvmfPkg/Bhyve/BhyveX64.fdf
index d40344d523e4..5d2586ae141a 100644
--- a/OvmfPkg/Bhyve/BhyvePkgX64.fdf
+++ b/OvmfPkg/Bhyve/BhyveX64.fdf
@@ -11,7 +11,7 @@
 
################################################################################

    [Defines]
-!include OvmfPkg/Bhyve/BhyvePkgDefines.fdf.inc
+!include OvmfPkg/Bhyve/BhyveDefines.fdf.inc
    #
  # Build the variable store and the firmware code as one unified
flash device
diff --git a/OvmfPkg/Bhyve/BhyvePkgDefines.fdf.inc
b/OvmfPkg/Bhyve/BhyveDefines.fdf.inc
similarity index 100%
rename from OvmfPkg/Bhyve/BhyvePkgDefines.fdf.inc
rename to OvmfPkg/Bhyve/BhyveDefines.fdf.inc


[PATCH v5 3/3] UefiPayloadPkg: Support variable size MMCONF space

Marcello Sylvester Bauer <marcello.bauer@...>
 

The default size is still 256MiB, but will be overwritten by
UefiPayloadPkg with the real MMCONF size.

e.g.: On embedded AMD platforms the MMCONF window size is usually
only 64MiB.

Fixes crash on platforms not exposing 256 buses.
Tested on:
* AMD Stoney Ridge

Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
Cc: Patrick Rudolph <patrick.rudolph@...>
Cc: Christian Walter <christian.walter@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Benjamin You <benjamin.you@...>
---
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 1 +
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 +
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc b/UefiPayloadPkg/Uefi=
PayloadPkgIa32X64.dsc
index 942bc9076634..5898a474f9e9 100644
--- a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
@@ -365,6 +365,7 @@ [PcdsDynamicDefault]
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|31=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|100=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0=0D
=0D
##########################################################################=
######=0D
#=0D
diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf b/UefiPayloadPkg/=
BlSupportDxe/BlSupportDxe.inf
index 1371d5eb7952..cebc81135565 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -54,6 +54,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize=0D
=0D
[Depex]=0D
TRUE=0D
diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c b/UefiPayloadPkg/Bl=
SupportDxe/BlSupportDxe.c
index a3974dcc02f8..a746d0581ee3 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
@@ -155,13 +155,15 @@ BlDxeEntryPoint (
}=0D
=0D
//=0D
- // Set PcdPciExpressBaseAddress by HOB info=0D
+ // Set PcdPciExpressBaseAddress and PcdPciExpressBaseSize by HOB info=0D
//=0D
GuidHob =3D GetFirstGuidHob (&gUefiAcpiBoardInfoGuid);=0D
if (GuidHob !=3D NULL) {=0D
AcpiBoardInfo =3D (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);=0D
Status =3D PcdSet64S (PcdPciExpressBaseAddress, AcpiBoardInfo->PcieBas=
eAddress);=0D
ASSERT_EFI_ERROR (Status);=0D
+ Status =3D PcdSet64S (PcdPciExpressBaseSize, AcpiBoardInfo->PcieBaseSi=
ze);=0D
+ ASSERT_EFI_ERROR (Status);=0D
}=0D
=0D
return EFI_SUCCESS;=0D
--=20
2.28.0


[PATCH v5 2/3] MdePkg: PciExpressLib support variable size MMCONF

Marcello Sylvester Bauer <marcello.bauer@...>
 

Add support for arbitrary sized MMCONF by introducing a new PCD.
Add a return value to point out invalid PCI addresses.

Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
Cc: Patrick Rudolph <patrick.rudolph@...>
Cc: Christian Walter <christian.walter@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>

MdePkg: Support variable size MMCONF

* ASSERT if pci address is out of bound
* Add return value for invalid address

Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>

MdePkg: remove out of bound assertion

Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
---
MdePkg/MdePkg.dec | 4 +
MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf | 6 +-
MdePkg/Library/DxeRuntimePciExpressLib/DxeRuntimePciExpressLib.inf | 1 +
MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf | 1 +
MdePkg/Include/Library/PciExpressLib.h | 5 +-
MdePkg/Library/BasePciExpressLib/PciExpressLib.c | 216 +=
+++++++++++++---
MdePkg/Library/DxeRuntimePciExpressLib/PciExpressLib.c | 247 +=
+++++++++++++++----
MdePkg/Library/SmmPciExpressLib/PciExpressLib.c | 218 +=
+++++++++++++---
8 files changed, 584 insertions(+), 114 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 73f6c2407357..812be75fb3b2 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2274,6 +2274,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynami=
c, PcdsDynamicEx]
# @Prompt PCI Express Base Address.=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000|UINT64|0x00=
00000a=0D
=0D
+ ## This value is used to set the size of PCI express hierarchy. The defa=
ult is 256 MB.=0D
+ # @Prompt PCI Express Base Size.=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0x10000000|UINT64|0x00000=
00f=0D
+=0D
## Default current ISO 639-2 language: English & French.=0D
# @Prompt Default Value of LangCodes Variable.=0D
gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultLangCodes|"engfraengfra"|=
VOID*|0x0000001c=0D
diff --git a/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf b/MdePk=
g/Library/BasePciExpressLib/BasePciExpressLib.inf
index a7edb74cde71..12734b022ac7 100644
--- a/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+++ b/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
@@ -1,7 +1,7 @@
## @file=0D
-# Instance of PCI Express Library using the 256 MB PCI Express MMIO windo=
w.=0D
+# Instance of PCI Express Library using the variable size PCI Express MMI=
O window.=0D
#=0D
-# PCI Express Library that uses the 256 MB PCI Express MMIO window to per=
form=0D
+# PCI Express Library that uses the variable size PCI Express MMIO window=
to perform=0D
# PCI Configuration cycles. Layers on top of an I/O Library instance.=0D
#=0D
# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>=
=0D
@@ -38,4 +38,4 @@ [LibraryClasses]
=0D
[Pcd]=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## CONSUMES=0D
-=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize ## CONSUMES=0D
diff --git a/MdePkg/Library/DxeRuntimePciExpressLib/DxeRuntimePciExpressLib=
.inf b/MdePkg/Library/DxeRuntimePciExpressLib/DxeRuntimePciExpressLib.inf
index 8d2ba1d18735..26a59bda1948 100644
--- a/MdePkg/Library/DxeRuntimePciExpressLib/DxeRuntimePciExpressLib.inf
+++ b/MdePkg/Library/DxeRuntimePciExpressLib/DxeRuntimePciExpressLib.inf
@@ -47,3 +47,4 @@ [LibraryClasses]
=0D
[Pcd]=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## CONSUMES=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize ## CONSUMES=0D
diff --git a/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf b/MdePkg/=
Library/SmmPciExpressLib/SmmPciExpressLib.inf
index 729f6a3083ba..78cab6352fac 100644
--- a/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf
+++ b/MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf
@@ -35,3 +35,4 @@ [LibraryClasses]
=0D
[Pcd]=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## CONSUMES=0D
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize ## CONSUMES=0D
diff --git a/MdePkg/Include/Library/PciExpressLib.h b/MdePkg/Include/Librar=
y/PciExpressLib.h
index 826fdcf7db6c..d78193a0a352 100644
--- a/MdePkg/Include/Library/PciExpressLib.h
+++ b/MdePkg/Include/Library/PciExpressLib.h
@@ -2,8 +2,9 @@
Provides services to access PCI Configuration Space using the MMIO PCI E=
xpress window.=0D
=0D
This library is identical to the PCI Library, except the access method f=
or performing PCI=0D
- configuration cycles must be through the 256 MB PCI Express MMIO window =
whose base address=0D
- is defined by PcdPciExpressBaseAddress.=0D
+ configuration cycles must be through the PCI Express MMIO window whose b=
ase address=0D
+ is defined by PcdPciExpressBaseAddress and size defined by PcdPciExpress=
BaseSize.=0D
+=0D
=0D
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
diff --git a/MdePkg/Library/BasePciExpressLib/PciExpressLib.c b/MdePkg/Libr=
ary/BasePciExpressLib/PciExpressLib.c
index 99a166c3609b..910dd75bb48c 100644
--- a/MdePkg/Library/BasePciExpressLib/PciExpressLib.c
+++ b/MdePkg/Library/BasePciExpressLib/PciExpressLib.c
@@ -22,7 +22,8 @@
=0D
/**=0D
Assert the validity of a PCI address. A valid PCI address should contain=
1's=0D
- only in the low 28 bits.=0D
+ only in the low 28 bits. PcdPciExpressBaseSize limits the size to the re=
al=0D
+ number of PCI busses in this segment.=0D
=0D
@param A The address to validate.=0D
=0D
@@ -79,6 +80,24 @@ GetPciExpressBaseAddress (
return (VOID*)(UINTN) PcdGet64 (PcdPciExpressBaseAddress);=0D
}=0D
=0D
+/**=0D
+ Gets the size of PCI Express.=0D
+=0D
+ This internal functions retrieves PCI Express Base Size via a PCD entry=
=0D
+ PcdPciExpressBaseSize.=0D
+=0D
+ @return The base size of PCI Express.=0D
+=0D
+**/=0D
+STATIC=0D
+UINTN=0D
+PcdPciExpressBaseSize (=0D
+ VOID=0D
+ )=0D
+{=0D
+ return (UINTN) PcdGet64 (PcdPciExpressBaseSize);=0D
+}=0D
+=0D
/**=0D
Reads an 8-bit PCI configuration register.=0D
=0D
@@ -91,7 +110,8 @@ GetPciExpressBaseAddress (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -101,6 +121,9 @@ PciExpressRead8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioRead8 ((UINTN) GetPciExpressBaseAddress () + Address);=0D
}=0D
=0D
@@ -117,7 +140,8 @@ PciExpressRead8 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -128,6 +152,9 @@ PciExpressWrite8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioWrite8 ((UINTN) GetPciExpressBaseAddress () + Address, Value)=
;=0D
}=0D
=0D
@@ -148,7 +175,8 @@ PciExpressWrite8 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -159,6 +187,9 @@ PciExpressOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioOr8 ((UINTN) GetPciExpressBaseAddress () + Address, OrData);=
=0D
}=0D
=0D
@@ -179,7 +210,8 @@ PciExpressOr8 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -190,6 +222,9 @@ PciExpressAnd8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioAnd8 ((UINTN) GetPciExpressBaseAddress () + Address, AndData)=
;=0D
}=0D
=0D
@@ -212,7 +247,8 @@ PciExpressAnd8 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -224,6 +260,9 @@ PciExpressAndThenOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioAndThenOr8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
AndData,=0D
@@ -249,7 +288,9 @@ PciExpressAndThenOr8 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..7.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configuration=
=0D
+ register.=0D
=0D
**/=0D
UINT8=0D
@@ -261,6 +302,9 @@ PciExpressBitFieldRead8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldRead8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -289,7 +333,8 @@ PciExpressBitFieldRead8 (
Range 0..7.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -302,6 +347,9 @@ PciExpressBitFieldWrite8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldWrite8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -334,7 +382,8 @@ PciExpressBitFieldWrite8 (
Range 0..7.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -347,6 +396,9 @@ PciExpressBitFieldOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldOr8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -379,7 +431,8 @@ PciExpressBitFieldOr8 (
Range 0..7.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -392,6 +445,9 @@ PciExpressBitFieldAnd8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldAnd8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -428,7 +484,8 @@ PciExpressBitFieldAnd8 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -442,6 +499,9 @@ PciExpressBitFieldAndThenOr8 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr8 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -464,7 +524,8 @@ PciExpressBitFieldAndThenOr8 (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -474,6 +535,9 @@ PciExpressRead16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioRead16 ((UINTN) GetPciExpressBaseAddress () + Address);=0D
}=0D
=0D
@@ -491,7 +555,8 @@ PciExpressRead16 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -502,6 +567,9 @@ PciExpressWrite16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioWrite16 ((UINTN) GetPciExpressBaseAddress () + Address, Value=
);=0D
}=0D
=0D
@@ -523,7 +591,8 @@ PciExpressWrite16 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -534,6 +603,9 @@ PciExpressOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioOr16 ((UINTN) GetPciExpressBaseAddress () + Address, OrData);=
=0D
}=0D
=0D
@@ -555,7 +627,8 @@ PciExpressOr16 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -566,6 +639,9 @@ PciExpressAnd16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioAnd16 ((UINTN) GetPciExpressBaseAddress () + Address, AndData=
);=0D
}=0D
=0D
@@ -589,7 +665,8 @@ PciExpressAnd16 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -601,6 +678,9 @@ PciExpressAndThenOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioAndThenOr16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
AndData,=0D
@@ -627,7 +707,9 @@ PciExpressAndThenOr16 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..15.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configurati=
on=0D
+ register.=0D
=0D
**/=0D
UINT16=0D
@@ -639,6 +721,9 @@ PciExpressBitFieldRead16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldRead16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -668,7 +753,8 @@ PciExpressBitFieldRead16 (
Range 0..15.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -681,6 +767,9 @@ PciExpressBitFieldWrite16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldWrite16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -714,7 +803,8 @@ PciExpressBitFieldWrite16 (
Range 0..15.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -727,6 +817,9 @@ PciExpressBitFieldOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldOr16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -760,7 +853,8 @@ PciExpressBitFieldOr16 (
Range 0..15.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -773,6 +867,9 @@ PciExpressBitFieldAnd16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldAnd16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -810,7 +907,8 @@ PciExpressBitFieldAnd16 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -824,6 +922,9 @@ PciExpressBitFieldAndThenOr16 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr16 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -846,7 +947,8 @@ PciExpressBitFieldAndThenOr16 (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -856,6 +958,9 @@ PciExpressRead32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioRead32 ((UINTN) GetPciExpressBaseAddress () + Address);=0D
}=0D
=0D
@@ -873,7 +978,8 @@ PciExpressRead32 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=
=0D
=0D
**/=0D
UINT32=0D
@@ -884,6 +990,9 @@ PciExpressWrite32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioWrite32 ((UINTN) GetPciExpressBaseAddress () + Address, Value=
);=0D
}=0D
=0D
@@ -905,7 +1014,8 @@ PciExpressWrite32 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -916,6 +1026,9 @@ PciExpressOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioOr32 ((UINTN) GetPciExpressBaseAddress () + Address, OrData);=
=0D
}=0D
=0D
@@ -937,7 +1050,8 @@ PciExpressOr32 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -948,6 +1062,9 @@ PciExpressAnd32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioAnd32 ((UINTN) GetPciExpressBaseAddress () + Address, AndData=
);=0D
}=0D
=0D
@@ -971,7 +1088,8 @@ PciExpressAnd32 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -983,6 +1101,9 @@ PciExpressAndThenOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioAndThenOr32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
AndData,=0D
@@ -1009,7 +1130,9 @@ PciExpressAndThenOr32 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..31.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI=0D
+ configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -1021,6 +1144,9 @@ PciExpressBitFieldRead32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldRead32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1050,7 +1176,8 @@ PciExpressBitFieldRead32 (
Range 0..31.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1063,6 +1190,9 @@ PciExpressBitFieldWrite32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldWrite32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1096,7 +1226,8 @@ PciExpressBitFieldWrite32 (
Range 0..31.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1109,6 +1240,9 @@ PciExpressBitFieldOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldOr32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1142,7 +1276,8 @@ PciExpressBitFieldOr32 (
Range 0..31.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1155,6 +1290,9 @@ PciExpressBitFieldAnd32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldAnd32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1192,7 +1330,8 @@ PciExpressBitFieldAnd32 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regi=
ster.=0D
=0D
**/=0D
UINT32=0D
@@ -1206,6 +1345,9 @@ PciExpressBitFieldAndThenOr32 (
)=0D
{=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr32 (=0D
(UINTN) GetPciExpressBaseAddress () + Address,=0D
StartBit,=0D
@@ -1235,7 +1377,8 @@ PciExpressBitFieldAndThenOr32 (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer receiving the data read.=0D
=0D
- @return Size read data from StartAddress.=0D
+ @retval (UINTN)-1 Invalid PCI address.=0D
+ @retval other Size read data from StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1249,6 +1392,9 @@ PciExpressReadBuffer (
UINTN ReturnValue;=0D
=0D
ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
+ if (StartAddress >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINTN) -1;=0D
+ }=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
if (Size =3D=3D 0) {=0D
@@ -1335,7 +1481,8 @@ PciExpressReadBuffer (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer containing the data to wri=
te.=0D
=0D
- @return Size written to StartAddress.=0D
+ @retval (UINTN)-1 Invalid PCI address.=0D
+ @retval other Size written to StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1349,6 +1496,9 @@ PciExpressWriteBuffer (
UINTN ReturnValue;=0D
=0D
ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
+ if (StartAddress >=3D PcdPciExpressBaseSize()) {=0D
+ return (UINTN) -1;=0D
+ }=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
if (Size =3D=3D 0) {=0D
diff --git a/MdePkg/Library/DxeRuntimePciExpressLib/PciExpressLib.c b/MdePk=
g/Library/DxeRuntimePciExpressLib/PciExpressLib.c
index b8995435109f..cb80725c5fa6 100644
--- a/MdePkg/Library/DxeRuntimePciExpressLib/PciExpressLib.c
+++ b/MdePkg/Library/DxeRuntimePciExpressLib/PciExpressLib.c
@@ -25,6 +25,16 @@
#include <Library/DxeServicesTableLib.h>=0D
#include <Library/UefiRuntimeLib.h>=0D
=0D
+/**=0D
+ Assert the validity of a PCI address. A valid PCI address should contain =
1's=0D
+ only in the low 28 bits.=0D
+=0D
+ @param A The address to validate.=0D
+=0D
+**/=0D
+#define ASSERT_INVALID_PCI_ADDRESS(A) \=0D
+ ASSERT (((A) & ~0xfffffff) =3D=3D 0)=0D
+=0D
///=0D
/// Define table for mapping PCI Express MMIO physical addresses to virtua=
l addresses at OS runtime=0D
///=0D
@@ -39,9 +49,10 @@ typedef struct {
EFI_EVENT mDxeRuntimePciExpressLibVirtualNot=
ifyEvent =3D NULL;=0D
=0D
///=0D
-/// Module global that contains the base physical address of the PCI Expre=
ss MMIO range.=0D
+/// Module global that contains the base physical address and size of the =
PCI Express MMIO range.=0D
///=0D
UINTN mDxeRuntimePciExpressLibPciExpress=
BaseAddress =3D 0;=0D
+UINTN mDxeRuntimePciExpressLibPciExpress=
BaseSize =3D 0;=0D
=0D
///=0D
/// The number of PCI devices that have been registered for runtime access=
.=0D
@@ -120,6 +131,7 @@ DxeRuntimePciExpressLibConstructor (
// Cache the physical address of the PCI Express MMIO range into a modul=
e global variable=0D
//=0D
mDxeRuntimePciExpressLibPciExpressBaseAddress =3D (UINTN) PcdGet64 (PcdP=
ciExpressBaseAddress);=0D
+ mDxeRuntimePciExpressLibPciExpressBaseSize =3D (UINTN) PcdGet64 (PcdPciE=
xpressBaseSize);=0D
=0D
//=0D
// Register SetVirtualAddressMap () notify function=0D
@@ -179,8 +191,12 @@ DxeRuntimePciExpressLibDestructor (
This internal functions retrieves PCI Express Base Address via a PCD ent=
ry=0D
PcdPciExpressBaseAddress.=0D
=0D
- @param Address The address that encodes the PCI Bus, Device, Function =
and Register.=0D
- @return The base address of PCI Express.=0D
+ If Address > 0x0FFFFFFF, then ASSERT().=0D
+=0D
+ @param Address The address that encodes the PCI Bus, Device, Function=
and Register.=0D
+=0D
+ @retval (UINTN)-1 Invalid PCI address.=0D
+ @retval other The base address of PCI Express.=0D
=0D
**/=0D
UINTN=0D
@@ -193,7 +209,14 @@ GetPciExpressAddress (
//=0D
// Make sure Address is valid=0D
//=0D
- ASSERT (((Address) & ~0xfffffff) =3D=3D 0);=0D
+ ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINTN) -1;=0D
+ }=0D
=0D
//=0D
// Convert Address to a physical address in the MMIO PCI Express range=0D
@@ -236,7 +259,6 @@ GetPciExpressAddress (
//=0D
// No match was found. This is a critical error at OS runtime, so ASSER=
T() and force a breakpoint.=0D
//=0D
- ASSERT (FALSE);=0D
CpuBreakpoint();=0D
=0D
//=0D
@@ -288,7 +310,14 @@ PciExpressRegisterForRuntimeAccess (
//=0D
// Make sure Address is valid=0D
//=0D
- ASSERT (((Address) & ~0xfffffff) =3D=3D 0);=0D
+ ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return RETURN_UNSUPPORTED;=0D
+ }=0D
=0D
//=0D
// Convert Address to a physical address in the MMIO PCI Express range=0D
@@ -354,8 +383,8 @@ PciExpressRegisterForRuntimeAccess (
=0D
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
-=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -364,6 +393,10 @@ PciExpressRead8 (
IN UINTN Address=0D
)=0D
{=0D
+ ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioRead8 (GetPciExpressAddress (Address));=0D
}=0D
=0D
@@ -380,7 +413,8 @@ PciExpressRead8 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -390,6 +424,9 @@ PciExpressWrite8 (
IN UINT8 Value=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioWrite8 (GetPciExpressAddress (Address), Value);=0D
}=0D
=0D
@@ -410,7 +447,8 @@ PciExpressWrite8 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -420,6 +458,9 @@ PciExpressOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioOr8 (GetPciExpressAddress (Address), OrData);=0D
}=0D
=0D
@@ -440,7 +481,8 @@ PciExpressOr8 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -450,6 +492,9 @@ PciExpressAnd8 (
IN UINT8 AndData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioAnd8 (GetPciExpressAddress (Address), AndData);=0D
}=0D
=0D
@@ -472,7 +517,8 @@ PciExpressAnd8 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -483,6 +529,9 @@ PciExpressAndThenOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioAndThenOr8 (=0D
GetPciExpressAddress (Address),=0D
AndData,=0D
@@ -508,7 +557,8 @@ PciExpressAndThenOr8 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..7.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configuration=
register.=0D
=0D
**/=0D
UINT8=0D
@@ -519,6 +569,9 @@ PciExpressBitFieldRead8 (
IN UINTN EndBit=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldRead8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -547,7 +600,8 @@ PciExpressBitFieldRead8 (
Range 0..7.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -559,6 +613,9 @@ PciExpressBitFieldWrite8 (
IN UINT8 Value=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldWrite8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -591,7 +648,8 @@ PciExpressBitFieldWrite8 (
Range 0..7.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -603,6 +661,9 @@ PciExpressBitFieldOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldOr8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -635,7 +696,8 @@ PciExpressBitFieldOr8 (
Range 0..7.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -647,6 +709,9 @@ PciExpressBitFieldAnd8 (
IN UINT8 AndData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldAnd8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -683,7 +748,8 @@ PciExpressBitFieldAnd8 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT8=0D
@@ -696,6 +762,9 @@ PciExpressBitFieldAndThenOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -718,7 +787,8 @@ PciExpressBitFieldAndThenOr8 (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -727,6 +797,9 @@ PciExpressRead16 (
IN UINTN Address=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioRead16 (GetPciExpressAddress (Address));=0D
}=0D
=0D
@@ -744,7 +817,8 @@ PciExpressRead16 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -754,6 +828,9 @@ PciExpressWrite16 (
IN UINT16 Value=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioWrite16 (GetPciExpressAddress (Address), Value);=0D
}=0D
=0D
@@ -775,7 +852,8 @@ PciExpressWrite16 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -785,6 +863,9 @@ PciExpressOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioOr16 (GetPciExpressAddress (Address), OrData);=0D
}=0D
=0D
@@ -806,7 +887,8 @@ PciExpressOr16 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -816,6 +898,9 @@ PciExpressAnd16 (
IN UINT16 AndData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioAnd16 (GetPciExpressAddress (Address), AndData);=0D
}=0D
=0D
@@ -839,7 +924,8 @@ PciExpressAnd16 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -850,6 +936,9 @@ PciExpressAndThenOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioAndThenOr16 (=0D
GetPciExpressAddress (Address),=0D
AndData,=0D
@@ -876,7 +965,8 @@ PciExpressAndThenOr16 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..15.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configurati=
on register.=0D
=0D
**/=0D
UINT16=0D
@@ -887,6 +977,9 @@ PciExpressBitFieldRead16 (
IN UINTN EndBit=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldRead16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -916,7 +1009,8 @@ PciExpressBitFieldRead16 (
Range 0..15.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -928,6 +1022,9 @@ PciExpressBitFieldWrite16 (
IN UINT16 Value=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldWrite16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -961,7 +1058,8 @@ PciExpressBitFieldWrite16 (
Range 0..15.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -973,6 +1071,9 @@ PciExpressBitFieldOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldOr16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1006,7 +1107,8 @@ PciExpressBitFieldOr16 (
Range 0..15.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -1018,6 +1120,9 @@ PciExpressBitFieldAnd16 (
IN UINT16 AndData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldAnd16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1055,7 +1160,8 @@ PciExpressBitFieldAnd16 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register=
.=0D
=0D
**/=0D
UINT16=0D
@@ -1068,6 +1174,9 @@ PciExpressBitFieldAndThenOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1090,7 +1199,8 @@ PciExpressBitFieldAndThenOr16 (
@param Address The address that encodes the PCI Bus, Device, Function a=
nd=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -1099,6 +1209,9 @@ PciExpressRead32 (
IN UINTN Address=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioRead32 (GetPciExpressAddress (Address));=0D
}=0D
=0D
@@ -1116,7 +1229,8 @@ PciExpressRead32 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=
=0D
=0D
**/=0D
UINT32=0D
@@ -1126,6 +1240,9 @@ PciExpressWrite32 (
IN UINT32 Value=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioWrite32 (GetPciExpressAddress (Address), Value);=0D
}=0D
=0D
@@ -1147,7 +1264,8 @@ PciExpressWrite32 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1157,6 +1275,9 @@ PciExpressOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioOr32 (GetPciExpressAddress (Address), OrData);=0D
}=0D
=0D
@@ -1178,7 +1299,8 @@ PciExpressOr32 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1188,6 +1310,9 @@ PciExpressAnd32 (
IN UINT32 AndData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioAnd32 (GetPciExpressAddress (Address), AndData);=0D
}=0D
=0D
@@ -1211,7 +1336,8 @@ PciExpressAnd32 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1222,6 +1348,9 @@ PciExpressAndThenOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioAndThenOr32 (=0D
GetPciExpressAddress (Address),=0D
AndData,=0D
@@ -1248,7 +1377,8 @@ PciExpressAndThenOr32 (
@param EndBit The ordinal of the most significant bit in the bit fie=
ld.=0D
Range 0..31.=0D
=0D
- @return The value of the bit field read from the PCI configuration regis=
ter.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configur=
ation register.=0D
=0D
**/=0D
UINT32=0D
@@ -1259,6 +1389,9 @@ PciExpressBitFieldRead32 (
IN UINTN EndBit=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldRead32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1288,7 +1421,8 @@ PciExpressBitFieldRead32 (
Range 0..31.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1300,6 +1434,9 @@ PciExpressBitFieldWrite32 (
IN UINT32 Value=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldWrite32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1333,7 +1470,8 @@ PciExpressBitFieldWrite32 (
Range 0..31.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1345,6 +1483,9 @@ PciExpressBitFieldOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldOr32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1378,7 +1519,8 @@ PciExpressBitFieldOr32 (
Range 0..31.=0D
@param AndData The value to AND with the PCI configuration register.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1390,6 +1532,9 @@ PciExpressBitFieldAnd32 (
IN UINT32 AndData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldAnd32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1427,7 +1572,8 @@ PciExpressBitFieldAnd32 (
@param AndData The value to AND with the PCI configuration register.=
=0D
@param OrData The value to OR with the result of the AND operation.=
=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regis=
ter.=0D
=0D
**/=0D
UINT32=0D
@@ -1440,6 +1586,9 @@ PciExpressBitFieldAndThenOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1469,7 +1618,8 @@ PciExpressBitFieldAndThenOr32 (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer receiving the data read.=0D
=0D
- @return Size read data from StartAddress.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other Size read data from StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1485,9 +1635,16 @@ PciExpressReadBuffer (
//=0D
// Make sure Address is valid=0D
//=0D
- ASSERT (((StartAddress) & ~0xfffffff) =3D=3D 0);=0D
+ ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (StartAddress >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINTN) -1;=0D
+ }=0D
+=0D
if (Size =3D=3D 0) {=0D
return Size;=0D
}=0D
@@ -1572,7 +1729,8 @@ PciExpressReadBuffer (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer containing the data to wri=
te.=0D
=0D
- @return Size written to StartAddress.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other Size written to StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1588,9 +1746,16 @@ PciExpressWriteBuffer (
//=0D
// Make sure Address is valid=0D
//=0D
- ASSERT (((StartAddress) & ~0xfffffff) =3D=3D 0);=0D
+ ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (StartAddress >=3D mDxeRuntimePciExpressLibPciExpressBaseSize) {=0D
+ return (UINTN) -1;=0D
+ }=0D
+=0D
if (Size =3D=3D 0) {=0D
return 0;=0D
}=0D
diff --git a/MdePkg/Library/SmmPciExpressLib/PciExpressLib.c b/MdePkg/Libra=
ry/SmmPciExpressLib/PciExpressLib.c
index 35b9f775a80b..97bd32c8d201 100644
--- a/MdePkg/Library/SmmPciExpressLib/PciExpressLib.c
+++ b/MdePkg/Library/SmmPciExpressLib/PciExpressLib.c
@@ -20,9 +20,10 @@
#include <Library/PcdLib.h>=0D
=0D
///=0D
-/// Module global that contains the base physical address of the PCI Expre=
ss MMIO range.=0D
+/// Module global that contains the base physical address and size of the =
PCI Express MMIO range.=0D
///=0D
UINTN mSmmPciExpressLibPciExpressBaseAddress =3D 0;=0D
+UINTN mSmmPciExpressLibPciExpressBaseSize =3D 0;=0D
=0D
/**=0D
The constructor function caches the PCI Express Base Address=0D
@@ -40,9 +41,10 @@ SmmPciExpressLibConstructor (
)=0D
{=0D
//=0D
- // Cache the physical address of the PCI Express MMIO range into a module=
global variable=0D
+ // Cache the physical address and size of the PCI Express MMIO range into=
a module global variable=0D
//=0D
mSmmPciExpressLibPciExpressBaseAddress =3D (UINTN) PcdGet64 (PcdPciExpres=
sBaseAddress);=0D
+ mSmmPciExpressLibPciExpressBaseSize =3D (UINTN) PcdGet64 (PcdPciExpressBa=
seSize);=0D
=0D
return EFI_SUCCESS;=0D
}=0D
@@ -97,8 +99,12 @@ PciExpressRegisterForRuntimeAccess (
mSmmPciExpressLibPciExpressBaseAddress is initialized in the library cons=
tructor from PCD entry=0D
PcdPciExpressBaseAddress.=0D
=0D
+ If Address > 0x0FFFFFFF, then ASSERT().=0D
+=0D
@param Address The address that encodes the PCI Bus, Device, Function and=
Register.=0D
- @return MMIO address corresponding to Address.=0D
+=0D
+ @retval (UINTN)-1 Invalid PCI address.=0D
+ @retval other MMIO address corresponding to Address.=0D
=0D
**/=0D
UINTN=0D
@@ -110,6 +116,12 @@ GetPciExpressAddress (
// Make sure Address is valid=0D
//=0D
ASSERT_INVALID_PCI_ADDRESS (Address);=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINTN) -1;=0D
+ }=0D
return mSmmPciExpressLibPciExpressBaseAddress + Address;=0D
}=0D
=0D
@@ -125,7 +137,8 @@ GetPciExpressAddress (
@param Address The address that encodes the PCI Bus, Device, Function and=
=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -134,6 +147,9 @@ PciExpressRead8 (
IN UINTN Address=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioRead8 (GetPciExpressAddress (Address));=0D
}=0D
=0D
@@ -150,7 +166,8 @@ PciExpressRead8 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -160,6 +177,9 @@ PciExpressWrite8 (
IN UINT8 Value=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioWrite8 (GetPciExpressAddress (Address), Value);=0D
}=0D
=0D
@@ -180,7 +200,8 @@ PciExpressWrite8 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -190,6 +211,9 @@ PciExpressOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioOr8 (GetPciExpressAddress (Address), OrData);=0D
}=0D
=0D
@@ -210,7 +234,8 @@ PciExpressOr8 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -220,6 +245,9 @@ PciExpressAnd8 (
IN UINT8 AndData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioAnd8 (GetPciExpressAddress (Address), AndData);=0D
}=0D
=0D
@@ -242,7 +270,8 @@ PciExpressAnd8 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -253,6 +282,9 @@ PciExpressAndThenOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioAndThenOr8 (=0D
GetPciExpressAddress (Address),=0D
AndData,=0D
@@ -278,7 +310,8 @@ PciExpressAndThenOr8 (
@param EndBit The ordinal of the most significant bit in the bit field.=0D
Range 0..7.=0D
=0D
- @return The value of the bit field read from the PCI configuration regist=
er.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configuration =
register.=0D
=0D
**/=0D
UINT8=0D
@@ -289,6 +322,9 @@ PciExpressBitFieldRead8 (
IN UINTN EndBit=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldRead8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -317,7 +353,8 @@ PciExpressBitFieldRead8 (
Range 0..7.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -329,6 +366,9 @@ PciExpressBitFieldWrite8 (
IN UINT8 Value=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldWrite8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -361,7 +401,8 @@ PciExpressBitFieldWrite8 (
Range 0..7.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -373,6 +414,9 @@ PciExpressBitFieldOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldOr8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -405,7 +449,8 @@ PciExpressBitFieldOr8 (
Range 0..7.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -417,6 +462,9 @@ PciExpressBitFieldAnd8 (
IN UINT8 AndData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldAnd8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -453,7 +501,8 @@ PciExpressBitFieldAnd8 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=0D
=0D
**/=0D
UINT8=0D
@@ -466,6 +515,9 @@ PciExpressBitFieldAndThenOr8 (
IN UINT8 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT8) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr8 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -488,7 +540,8 @@ PciExpressBitFieldAndThenOr8 (
@param Address The address that encodes the PCI Bus, Device, Function and=
=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -497,6 +550,9 @@ PciExpressRead16 (
IN UINTN Address=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioRead16 (GetPciExpressAddress (Address));=0D
}=0D
=0D
@@ -514,7 +570,8 @@ PciExpressRead16 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT16=0D
@@ -524,6 +581,9 @@ PciExpressWrite16 (
IN UINT16 Value=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioWrite16 (GetPciExpressAddress (Address), Value);=0D
}=0D
=0D
@@ -545,7 +605,8 @@ PciExpressWrite16 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -555,6 +616,9 @@ PciExpressOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioOr16 (GetPciExpressAddress (Address), OrData);=0D
}=0D
=0D
@@ -576,7 +640,8 @@ PciExpressOr16 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -586,6 +651,9 @@ PciExpressAnd16 (
IN UINT16 AndData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioAnd16 (GetPciExpressAddress (Address), AndData);=0D
}=0D
=0D
@@ -609,7 +677,8 @@ PciExpressAnd16 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -620,6 +689,9 @@ PciExpressAndThenOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioAndThenOr16 (=0D
GetPciExpressAddress (Address),=0D
AndData,=0D
@@ -646,7 +718,8 @@ PciExpressAndThenOr16 (
@param EndBit The ordinal of the most significant bit in the bit field.=0D
Range 0..15.=0D
=0D
- @return The value of the bit field read from the PCI configuration regist=
er.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configuratio=
n register.=0D
=0D
**/=0D
UINT16=0D
@@ -657,6 +730,9 @@ PciExpressBitFieldRead16 (
IN UINTN EndBit=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldRead16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -686,7 +762,8 @@ PciExpressBitFieldRead16 (
Range 0..15.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -698,6 +775,9 @@ PciExpressBitFieldWrite16 (
IN UINT16 Value=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldWrite16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -731,7 +811,8 @@ PciExpressBitFieldWrite16 (
Range 0..15.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -743,6 +824,9 @@ PciExpressBitFieldOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldOr16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -776,7 +860,8 @@ PciExpressBitFieldOr16 (
Range 0..15.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -788,6 +873,9 @@ PciExpressBitFieldAnd16 (
IN UINT16 AndData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldAnd16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -825,7 +913,8 @@ PciExpressBitFieldAnd16 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration register.=
=0D
=0D
**/=0D
UINT16=0D
@@ -838,6 +927,9 @@ PciExpressBitFieldAndThenOr16 (
IN UINT16 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT16) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr16 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -860,7 +952,8 @@ PciExpressBitFieldAndThenOr16 (
@param Address The address that encodes the PCI Bus, Device, Function and=
=0D
Register.=0D
=0D
- @return The read value from the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The read value from the PCI configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -869,6 +962,9 @@ PciExpressRead32 (
IN UINTN Address=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioRead32 (GetPciExpressAddress (Address));=0D
}=0D
=0D
@@ -886,7 +982,8 @@ PciExpressRead32 (
Register.=0D
@param Value The value to write.=0D
=0D
- @return The value written to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written to the PCI configuration register.=0D
=0D
**/=0D
UINT32=0D
@@ -896,6 +993,9 @@ PciExpressWrite32 (
IN UINT32 Value=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioWrite32 (GetPciExpressAddress (Address), Value);=0D
}=0D
=0D
@@ -917,7 +1017,8 @@ PciExpressWrite32 (
Register.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -927,6 +1028,9 @@ PciExpressOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioOr32 (GetPciExpressAddress (Address), OrData);=0D
}=0D
=0D
@@ -948,7 +1052,8 @@ PciExpressOr32 (
Register.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -958,6 +1063,9 @@ PciExpressAnd32 (
IN UINT32 AndData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioAnd32 (GetPciExpressAddress (Address), AndData);=0D
}=0D
=0D
@@ -981,7 +1089,8 @@ PciExpressAnd32 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -992,6 +1101,9 @@ PciExpressAndThenOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioAndThenOr32 (=0D
GetPciExpressAddress (Address),=0D
AndData,=0D
@@ -1018,7 +1130,8 @@ PciExpressAndThenOr32 (
@param EndBit The ordinal of the most significant bit in the bit field.=0D
Range 0..31.=0D
=0D
- @return The value of the bit field read from the PCI configuration regist=
er.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value of the bit field read from the PCI configura=
tion register.=0D
=0D
**/=0D
UINT32=0D
@@ -1029,6 +1142,9 @@ PciExpressBitFieldRead32 (
IN UINTN EndBit=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldRead32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1058,7 +1174,8 @@ PciExpressBitFieldRead32 (
Range 0..31.=0D
@param Value The new value of the bit field.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -1070,6 +1187,9 @@ PciExpressBitFieldWrite32 (
IN UINT32 Value=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldWrite32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1103,7 +1223,8 @@ PciExpressBitFieldWrite32 (
Range 0..31.=0D
@param OrData The value to OR with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -1115,6 +1236,9 @@ PciExpressBitFieldOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldOr32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1148,7 +1272,8 @@ PciExpressBitFieldOr32 (
Range 0..31.=0D
@param AndData The value to AND with the PCI configuration register.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -1160,6 +1285,9 @@ PciExpressBitFieldAnd32 (
IN UINT32 AndData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldAnd32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1197,7 +1325,8 @@ PciExpressBitFieldAnd32 (
@param AndData The value to AND with the PCI configuration register.=0D
@param OrData The value to OR with the result of the AND operation.=0D
=0D
- @return The value written back to the PCI configuration register.=0D
+ @retval 0xFFFFFFFF Invalid PCI address.=0D
+ @retval other The value written back to the PCI configuration regist=
er.=0D
=0D
**/=0D
UINT32=0D
@@ -1210,6 +1339,9 @@ PciExpressBitFieldAndThenOr32 (
IN UINT32 OrData=0D
)=0D
{=0D
+ if (Address >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINT32) -1;=0D
+ }=0D
return MmioBitFieldAndThenOr32 (=0D
GetPciExpressAddress (Address),=0D
StartBit,=0D
@@ -1239,7 +1371,8 @@ PciExpressBitFieldAndThenOr32 (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer receiving the data read.=0D
=0D
- @return Size read data from StartAddress.=0D
+ @retval (UINTN)-1 Invalid PCI address.=0D
+ @retval other Size read data from StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1258,6 +1391,13 @@ PciExpressReadBuffer (
ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (StartAddress >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINTN) -1;=0D
+ }=0D
+=0D
if (Size =3D=3D 0) {=0D
return Size;=0D
}=0D
@@ -1342,7 +1482,8 @@ PciExpressReadBuffer (
@param Size The size in bytes of the transfer.=0D
@param Buffer The pointer to a buffer containing the data to write.=0D
=0D
- @return Size written to StartAddress.=0D
+ @retval (UINTN)-1 Invalid PCI address.=0D
+ @retval other Size written to StartAddress.=0D
=0D
**/=0D
UINTN=0D
@@ -1361,6 +1502,13 @@ PciExpressWriteBuffer (
ASSERT_INVALID_PCI_ADDRESS (StartAddress);=0D
ASSERT (((StartAddress & 0xFFF) + Size) <=3D 0x1000);=0D
=0D
+ //=0D
+ // Make sure the Address is in MMCONF address space=0D
+ //=0D
+ if (StartAddress >=3D mSmmPciExpressLibPciExpressBaseSize) {=0D
+ return (UINTN) -1;=0D
+ }=0D
+=0D
=0D
if (Size =3D=3D 0) {=0D
return 0;=0D
--=20
2.28.0


[PATCH v5 1/3] UefiPayloadPkg: Store the size of the MMCONF window

Marcello Sylvester Bauer <marcello.bauer@...>
 

From: Patrick Rudolph <patrick.rudolph@...>

Store the real size of the Pcie Memory Mapped Address Space.
This change is necessary to support variable size of MMCONF spaces.

Signed-off-by: Patrick Rudolph <patrick.rudolph@...>
Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@...>
Cc: Patrick Rudolph <patrick.rudolph@...>
Cc: Christian Walter <christian.walter@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Guo Dong <guo.dong@...>
Cc: Benjamin You <benjamin.you@...>
---
UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h | 1 +
UefiPayloadPkg/BlSupportPei/BlSupportPei.c | 3 +++
2 files changed, 4 insertions(+)

diff --git a/UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h b/UefiPayloadP=
kg/Include/Guid/AcpiBoardInfoGuid.h
index fe783fe5e14c..043b748ae4a9 100644
--- a/UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h
+++ b/UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h
@@ -24,6 +24,7 @@ typedef struct {
UINT64 PmTimerRegBase;=0D
UINT64 ResetRegAddress;=0D
UINT64 PcieBaseAddress;=0D
+ UINT64 PcieBaseSize;=0D
} ACPI_BOARD_INFO;=0D
=0D
#endif=0D
diff --git a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c b/UefiPayloadPkg/Bl=
SupportPei/BlSupportPei.c
index 22972453117a..a7e99f9ec6de 100644
--- a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
+++ b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
@@ -240,8 +240,10 @@ Done:
if (MmCfgHdr !=3D NULL) {=0D
MmCfgBase =3D (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BAS=
E_ADDRESS_ALLOCATION_STRUCTURE *)((UINT8*) MmCfgHdr + sizeof (*MmCfgHdr));=
=0D
AcpiBoardInfo->PcieBaseAddress =3D MmCfgBase->BaseAddress;=0D
+ AcpiBoardInfo->PcieBaseSize =3D (MmCfgBase->EndBusNumber + 1 - MmCfgBa=
se->StartBusNumber) * 4096 * 32 * 8;=0D
} else {=0D
AcpiBoardInfo->PcieBaseAddress =3D 0;=0D
+ AcpiBoardInfo->PcieBaseSize =3D 0;=0D
}=0D
DEBUG ((DEBUG_INFO, "PmCtrl Reg 0x%lx\n", AcpiBoardInfo->PmCtrlRegBase=
));=0D
DEBUG ((DEBUG_INFO, "PmTimer Reg 0x%lx\n", AcpiBoardInfo->PmTimerRegBas=
e));=0D
@@ -250,6 +252,7 @@ Done:
DEBUG ((DEBUG_INFO, "PmEvt Reg 0x%lx\n", AcpiBoardInfo->PmEvtBase));=
=0D
DEBUG ((DEBUG_INFO, "PmGpeEn Reg 0x%lx\n", AcpiBoardInfo->PmGpeEnBase))=
;=0D
DEBUG ((DEBUG_INFO, "PcieBaseAddr 0x%lx\n", AcpiBoardInfo->PcieBaseAddre=
ss));=0D
+ DEBUG ((DEBUG_INFO, "PcieBaseSize 0x%lx\n", AcpiBoardInfo->PcieBaseSize)=
);=0D
=0D
//=0D
// Verify values for proper operation=0D
--=20
2.28.0


[PATCH v5 0/3] UefiPayloadPkg: Runtime MMCONF

Marcello Sylvester Bauer <marcello.bauer@...>
 

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

v4:
* MdePkg: undo default PcdPciExpressBaseSize off by one

v3:
* split patch 2 by package
* MdePkg/PciExpress:
- PciExpressXX add return value specification
- Undo remove of ASSERT()
- PcdPciExpressBaseSize() correct function header
- correct return value types

v2:
* rebased with regards to commit 3900a63e3a1b9ba9a4105bedead7b986188cec2c
* add MdePkg Maintainer

Marcello Sylvester Bauer (2):
MdePkg: PciExpressLib support variable size MMCONF
UefiPayloadPkg: Support variable size MMCONF space

Patrick Rudolph (1):
UefiPayloadPkg: Store the size of the MMCONF window

MdePkg/MdePkg.dec | 4 +
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 1 +
MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf | 6 +-
MdePkg/Library/DxeRuntimePciExpressLib/DxeRuntimePciExpressLib.inf | 1 +
MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf | 1 +
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 +
MdePkg/Include/Library/PciExpressLib.h | 5 +-
UefiPayloadPkg/Include/Guid/AcpiBoardInfoGuid.h | 1 +
MdePkg/Library/BasePciExpressLib/PciExpressLib.c | 216 ++++++++++++++---
MdePkg/Library/DxeRuntimePciExpressLib/PciExpressLib.c | 247 ++++++++++++++++----
MdePkg/Library/SmmPciExpressLib/PciExpressLib.c | 218 ++++++++++++++---
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 4 +-
UefiPayloadPkg/BlSupportPei/BlSupportPei.c | 3 +
13 files changed, 593 insertions(+), 115 deletions(-)

--
2.28.0


[PATCH v4 8/8] IntelFsp2WrapperPkg/dsc: add HashLib, Tpm2CommandLib and Tpm2DeviceLib

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Qi Zhang <qi1.zhang@...>
---
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc b/IntelFsp2Wrapper=
Pkg/IntelFsp2WrapperPkg.dsc
index aa2eb26c33..738342b69b 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
@@ -52,6 +52,8 @@
PlatformSecLib|IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSa=
mple/SecFspWrapperPlatformSecLibSample.inf=0D
FspWrapperHobProcessLib|IntelFsp2WrapperPkg/Library/PeiFspWrapperHobProc=
essLibSample/PeiFspWrapperHobProcessLibSample.inf=0D
=0D
+ Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf=0D
+=0D
[LibraryClasses.common.PEIM,LibraryClasses.common.PEI_CORE]=0D
PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf=0D
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/Pei=
ServicesTablePointerLib.inf=0D
@@ -60,6 +62,8 @@
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf=0D
TpmMeasurementLib|SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasure=
mentLib.inf=0D
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
+ HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRou=
terPei.inf=0D
+ Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.in=
f=0D
=0D
[LibraryClasses.common.DXE_DRIVER]=0D
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntry=
Point.inf=0D
--=20
2.26.2.windows.1


[PATCH v4 7/8] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Qi Zhang <qi1.zhang@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Signed-off-by: Qi Zhang <qi1.zhang@...>
---
SecurityPkg/Include/Ppi/Tcg.h | 5 +++++
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +++++++-----
2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/SecurityPkg/Include/Ppi/Tcg.h b/SecurityPkg/Include/Ppi/Tcg.h
index 0e943f2465..22f47f9817 100644
--- a/SecurityPkg/Include/Ppi/Tcg.h
+++ b/SecurityPkg/Include/Ppi/Tcg.h
@@ -18,6 +18,11 @@ typedef struct _EDKII_TCG_PPI EDKII_TCG_PPI;
//=0D
#define EDKII_TCG_PRE_HASH 0x0000000000000001=0D
=0D
+//=0D
+// This bit is shall be set when HashData is the pre-hash digest and log o=
nly.=0D
+//=0D
+#define EDKII_TCG_PRE_HASH_LOG_ONLY 0x0000000000000002=0D
+=0D
/**=0D
Tpm measure and log data, and extend the measurement result into a speci=
fic PCR.=0D
=0D
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tc=
g2Pei.c
index 246968bb7f..0e770f4485 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -453,13 +453,15 @@ HashLogExtendEvent (
return EFI_DEVICE_ERROR;=0D
}=0D
=0D
- if(Flags & EDKII_TCG_PRE_HASH) {=0D
+ if ((Flags & EDKII_TCG_PRE_HASH) !=3D 0 || (Flags & EDKII_TCG_PRE_HASH_L=
OG_ONLY) !=3D 0) {=0D
ZeroMem (&DigestList, sizeof(DigestList));=0D
CopyMem (&DigestList, HashData, sizeof(DigestList));=0D
- Status =3D Tpm2PcrExtend (=0D
- 0,=0D
- &DigestList=0D
- );=0D
+ if ((Flags & EDKII_TCG_PRE_HASH) !=3D0 ) {=0D
+ Status =3D Tpm2PcrExtend (=0D
+ NewEventHdr->PCRIndex,=0D
+ &DigestList=0D
+ );=0D
+ }=0D
} else {=0D
Status =3D HashAndExtend (=0D
NewEventHdr->PCRIndex,=0D
--=20
2.26.2.windows.1


[PATCH v4 6/8] IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and PcdFspMeasurementConfig.

Qi Zhang
 

From: Jiewen Yao <jiewen.yao@...>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Qi Zhang <qi1.zhang@...>
Signed-off-by: Jiewen Yao <jiewen.yao@...>
---
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec | 17 +++++++++++++++++
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 6 +++++-
2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec b/IntelFsp2Wrapper=
Pkg/IntelFsp2WrapperPkg.dec
index faf2be621c..cb41ca9807 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -92,6 +92,23 @@
#=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection|0x00000001|UINT8|0x4=
000000A=0D
=0D
+ ## This PCD decides how FSP is measured=0D
+ # 1) The BootGuard ACM may already measured the FSP component, such as F=
SPT/FSPM.=0D
+ # We need a flag (PCD) to indicate if there is need to do such FSP measu=
rement or NOT.=0D
+ # 2) The FSP binary includes FSP code and FSP UPD region. The UPD region=
is considered=0D
+ # as configuration block, and it may be updated by OEM by design.=0D
+ # This flag (PCD) is to indicate if we need isolate the the UPD region f=
rom the FSP code region.=0D
+ # BIT0: Need measure FSP. (for FSP1.x) - reserved in FSP2.=0D
+ # BIT1: Need measure FSPT. (for FSP 2.x)=0D
+ # BIT2: Need measure FSPM. (for FSP 2.x)=0D
+ # BIT3: Need measure FSPS. (for FSP 2.x)=0D
+ # BIT4~30: reserved.=0D
+ # BIT31: Need isolate UPD region measurement.=0D
+ #0: measure FSP[T|M|S] as one binary in one record (PCR0).=0D
+ #1: measure FSP UPD region in one record (PCR1), the FSP code without =
UPD in another record (PCR0).=0D
+ #=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig|0x00000000|UINT3=
2|0x4000000B=0D
+=0D
[PcdsFixedAtBuild, PcdsPatchableInModule,PcdsDynamic,PcdsDynamicEx]=0D
#=0D
## These are the base address of FSP-M/S=0D
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc b/IntelFsp2Wrapper=
Pkg/IntelFsp2WrapperPkg.dsc
index cb4f69285d..aa2eb26c33 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
@@ -1,7 +1,7 @@
## @file=0D
# Provides drivers and definitions to support fsp in EDKII bios.=0D
#=0D
-# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
##=0D
@@ -45,6 +45,7 @@
# FSP Wrapper Lib=0D
FspWrapperApiLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFs=
pWrapperApiLib.inf=0D
FspWrapperApiTestLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiTestLi=
bNull/BaseFspWrapperApiTestLibNull.inf=0D
+ FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/Base=
FspMeasurementLib.inf=0D
=0D
# FSP platform sample=0D
FspWrapperPlatformLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperPlatform=
LibSample/BaseFspWrapperPlatformLibSample.inf=0D
@@ -57,6 +58,8 @@
PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf=0D
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAlloc=
ationLib.inf=0D
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf=0D
+ TpmMeasurementLib|SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasure=
mentLib.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
[LibraryClasses.common.DXE_DRIVER]=0D
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntry=
Point.inf=0D
@@ -73,6 +76,7 @@
IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecFspWrap=
perPlatformSecLibSample.inf=0D
IntelFsp2WrapperPkg/Library/PeiFspWrapperHobProcessLibSample/PeiFspWrapp=
erHobProcessLibSample.inf=0D
IntelFsp2WrapperPkg/Library/PeiFspWrapperApiTestLib/PeiFspWrapperApiTest=
Lib.inf=0D
+ IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.=
inf=0D
=0D
IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf=0D
IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf=0D
--=20
2.26.2.windows.1


[PATCH v4 5/8] SecurityPkg/dsc: add FvEventLogRecordLib

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Signed-off-by: Qi Zhang <qi1.zhang@...>
---
SecurityPkg/SecurityPkg.dec | 3 +++
SecurityPkg/SecurityPkg.dsc | 2 ++
2 files changed, 5 insertions(+)

diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 42fc48cc1f..1b7d62e802 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -84,6 +84,9 @@
#=0D
VariableKeyLib|Include/Library/VariableKeyLib.h=0D
=0D
+ ## @libraryclass Provides interfaces about firmware TPM measurement.=0D
+ #=0D
+ TcgEventLogRecordLib|Include/Library/TcgEventLogRecordLib.h=0D
[Guids]=0D
## Security package token space guid.=0D
# Include/Guid/SecurityPkgTokenSpace.h=0D
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 28effe3eda..36d15b79f9 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -66,6 +66,7 @@
ResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSyst=
emLibNull.inf=0D
VariableKeyLib|SecurityPkg/Library/VariableKeyLibNull/VariableKeyLibNull=
.inf=0D
RpmcLib|SecurityPkg/Library/RpmcLibNull/RpmcLibNull.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
[LibraryClasses.ARM]=0D
#=0D
@@ -240,6 +241,7 @@
SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf=0D
SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf=0D
SecurityPkg/Library/TcgPpVendorLibNull/TcgPpVendorLibNull.inf=0D
+ SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.inf=0D
=0D
[Components.IA32, Components.X64, Components.ARM, Components.AARCH64]=0D
SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf=0D
--=20
2.26.2.windows.1


[PATCH v4 4/8] IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.

Qi Zhang
 

From: Jiewen Yao <jiewen.yao@...>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Qi Zhang <qi1.zhang@...>
Signed-off-by: Jiewen Yao <jiewen.yao@...>
---
.../FspmWrapperPeim/FspmWrapperPeim.c | 90 ++++++++++++++++++-
.../FspmWrapperPeim/FspmWrapperPeim.inf | 20 +++--
.../FspsWrapperPeim/FspsWrapperPeim.c | 86 +++++++++++++++++-
.../FspsWrapperPeim/FspsWrapperPeim.inf | 27 +++---
4 files changed, 204 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelF=
sp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 265b77ed60..24ab534620 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -3,7 +3,7 @@
register TemporaryRamDonePpi to call TempRamExit API, and register Memor=
yDiscoveredPpi=0D
notify to call FspSiliconInit API.=0D
=0D
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>=0D
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -25,11 +25,14 @@
#include <Library/FspWrapperPlatformLib.h>=0D
#include <Library/FspWrapperHobProcessLib.h>=0D
#include <Library/FspWrapperApiLib.h>=0D
+#include <Library/FspMeasurementLib.h>=0D
=0D
#include <Ppi/FspSiliconInitDone.h>=0D
#include <Ppi/EndOfPeiPhase.h>=0D
#include <Ppi/MemoryDiscovered.h>=0D
#include <Ppi/SecPlatformInformation.h>=0D
+#include <Ppi/Tcg.h>=0D
+#include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h>=0D
#include <Library/FspWrapperApiTestLib.h>=0D
#include <FspEas.h>=0D
#include <FspStatusCode.h>=0D
@@ -147,7 +150,21 @@ FspmWrapperInit (
VOID=0D
)=0D
{=0D
- EFI_STATUS Status;=0D
+ EFI_STATUS Status;=0D
+ EFI_PEI_FIRMWARE_VOLUME_INFO_MEASUREMENT_EXCLUDED_PPI *MeasurementExclud=
edFvPpi;=0D
+ EFI_PEI_PPI_DESCRIPTOR *MeasurementExclud=
edPpiList;=0D
+=0D
+ MeasurementExcludedFvPpi =3D AllocatePool (sizeof(*MeasurementExcludedFv=
Ppi));=0D
+ ASSERT(MeasurementExcludedFvPpi !=3D NULL);=0D
+ MeasurementExcludedFvPpi->Count =3D 1;=0D
+ MeasurementExcludedFvPpi->Fv[0].FvBase =3D PcdGet32 (PcdFspmBaseAddress)=
;=0D
+ MeasurementExcludedFvPpi->Fv[0].FvLength =3D ((EFI_FIRMWARE_VOLUME_HEADE=
R *) (UINTN) PcdGet32 (PcdFspmBaseAddress))->FvLength;=0D
+=0D
+ MeasurementExcludedPpiList =3D AllocatePool (sizeof(*MeasurementExcluded=
PpiList));=0D
+ ASSERT(MeasurementExcludedPpiList !=3D NULL);=0D
+ MeasurementExcludedPpiList->Flags =3D EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_P=
EI_PPI_DESCRIPTOR_TERMINATE_LIST;=0D
+ MeasurementExcludedPpiList->Guid =3D &gEfiPeiFirmwareVolumeInfoMeasurem=
entExcludedPpiGuid;=0D
+ MeasurementExcludedPpiList->Ppi =3D MeasurementExcludedFvPpi;=0D
=0D
Status =3D EFI_SUCCESS;=0D
=0D
@@ -155,6 +172,9 @@ FspmWrapperInit (
Status =3D PeiFspMemoryInit ();=0D
ASSERT_EFI_ERROR (Status);=0D
} else {=0D
+ Status =3D PeiServicesInstallPpi (MeasurementExcludedPpiList);=0D
+ ASSERT_EFI_ERROR (Status);=0D
+=0D
PeiServicesInstallFvInfoPpi (=0D
NULL,=0D
(VOID *)(UINTN) PcdGet32 (PcdFspmBaseAddress),=0D
@@ -167,6 +187,67 @@ FspmWrapperInit (
return Status;=0D
}=0D
=0D
+/**=0D
+ This function is called after TCG installed PPI.=0D
+=0D
+ @param[in] PeiServices Pointer to PEI Services Table.=0D
+ @param[in] NotifyDesc Pointer to the descriptor for the Notification=
event that=0D
+ caused this function to execute.=0D
+ @param[in] Ppi Pointer to the PPI data associated with this f=
unction.=0D
+=0D
+ @retval EFI_STATUS Always return EFI_SUCCESS=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+TcgPpiNotify (=0D
+ IN EFI_PEI_SERVICES **PeiServices,=0D
+ IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,=0D
+ IN VOID *Ppi=0D
+ );=0D
+=0D
+EFI_PEI_NOTIFY_DESCRIPTOR mTcgPpiNotifyDesc =3D {=0D
+ (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINA=
TE_LIST),=0D
+ &gEdkiiTcgPpiGuid,=0D
+ TcgPpiNotify=0D
+};=0D
+=0D
+/**=0D
+ This function is called after TCG installed PPI.=0D
+=0D
+ @param[in] PeiServices Pointer to PEI Services Table.=0D
+ @param[in] NotifyDesc Pointer to the descriptor for the Notification=
event that=0D
+ caused this function to execute.=0D
+ @param[in] Ppi Pointer to the PPI data associated with this f=
unction.=0D
+=0D
+ @retval EFI_STATUS Always return EFI_SUCCESS=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+TcgPpiNotify (=0D
+ IN EFI_PEI_SERVICES **PeiServices,=0D
+ IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,=0D
+ IN VOID *Ppi=0D
+ )=0D
+{=0D
+ UINT32 FspMeasureMask;=0D
+=0D
+ DEBUG ((DEBUG_INFO, "TcgPpiNotify FSPM\n"));=0D
+=0D
+ FspMeasureMask =3D PcdGet32 (PcdFspMeasurementConfig);=0D
+=0D
+ if ((FspMeasureMask & FSP_MEASURE_FSPT) !=3D 0) {=0D
+ MeasureFspFirmwareBlob (0, "FSPT", PcdGet32(PcdFsptBaseAddress),=0D
+ (UINT32)((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN=
) PcdGet32 (PcdFsptBaseAddress))->FvLength);=0D
+ }=0D
+=0D
+ if ((FspMeasureMask & FSP_MEASURE_FSPM) !=3D 0) {=0D
+ MeasureFspFirmwareBlob (0, "FSPM", PcdGet32(PcdFspmBaseAddress),=0D
+ (UINT32)((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN=
) PcdGet32 (PcdFspmBaseAddress))->FvLength);=0D
+ }=0D
+=0D
+ return EFI_SUCCESS;=0D
+}=0D
+=0D
/**=0D
This is the entrypoint of PEIM=0D
=0D
@@ -182,8 +263,13 @@ FspmWrapperPeimEntryPoint (
IN CONST EFI_PEI_SERVICES **PeiServices=0D
)=0D
{=0D
+ EFI_STATUS Status;=0D
+=0D
DEBUG((DEBUG_INFO, "FspmWrapperPeimEntryPoint\n"));=0D
=0D
+ Status =3D PeiServicesNotifyPpi (&mTcgPpiNotifyDesc);=0D
+ ASSERT_EFI_ERROR (Status);=0D
+=0D
FspmWrapperInit ();=0D
=0D
return EFI_SUCCESS;=0D
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf b/Inte=
lFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index dce7ef3d0b..c3578397b6 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -6,7 +6,7 @@
# register TemporaryRamDonePpi to call TempRamExit API, and register Memor=
yDiscoveredPpi=0D
# notify to call FspSiliconInit API.=0D
#=0D
-# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>=
=0D
+# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -44,17 +44,22 @@
TimerLib=0D
FspWrapperApiLib=0D
FspWrapperApiTestLib=0D
+ FspMeasurementLib=0D
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
+ MdeModulePkg/MdeModulePkg.dec=0D
UefiCpuPkg/UefiCpuPkg.dec=0D
+ SecurityPkg/SecurityPkg.dec=0D
IntelFsp2Pkg/IntelFsp2Pkg.dec=0D
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec=0D
=0D
[Pcd]=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ## CONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress ## CONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig ## CONSUMES=0D
=0D
[Sources]=0D
FspmWrapperPeim.c=0D
@@ -63,5 +68,10 @@
gFspHobGuid ## PRODUCES ## HOB=0D
gFspApiPerformanceGuid ## SOMETIMES_CONSUMES ## GUID=0D
=0D
+[Ppis]=0D
+ gEdkiiTcgPpiGuid ## NOTIFY=0D
+ gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid ## PRODUCES=0D
+=0D
[Depex]=0D
- gEfiPeiMasterBootModePpiGuid=0D
+ gEfiPeiMasterBootModePpiGuid AND=0D
+ gPeiTpmInitializationDonePpiGuid=0D
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelF=
sp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index b20f0805a0..9d4f279e81 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -3,7 +3,7 @@
register TemporaryRamDonePpi to call TempRamExit API, and register Memor=
yDiscoveredPpi=0D
notify to call FspSiliconInit API.=0D
=0D
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>=0D
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -24,12 +24,15 @@
#include <Library/TimerLib.h>=0D
#include <Library/PerformanceLib.h>=0D
#include <Library/FspWrapperApiLib.h>=0D
+#include <Library/FspMeasurementLib.h>=0D
=0D
#include <Ppi/FspSiliconInitDone.h>=0D
#include <Ppi/EndOfPeiPhase.h>=0D
#include <Ppi/MemoryDiscovered.h>=0D
#include <Ppi/TemporaryRamDone.h>=0D
#include <Ppi/SecPlatformInformation.h>=0D
+#include <Ppi/Tcg.h>=0D
+#include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h>=0D
#include <Library/FspWrapperApiTestLib.h>=0D
#include <FspEas.h>=0D
#include <FspStatusCode.h>=0D
@@ -379,7 +382,25 @@ FspsWrapperInitDispatchMode (
VOID=0D
)=0D
{=0D
- EFI_STATUS Status;=0D
+ EFI_STATUS Status;=0D
+ EFI_PEI_FIRMWARE_VOLUME_INFO_MEASUREMENT_EXCLUDED_PPI *MeasurementExclud=
edFvPpi;=0D
+ EFI_PEI_PPI_DESCRIPTOR *MeasurementExclud=
edPpiList;=0D
+=0D
+ MeasurementExcludedFvPpi =3D AllocatePool (sizeof(*MeasurementExcludedFv=
Ppi));=0D
+ ASSERT(MeasurementExcludedFvPpi !=3D NULL);=0D
+ MeasurementExcludedFvPpi->Count =3D 1;=0D
+ MeasurementExcludedFvPpi->Fv[0].FvBase =3D PcdGet32 (PcdFspsBaseAddress)=
;=0D
+ MeasurementExcludedFvPpi->Fv[0].FvLength =3D ((EFI_FIRMWARE_VOLUME_HEADE=
R *) (UINTN) PcdGet32 (PcdFspsBaseAddress))->FvLength;=0D
+=0D
+ MeasurementExcludedPpiList =3D AllocatePool (sizeof(*MeasurementExcluded=
PpiList));=0D
+ ASSERT(MeasurementExcludedPpiList !=3D NULL);=0D
+ MeasurementExcludedPpiList->Flags =3D EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_P=
EI_PPI_DESCRIPTOR_TERMINATE_LIST;=0D
+ MeasurementExcludedPpiList->Guid =3D &gEfiPeiFirmwareVolumeInfoMeasurem=
entExcludedPpiGuid;=0D
+ MeasurementExcludedPpiList->Ppi =3D MeasurementExcludedFvPpi;=0D
+=0D
+ Status =3D PeiServicesInstallPpi (MeasurementExcludedPpiList);=0D
+ ASSERT_EFI_ERROR (Status);=0D
+=0D
//=0D
// FSP-S Wrapper running in Dispatch mode and reports FSP-S FV to PEI di=
spatcher.=0D
//=0D
@@ -398,6 +419,62 @@ FspsWrapperInitDispatchMode (
return Status;=0D
}=0D
=0D
+/**=0D
+ This function is called after TCG installed PPI.=0D
+=0D
+ @param[in] PeiServices Pointer to PEI Services Table.=0D
+ @param[in] NotifyDesc Pointer to the descriptor for the Notification=
event that=0D
+ caused this function to execute.=0D
+ @param[in] Ppi Pointer to the PPI data associated with this f=
unction.=0D
+=0D
+ @retval EFI_STATUS Always return EFI_SUCCESS=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+TcgPpiNotify (=0D
+ IN EFI_PEI_SERVICES **PeiServices,=0D
+ IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,=0D
+ IN VOID *Ppi=0D
+ );=0D
+=0D
+EFI_PEI_NOTIFY_DESCRIPTOR mTcgPpiNotifyDesc =3D {=0D
+ (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINA=
TE_LIST),=0D
+ &gEdkiiTcgPpiGuid,=0D
+ TcgPpiNotify=0D
+};=0D
+=0D
+/**=0D
+ This function is called after TCG installed PPI.=0D
+=0D
+ @param[in] PeiServices Pointer to PEI Services Table.=0D
+ @param[in] NotifyDesc Pointer to the descriptor for the Notification=
event that=0D
+ caused this function to execute.=0D
+ @param[in] Ppi Pointer to the PPI data associated with this f=
unction.=0D
+=0D
+ @retval EFI_STATUS Always return EFI_SUCCESS=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+TcgPpiNotify (=0D
+ IN EFI_PEI_SERVICES **PeiServices,=0D
+ IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,=0D
+ IN VOID *Ppi=0D
+ )=0D
+{=0D
+ UINT32 FspMeasureMask;=0D
+=0D
+ DEBUG ((DEBUG_INFO, "TcgPpiNotify FSPS\n"));=0D
+=0D
+ FspMeasureMask =3D PcdGet32 (PcdFspMeasurementConfig);=0D
+=0D
+ if ((FspMeasureMask & FSP_MEASURE_FSPS) !=3D 0) {=0D
+ MeasureFspFirmwareBlob (0, "FSPS", PcdGet32(PcdFspsBaseAddress),=0D
+ (UINT32)((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN=
) PcdGet32 (PcdFspsBaseAddress))->FvLength);=0D
+ }=0D
+=0D
+ return EFI_SUCCESS;=0D
+}=0D
+=0D
/**=0D
This is the entrypoint of PEIM.=0D
=0D
@@ -413,8 +490,13 @@ FspsWrapperPeimEntryPoint (
IN CONST EFI_PEI_SERVICES **PeiServices=0D
)=0D
{=0D
+ EFI_STATUS Status;=0D
+=0D
DEBUG ((DEBUG_INFO, "FspsWrapperPeimEntryPoint\n"));=0D
=0D
+ Status =3D PeiServicesNotifyPpi (&mTcgPpiNotifyDesc);=0D
+ ASSERT_EFI_ERROR (Status);=0D
+=0D
if (PcdGet8 (PcdFspModeSelection) =3D=3D 1) {=0D
FspsWrapperInitApiMode ();=0D
} else {=0D
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf b/Inte=
lFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index 7da92991c8..884514747f 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -6,7 +6,7 @@
# register TemporaryRamDonePpi to call TempRamExit API, and register Memor=
yDiscoveredPpi=0D
# notify to call FspSiliconInit API.=0D
#=0D
-# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>=
=0D
+# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -44,24 +44,30 @@
PerformanceLib=0D
FspWrapperApiLib=0D
FspWrapperApiTestLib=0D
+ FspMeasurementLib=0D
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
+ MdeModulePkg/MdeModulePkg.dec=0D
UefiCpuPkg/UefiCpuPkg.dec=0D
+ SecurityPkg/SecurityPkg.dec=0D
IntelFsp2Pkg/IntelFsp2Pkg.dec=0D
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec=0D
=0D
[Ppis]=0D
- gTopOfTemporaryRamPpiGuid ## PRODUCES=0D
- gFspSiliconInitDonePpiGuid ## PRODUCES=0D
- gEfiEndOfPeiSignalPpiGuid ## PRODUCES=0D
- gEfiTemporaryRamDonePpiGuid ## PRODUCES=0D
- gEfiPeiMemoryDiscoveredPpiGuid ## NOTIFY=0D
+ gTopOfTemporaryRamPpiGuid ## PRODUCES=0D
+ gFspSiliconInitDonePpiGuid ## PRODUCES=0D
+ gEfiEndOfPeiSignalPpiGuid ## PRODUCES=0D
+ gEfiTemporaryRamDonePpiGuid ## PRODUCES=0D
+ gEfiPeiMemoryDiscoveredPpiGuid ## NOTIFY=0D
+ gEdkiiTcgPpiGuid ## NOTIFY=0D
+ gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid ## PRODUCES=0D
=0D
[Pcd]=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress ## CONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress ## CONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection ## CONSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig ## CONSUMES=0D
=0D
[Guids]=0D
gFspHobGuid ## CONSUMES ## HOB=0D
@@ -71,4 +77,5 @@
FspsWrapperPeim.c=0D
=0D
[Depex]=0D
- gEfiPeiMemoryDiscoveredPpiGuid=0D
+ gEfiPeiMemoryDiscoveredPpiGuid AND=0D
+ gPeiTpmInitializationDonePpiGuid=0D
--=20
2.26.2.windows.1


[PATCH v4 3/8] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.

Qi Zhang
 

From: Jiewen Yao <jiewen.yao@...>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Qi Zhang <qi1.zhang@...>
Signed-off-by: Jiewen Yao <jiewen.yao@...>
---
.../BaseFspMeasurementLib.inf | 54 ++++
.../BaseFspMeasurementLib/FspMeasurementLib.c | 248 ++++++++++++++++++
2 files changed, 302 insertions(+)
create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseF=
spMeasurementLib.inf
create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMe=
asurementLib.c

diff --git a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasu=
rementLib.inf b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMe=
asurementLib.inf
new file mode 100644
index 0000000000..1b5f0012aa
--- /dev/null
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementL=
ib.inf
@@ -0,0 +1,54 @@
+## @file=0D
+# Provides FSP measurement functions.=0D
+#=0D
+# This library provides MeasureFspFirmwareBlob() to measure FSP binary.=0D
+#=0D
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+#=0D
+##=0D
+=0D
+[Defines]=0D
+ INF_VERSION =3D 0x00010005=0D
+ BASE_NAME =3D FspMeasurementLib=0D
+ FILE_GUID =3D 890B12B4-56CC-453E-B062-4597FC6D3D8C=
=0D
+ MODULE_TYPE =3D BASE=0D
+ VERSION_STRING =3D 1.0=0D
+ LIBRARY_CLASS =3D FspMeasurementLib=0D
+=0D
+#=0D
+# The following information is for reference only and not required by the =
build tools.=0D
+#=0D
+# VALID_ARCHITECTURES =3D IA32 X64=0D
+#=0D
+=0D
+[Sources]=0D
+ FspMeasurementLib.c=0D
+=0D
+[Packages]=0D
+ MdePkg/MdePkg.dec=0D
+ MdeModulePkg/MdeModulePkg.dec=0D
+ SecurityPkg/SecurityPkg.dec=0D
+ IntelFsp2Pkg/IntelFsp2Pkg.dec=0D
+ IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec=0D
+=0D
+[LibraryClasses]=0D
+ BaseLib=0D
+ BaseMemoryLib=0D
+ DebugLib=0D
+ PrintLib=0D
+ PcdLib=0D
+ PeiServicesLib=0D
+ PeiServicesTablePointerLib=0D
+ FspWrapperApiLib=0D
+ TcgEventLogRecordLib=0D
+ HashLib=0D
+=0D
+[Ppis]=0D
+ gEdkiiTcgPpiGuid ## CO=
NSUMES=0D
+=0D
+[Pcd]=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig ## CO=
NSUMES=0D
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ## CO=
NSUMES=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ## CO=
NSUMES=0D
+=0D
diff --git a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasureme=
ntLib.c b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementL=
ib.c
new file mode 100644
index 0000000000..0fe0606a6d
--- /dev/null
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
@@ -0,0 +1,248 @@
+/** @file=0D
+ This library is used by FSP modules to measure data to TPM.=0D
+=0D
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#include <PiPei.h>=0D
+#include <Uefi.h>=0D
+=0D
+#include <Library/BaseMemoryLib.h>=0D
+#include <Library/PeiServicesLib.h>=0D
+#include <Library/PeiServicesTablePointerLib.h>=0D
+#include <Library/PcdLib.h>=0D
+#include <Library/PrintLib.h>=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/FspWrapperApiLib.h>=0D
+#include <Library/TpmMeasurementLib.h>=0D
+#include <Library/FspMeasurementLib.h>=0D
+#include <Library/TcgEventLogRecordLib.h>=0D
+#include <Library/HashLib.h>=0D
+=0D
+#include <Ppi/Tcg.h>=0D
+#include <IndustryStandard/UefiTcgPlatform.h>=0D
+=0D
+/**=0D
+ Tpm measure and log data, and extend the measurement result into a speci=
fic PCR.=0D
+=0D
+ @param[in] PcrIndex PCR Index.=0D
+ @param[in] EventType Event type.=0D
+ @param[in] EventLog Measurement event log.=0D
+ @param[in] LogLen Event log length in bytes.=0D
+ @param[in] HashData The start of the data buffer to be hashed, =
extended.=0D
+ @param[in] HashDataLen The length, in bytes, of the buffer referen=
ced by HashData=0D
+ @param[in] Flags Bitmap providing additional information.=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+TpmMeasureAndLogDataWithFlags (=0D
+ IN UINT32 PcrIndex,=0D
+ IN UINT32 EventType,=0D
+ IN VOID *EventLog,=0D
+ IN UINT32 LogLen,=0D
+ IN VOID *HashData,=0D
+ IN UINT64 HashDataLen,=0D
+ IN UINT64 Flags=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ EDKII_TCG_PPI *TcgPpi;=0D
+ TCG_PCR_EVENT_HDR TcgEventHdr;=0D
+=0D
+ Status =3D PeiServicesLocatePpi(=0D
+ &gEdkiiTcgPpiGuid,=0D
+ 0,=0D
+ NULL,=0D
+ (VOID**)&TcgPpi=0D
+ );=0D
+ if (EFI_ERROR(Status)) {=0D
+ return Status;=0D
+ }=0D
+=0D
+ TcgEventHdr.PCRIndex =3D PcrIndex;=0D
+ TcgEventHdr.EventType =3D EventType;=0D
+ TcgEventHdr.EventSize =3D LogLen;=0D
+=0D
+ Status =3D TcgPpi->HashLogExtendEvent (=0D
+ TcgPpi,=0D
+ Flags,=0D
+ HashData,=0D
+ (UINTN)HashDataLen,=0D
+ &TcgEventHdr,=0D
+ EventLog=0D
+ );=0D
+ return Status;=0D
+}=0D
+=0D
+/**=0D
+ Measure a FSP FirmwareBlob.=0D
+=0D
+ @param[in] Description Description for this FirmwareBlob.=0D
+ @param[in] FirmwareBlobBase Base address of this FirmwareBlob.=0D
+ @param[in] FirmwareBlobLength Size in bytes of this FirmwareBlob.=
=0D
+ @param[in] CfgRegionOffset Configuration region offset in bytes=
.=0D
+ @param[in] CfgRegionSize Configuration region in bytes.=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+**/=0D
+STATIC=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureFspFirmwareBlobWithCfg (=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_PHYSICAL_ADDRESS FirmwareBlobBase,=0D
+ IN UINT64 FirmwareBlobLength,=0D
+ IN UINT32 CfgRegionOffset,=0D
+ IN UINT32 CfgRegionSize=0D
+ )=0D
+{=0D
+ EFI_PLATFORM_FIRMWARE_BLOB FvBlob, UpdBlob;=0D
+ PLATFORM_FIRMWARE_BLOB2_STRUCT FvBlob2, UpdBlob2;=0D
+ VOID *FvName;=0D
+ UINT32 FvEventType;=0D
+ VOID *FvEventLog, *UpdEventLog;=0D
+ UINT32 FvEventLogSize, UpdEventLogSize;=0D
+ EFI_STATUS Status;=0D
+ HASH_HANDLE HashHandle;=0D
+ UINT8 *HashBase;=0D
+ UINTN HashSize;=0D
+ TPML_DIGEST_VALUES DigestList;=0D
+=0D
+ FvName =3D TpmMeasurementGetFvName (FirmwareBlobBase, FirmwareBlobLength=
);=0D
+=0D
+ if (((Description !=3D NULL) || (FvName !=3D NULL)) &&=0D
+ (PcdGet32(PcdTcgPfpMeasurementRevision) >=3D TCG_EfiSpecIDEventStruc=
t_SPEC_ERRATA_TPM2_REV_105)) {=0D
+ if (Description !=3D NULL) {=0D
+ AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDesc=
ription), "%a", Description);=0D
+ AsciiSPrint((CHAR8*)UpdBlob2.BlobDescription, sizeof(UpdBlob2.BlobDe=
scription), "%aUDP", Description);=0D
+ } else {=0D
+ AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDesc=
ription), "Fv(%g)", FvName);=0D
+ AsciiSPrint((CHAR8*)UpdBlob2.BlobDescription, sizeof(UpdBlob2.BlobDe=
scription), "(%g)UDP", FvName);=0D
+ }=0D
+=0D
+ FvBlob2.BlobDescriptionSize =3D sizeof(FvBlob2.BlobDescription);=0D
+ FvBlob2.BlobBase =3D FirmwareBlobBase;=0D
+ FvBlob2.BlobLength =3D FirmwareBlobLength;=0D
+ FvEventType =3D EV_EFI_PLATFORM_FIRMWARE_BLOB2;=0D
+ FvEventLog =3D &FvBlob2;=0D
+ FvEventLogSize =3D sizeof(FvBlob2);=0D
+=0D
+ UpdBlob2.BlobDescriptionSize =3D sizeof(UpdBlob2.BlobDescription);=0D
+ UpdBlob2.BlobBase =3D CfgRegionOffset;=0D
+ UpdBlob2.BlobLength =3D CfgRegionSize;=0D
+ UpdEventLog =3D &UpdBlob2;=0D
+ UpdEventLogSize =3D sizeof(UpdBlob2);=0D
+ } else {=0D
+ FvBlob.BlobBase =3D FirmwareBlobBase;=0D
+ FvBlob.BlobLength =3D FirmwareBlobLength;=0D
+ FvEventType =3D EV_EFI_PLATFORM_FIRMWARE_BLOB;=0D
+ FvEventLog =3D &FvBlob;=0D
+ FvEventLogSize =3D sizeof(FvBlob);=0D
+=0D
+ UpdBlob.BlobBase =3D CfgRegionOffset;=0D
+ UpdBlob.BlobLength =3D CfgRegionSize;=0D
+ UpdEventLog =3D &UpdBlob;=0D
+ UpdEventLogSize =3D sizeof(UpdBlob);=0D
+ }=0D
+=0D
+ /** Initialize a SHA hash context. **/=0D
+ Status =3D HashStart (&HashHandle);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "HashStart failed - %r\n", Status));=0D
+ return Status;=0D
+ }=0D
+=0D
+ /** Hash FSP binary before UDP **/=0D
+ HashBase =3D (UINT8 *) (UINTN) FirmwareBlobBase;=0D
+ HashSize =3D (UINTN) CfgRegionOffset;=0D
+ Status =3D HashUpdate (HashHandle, HashBase, HashSize);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));=0D
+ return Status;=0D
+ }=0D
+=0D
+ /** Hash FSP binary after UDP **/=0D
+ HashBase =3D (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset + CfgR=
egionSize;=0D
+ HashSize =3D (UINTN)(FirmwareBlobLength - CfgRegionOffset - CfgRegionSiz=
e);=0D
+ Status =3D HashUpdate (HashHandle, HashBase, HashSize);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));=0D
+ return Status;=0D
+ }=0D
+=0D
+ /** Finalize the SHA hash. **/=0D
+ Status =3D HashCompleteAndExtend (HashHandle, 0, NULL, 0, &DigestList);=
=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "HashCompleteAndExtend failed - %r\n", Status));=
=0D
+ return Status;=0D
+ }=0D
+=0D
+ Status =3D TpmMeasureAndLogDataWithFlags (=0D
+ 0,=0D
+ FvEventType,=0D
+ FvEventLog,=0D
+ FvEventLogSize,=0D
+ (UINT8 *) &DigestList,=0D
+ (UINTN) sizeof(DigestList),=0D
+ EDKII_TCG_PRE_HASH_LOG_ONLY=0D
+ );=0D
+=0D
+ Status =3D TpmMeasureAndLogData (=0D
+ 1,=0D
+ EV_PLATFORM_CONFIG_FLAGS,=0D
+ UpdEventLog,=0D
+ UpdEventLogSize,=0D
+ (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset,=0D
+ CfgRegionSize=0D
+ );=0D
+=0D
+ return Status;=0D
+}=0D
+=0D
+/**=0D
+ Measure a FSP FirmwareBlob.=0D
+=0D
+ @param[in] PcrIndex PCR Index.=0D
+ @param[in] Description Description for this FirmwareBlob.=0D
+ @param[in] FirmwareBlobBase Base address of this FirmwareBlob.=0D
+ @param[in] FirmwareBlobLength Size in bytes of this FirmwareBlob.=
=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureFspFirmwareBlob (=0D
+ IN UINT32 PcrIndex,=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_PHYSICAL_ADDRESS FirmwareBlobBase,=0D
+ IN UINT64 FirmwareBlobLength=0D
+ )=0D
+{=0D
+ UINT32 FspMeasureMask;=0D
+ FSP_INFO_HEADER *FspHeaderPtr;=0D
+=0D
+ FspMeasureMask =3D PcdGet32 (PcdFspMeasurementConfig);=0D
+ if ((FspMeasureMask & FSP_MEASURE_FSPUPD) !=3D 0) {=0D
+ FspHeaderPtr =3D (FSP_INFO_HEADER *) FspFindFspHeader (FirmwareBlobBas=
e);=0D
+ if (FspHeaderPtr !=3D NULL) {=0D
+ return MeasureFspFirmwareBlobWithCfg(Description, FirmwareBlobBase, =
FirmwareBlobLength,=0D
+ FspHeaderPtr->CfgRegionOffset, =
FspHeaderPtr->CfgRegionSize);=0D
+ }=0D
+ }=0D
+=0D
+ return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase, Fir=
mwareBlobLength);=0D
+}=0D
+=0D
--=20
2.26.2.windows.1


[PATCH v4 2/8] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.

Qi Zhang
 

From: Jiewen Yao <jiewen.yao@...>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Qi Zhang <qi1.zhang@...>
Signed-off-by: Jiewen Yao <jiewen.yao@...>
---
.../Include/Library/FspMeasurementLib.h | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h

diff --git a/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h b/Inte=
lFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
new file mode 100644
index 0000000000..4620b4b08e
--- /dev/null
+++ b/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
@@ -0,0 +1,39 @@
+/** @file=0D
+ This library is used by FSP modules to measure data to TPM.=0D
+=0D
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef _FSP_MEASUREMENT_LIB_H_=0D
+#define _FSP_MEASUREMENT_LIB_H_=0D
+=0D
+#define FSP_MEASURE_FSP BIT0=0D
+#define FSP_MEASURE_FSPT BIT1=0D
+#define FSP_MEASURE_FSPM BIT2=0D
+#define FSP_MEASURE_FSPS BIT3=0D
+#define FSP_MEASURE_FSPUPD BIT31=0D
+=0D
+/**=0D
+ Measure a FSP FirmwareBlob.=0D
+=0D
+ @param[in] PcrIndex PCR Index.=0D
+ @param[in] Description Description for this FirmwareBlob.=0D
+ @param[in] FirmwareBlobBase Base address of this FirmwareBlob.=0D
+ @param[in] FirmwareBlobLength Size in bytes of this FirmwareBlob.=
=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+*/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureFspFirmwareBlob (=0D
+ IN UINT32 PcrIndex,=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_PHYSICAL_ADDRESS FirmwareBlobBase,=0D
+ IN UINT64 FirmwareBlobLength=0D
+ );=0D
+#endif=0D
--=20
2.26.2.windows.1


[PATCH v4 1/8] SecurityPkg/TcgEventLogRecordLib: add new lib for firmware measurement

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2376

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Signed-off-by: Qi Zhang <qi1.zhang@...>
---
.../Include/Library/TcgEventLogRecordLib.h | 97 +++++++++
.../TcgEventLogRecordLib.c | 197 ++++++++++++++++++
.../TcgEventLogRecordLib.inf | 40 ++++
.../TcgEventLogRecordLib.uni | 17 ++
4 files changed, 351 insertions(+)
create mode 100644 SecurityPkg/Include/Library/TcgEventLogRecordLib.h
create mode 100644 SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRec=
ordLib.c
create mode 100644 SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRec=
ordLib.inf
create mode 100644 SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRec=
ordLib.uni

diff --git a/SecurityPkg/Include/Library/TcgEventLogRecordLib.h b/SecurityP=
kg/Include/Library/TcgEventLogRecordLib.h
new file mode 100644
index 0000000000..99d634c34e
--- /dev/null
+++ b/SecurityPkg/Include/Library/TcgEventLogRecordLib.h
@@ -0,0 +1,97 @@
+/** @file=0D
+ This library is used by other modules to measure Firmware to TPM.=0D
+=0D
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef _TCG_EVENTLOGRECORD_LIB_H_=0D
+#define _TCG_EVENTLOGRECORD_LIB_H_=0D
+=0D
+#include <Uefi.h>=0D
+=0D
+#pragma pack (1)=0D
+=0D
+#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXX=
XX)"=0D
+typedef struct {=0D
+ UINT8 BlobDescriptionSize;=0D
+ UINT8 BlobDescription[sizeof(PLATFORM_FIRMWA=
RE_BLOB_DESC)];=0D
+ EFI_PHYSICAL_ADDRESS BlobBase;=0D
+ UINT64 BlobLength;=0D
+} PLATFORM_FIRMWARE_BLOB2_STRUCT;=0D
+=0D
+#define HANDOFF_TABLE_POINTER_DESC "1234567890ABCDEF"=0D
+typedef struct {=0D
+ UINT8 TableDescriptionSize;=0D
+ UINT8 TableDescription[sizeof(HANDOFF_TABLE_=
POINTER_DESC)];=0D
+ UINT64 NumberOfTables;=0D
+ EFI_CONFIGURATION_TABLE TableEntry[1];=0D
+} HANDOFF_TABLE_POINTERS2_STRUCT;=0D
+=0D
+#pragma pack ()=0D
+=0D
+/**=0D
+ Get the FvName from the FV header.=0D
+=0D
+ Causion: The FV is untrusted input.=0D
+=0D
+ @param[in] FvBase Base address of FV image.=0D
+ @param[in] FvLength Length of FV image.=0D
+=0D
+ @return FvName pointer=0D
+ @retval NULL FvName is NOT found=0D
+**/=0D
+VOID *=0D
+TpmMeasurementGetFvName (=0D
+ IN EFI_PHYSICAL_ADDRESS FvBase,=0D
+ IN UINT64 FvLength=0D
+ );=0D
+=0D
+/**=0D
+ Measure a FirmwareBlob.=0D
+=0D
+ @param[in] PcrIndex PCR Index.=0D
+ @param[in] Description Description for this FirmwareBlob.=0D
+ @param[in] FirmwareBlobBase Base address of this FirmwareBlob.=0D
+ @param[in] FirmwareBlobLength Size in bytes of this FirmwareBlob.=
=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+*/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureFirmwareBlob (=0D
+ IN UINT32 PcrIndex,=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_PHYSICAL_ADDRESS FirmwareBlobBase,=0D
+ IN UINT64 FirmwareBlobLength=0D
+ );=0D
+=0D
+/**=0D
+ Measure a HandoffTable.=0D
+=0D
+ @param[in] PcrIndex PcrIndex of the measurement.=0D
+ @param[in] Description Description for this HandoffTable.=0D
+ @param[in] TableGuid GUID of this HandoffTable.=0D
+ @param[in] TableAddress Base address of this HandoffTable.=0D
+ @param[in] TableLength Size in bytes of this HandoffTable.=
=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+*/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureHandoffTable (=0D
+ IN UINT32 PcrIndex,=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_GUID *TableGuid,=0D
+ IN VOID *TableAddress,=0D
+ IN UINTN TableLength=0D
+ );=0D
+=0D
+#endif=0D
diff --git a/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.=
c b/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.c
new file mode 100644
index 0000000000..e8a53fca0d
--- /dev/null
+++ b/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.c
@@ -0,0 +1,197 @@
+/** @file=0D
+ This library is used by other modules to measure data to TPM.=0D
+=0D
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#include <Uefi/UefiBaseType.h>=0D
+#include <Pi/PiFirmwareVolume.h>=0D
+=0D
+#include <Library/BaseMemoryLib.h>=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/ReportStatusCodeLib.h>=0D
+#include <Library/PcdLib.h>=0D
+#include <Library/PrintLib.h>=0D
+#include <Library/TcgEventLogRecordLib.h>=0D
+#include <Library/TpmMeasurementLib.h>=0D
+=0D
+#include <IndustryStandard/UefiTcgPlatform.h>=0D
+=0D
+/**=0D
+ Get the FvName from the FV header.=0D
+=0D
+ Causion: The FV is untrusted input.=0D
+=0D
+ @param[in] FvBase Base address of FV image.=0D
+ @param[in] FvLength Length of FV image.=0D
+=0D
+ @return FvName pointer=0D
+ @retval NULL FvName is NOT found=0D
+**/=0D
+VOID *=0D
+TpmMeasurementGetFvName (=0D
+ IN EFI_PHYSICAL_ADDRESS FvBase,=0D
+ IN UINT64 FvLength=0D
+ )=0D
+{=0D
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;=0D
+ EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;=0D
+=0D
+ if (FvBase >=3D MAX_ADDRESS) {=0D
+ return NULL;=0D
+ }=0D
+ if (FvLength >=3D MAX_ADDRESS - FvBase) {=0D
+ return NULL;=0D
+ }=0D
+ if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {=0D
+ return NULL;=0D
+ }=0D
+=0D
+ FvHeader =3D (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;=0D
+ if (FvHeader->Signature !=3D EFI_FVH_SIGNATURE) {=0D
+ return NULL;=0D
+ }=0D
+ if (FvHeader->ExtHeaderOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {=0D
+ return NULL;=0D
+ }=0D
+ if (FvHeader->ExtHeaderOffset + sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) >=
FvLength) {=0D
+ return NULL;=0D
+ }=0D
+ FvExtHeader =3D (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase + FvHea=
der->ExtHeaderOffset);=0D
+=0D
+ return &FvExtHeader->FvName;=0D
+}=0D
+=0D
+/**=0D
+ Measure a FirmwareBlob.=0D
+=0D
+ @param[in] PcrIndex PcrIndex of the measurement.=0D
+ @param[in] Description Description for this FirmwareBlob.=0D
+ @param[in] FirmwareBlobBase Base address of this FirmwareBlob.=0D
+ @param[in] FirmwareBlobLength Size in bytes of this FirmwareBlob.=
=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureFirmwareBlob (=0D
+ IN UINT32 PcrIndex,=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_PHYSICAL_ADDRESS FirmwareBlobBase,=0D
+ IN UINT64 FirmwareBlobLength=0D
+ )=0D
+{=0D
+ EFI_PLATFORM_FIRMWARE_BLOB FvBlob;=0D
+ PLATFORM_FIRMWARE_BLOB2_STRUCT FvBlob2;=0D
+ VOID *FvName;=0D
+ UINT32 EventType;=0D
+ VOID *EventLog;=0D
+ UINT32 EventLogSize;=0D
+ EFI_STATUS Status;=0D
+=0D
+ FvName =3D TpmMeasurementGetFvName (FirmwareBlobBase, FirmwareBlobLength=
);=0D
+=0D
+ if (((Description !=3D NULL) || (FvName !=3D NULL)) &&=0D
+ (PcdGet32(PcdTcgPfpMeasurementRevision) >=3D TCG_EfiSpecIDEventStruc=
t_SPEC_ERRATA_TPM2_REV_105)) {=0D
+ if (Description !=3D NULL) {=0D
+ AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDesc=
ription), "%a", Description);=0D
+ } else {=0D
+ AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDesc=
ription), "Fv(%g)", FvName);=0D
+ }=0D
+=0D
+ FvBlob2.BlobDescriptionSize =3D sizeof(FvBlob2.BlobDescription);=0D
+ FvBlob2.BlobBase =3D FirmwareBlobBase;=0D
+ FvBlob2.BlobLength =3D FirmwareBlobLength;=0D
+=0D
+ EventType =3D EV_EFI_PLATFORM_FIRMWARE_BLOB2;=0D
+ EventLog =3D &FvBlob2;=0D
+ EventLogSize =3D sizeof(FvBlob2);=0D
+ } else {=0D
+ FvBlob.BlobBase =3D FirmwareBlobBase;=0D
+ FvBlob.BlobLength =3D FirmwareBlobLength;=0D
+=0D
+ EventType =3D EV_EFI_PLATFORM_FIRMWARE_BLOB;=0D
+ EventLog =3D &FvBlob;=0D
+ EventLogSize =3D sizeof(FvBlob);=0D
+ }=0D
+=0D
+ Status =3D TpmMeasureAndLogData (=0D
+ PcrIndex,=0D
+ EventType,=0D
+ EventLog,=0D
+ EventLogSize,=0D
+ (VOID*)(UINTN)FirmwareBlobBase,=0D
+ FirmwareBlobLength=0D
+ );=0D
+=0D
+ return Status;=0D
+}=0D
+=0D
+/**=0D
+ Measure a HandoffTable.=0D
+=0D
+ @param[in] PcrIndex PcrIndex of the measurement.=0D
+ @param[in] Description Description for this HandoffTable.=0D
+ @param[in] TableGuid GUID of this HandoffTable.=0D
+ @param[in] TableAddress Base address of this HandoffTable.=0D
+ @param[in] TableLength Size in bytes of this HandoffTable.=
=0D
+=0D
+ @retval EFI_SUCCESS Operation completed successfully.=0D
+ @retval EFI_UNSUPPORTED TPM device not available.=0D
+ @retval EFI_OUT_OF_RESOURCES Out of memory.=0D
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+MeasureHandoffTable (=0D
+ IN UINT32 PcrIndex,=0D
+ IN CHAR8 *Description OPTIONAL,=0D
+ IN EFI_GUID *TableGuid,=0D
+ IN VOID *TableAddress,=0D
+ IN UINTN TableLength=0D
+ )=0D
+{=0D
+ EFI_HANDOFF_TABLE_POINTERS HandoffTables;=0D
+ HANDOFF_TABLE_POINTERS2_STRUCT HandoffTables2;=0D
+ UINT32 EventType;=0D
+ VOID *EventLog;=0D
+ UINT32 EventLogSize;=0D
+ EFI_STATUS Status;=0D
+=0D
+ if ((Description !=3D NULL) &&=0D
+ (PcdGet32(PcdTcgPfpMeasurementRevision) >=3D TCG_EfiSpecIDEventStruc=
t_SPEC_ERRATA_TPM2_REV_105)) {=0D
+ AsciiSPrint((CHAR8*)HandoffTables2.TableDescription, sizeof(HandoffTab=
les2.TableDescription), "%a", Description);=0D
+=0D
+ HandoffTables2.TableDescriptionSize =3D sizeof(HandoffTables2.TableDes=
cription);=0D
+ HandoffTables2.NumberOfTables =3D 1;=0D
+ CopyGuid (&(HandoffTables2.TableEntry[0].VendorGuid), TableGuid);=0D
+ HandoffTables2.TableEntry[0].VendorTable =3D TableAddress;=0D
+=0D
+ EventType =3D EV_EFI_HANDOFF_TABLES2;=0D
+ EventLog =3D &HandoffTables2;=0D
+ EventLogSize =3D sizeof(HandoffTables2);=0D
+ } else {=0D
+ HandoffTables.NumberOfTables =3D 1;=0D
+ CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), TableGuid);=0D
+ HandoffTables.TableEntry[0].VendorTable =3D TableAddress;=0D
+=0D
+ EventType =3D EV_EFI_HANDOFF_TABLES;=0D
+ EventLog =3D &HandoffTables;=0D
+ EventLogSize =3D sizeof(HandoffTables);=0D
+ }=0D
+=0D
+ Status =3D TpmMeasureAndLogData (=0D
+ PcrIndex,=0D
+ EventType,=0D
+ EventLog,=0D
+ EventLogSize,=0D
+ TableAddress,=0D
+ TableLength=0D
+ );=0D
+ return Status;=0D
+}=0D
diff --git a/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.=
inf b/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.inf
new file mode 100644
index 0000000000..71388f43f6
--- /dev/null
+++ b/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.inf
@@ -0,0 +1,40 @@
+## @file=0D
+# Provides interface for firmwware TPM measurement=0D
+#=0D
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+#=0D
+##=0D
+=0D
+[Defines]=0D
+ INF_VERSION =3D 0x00010005=0D
+ BASE_NAME =3D TcgEventLogRecordLib=0D
+ MODULE_UNI_FILE =3D TcgEventLogRecordLib.uni=0D
+ FILE_GUID =3D F8125B2A-3922-4A22-A6F8-3B6159A25A3B=
=0D
+ MODULE_TYPE =3D BASE=0D
+ VERSION_STRING =3D 1.0=0D
+ LIBRARY_CLASS =3D NULL=0D
+=0D
+#=0D
+# The following information is for reference only and not required by the =
build tools.=0D
+#=0D
+# VALID_ARCHITECTURES =3D IA32 X64=0D
+#=0D
+=0D
+[Sources]=0D
+ TcgEventLogRecordLib.c=0D
+=0D
+[Packages]=0D
+ MdePkg/MdePkg.dec=0D
+ MdeModulePkg/MdeModulePkg.dec=0D
+ SecurityPkg/SecurityPkg.dec=0D
+=0D
+[LibraryClasses]=0D
+ BaseLib=0D
+ BaseMemoryLib=0D
+ DebugLib=0D
+ PcdLib=0D
+ TpmMeasurementLib=0D
+=0D
+[Pcd]=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ## =
CONSUMES=0D
diff --git a/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.=
uni b/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.uni
new file mode 100644
index 0000000000..b1ca410074
--- /dev/null
+++ b/SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.uni
@@ -0,0 +1,17 @@
+// /** @file=0D
+// Provides interface for firmwware TPM measurement=0D
+//=0D
+// This library provides MeasureFirmwareBlob() and MeasureHandoffTable()=0D
+// to measure and log data, and extend the measurement result into a speci=
fic PCR.=0D
+//=0D
+// Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+//=0D
+// SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+//=0D
+// **/=0D
+=0D
+=0D
+#string STR_MODULE_ABSTRACT #language en-US "Provides Firmware=
TPM measurement functions for TPM1.2 and TPM 2.0"=0D
+=0D
+#string STR_MODULE_DESCRIPTION #language en-US "This library prov=
ides MeasureFirmwareBlob() and MeasureHandoffTable() to measure and log dat=
a, and extend the measurement result into a specific PCR."=0D
+=0D
--=20
2.26.2.windows.1


[PATCH v4 0/8] Need add a FSP binary measurement

Qi Zhang
 

v4 change:
rename FvEventLogRecordLib to TcgEventLogRecordLib.
v3 change:
add a new lib FvEventLogRecordLib for gerneric code.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

The EDKII BIOS calls FSP API in FSP Wrapper Pkg.
This FSP code need to be measured into TPM.

We need add a generic module in FSP Wrapper Pkg code to measure:
1) FSP-T, FSP-M, FSP-S in API mode.
2) FSP-T in Dispatch-mode. The FSP-M and FSP-S will be reported
as standard FV and they will be measured by TCG-PEI.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Qi Zhang <qi1.zhang@...>

Jiewen Yao (4):
IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.
IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and
PcdFspMeasurementConfig.

Qi Zhang (4):
SecurityPkg/TcgEventLogRecordLib: add new lib for firmware measurement
SecurityPkg/dsc: add FvEventLogRecordLib
SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
IntelFsp2WrapperPkg/dsc: add HashLib, Tpm2CommandLib and Tpm2DeviceLib

.../FspmWrapperPeim/FspmWrapperPeim.c | 90 ++++++-
.../FspmWrapperPeim/FspmWrapperPeim.inf | 20 +-
.../FspsWrapperPeim/FspsWrapperPeim.c | 86 +++++-
.../FspsWrapperPeim/FspsWrapperPeim.inf | 27 +-
.../Include/Library/FspMeasurementLib.h | 39 +++
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec | 17 ++
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 10 +-
.../BaseFspMeasurementLib.inf | 54 ++++
.../BaseFspMeasurementLib/FspMeasurementLib.c | 248 ++++++++++++++++++
.../Include/Library/TcgEventLogRecordLib.h | 97 +++++++
SecurityPkg/Include/Ppi/Tcg.h | 5 +
.../TcgEventLogRecordLib.c | 197 ++++++++++++++
.../TcgEventLogRecordLib.inf | 40 +++
.../TcgEventLogRecordLib.uni | 17 ++
SecurityPkg/SecurityPkg.dec | 3 +
SecurityPkg/SecurityPkg.dsc | 2 +
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +-
17 files changed, 939 insertions(+), 25 deletions(-)
create mode 100644 IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
create mode 100644 SecurityPkg/Include/Library/TcgEventLogRecordLib.h
create mode 100644 SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.c
create mode 100644 SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.inf
create mode 100644 SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.uni

--
2.26.2.windows.1


Re: [PATCH v6 00/14] Add the VariablePolicy feature

Bret Barkelew
 

Responses below…

 

- Bret

 

From: Dandan Bi via groups.io
Sent: Tuesday, August 11, 2020 6:52 AM
To: devel@edk2.groups.io; Bi, Dandan; bret@...
Cc: Yao, Jiewen; Zhang, Chao B; Wang, Jian J; Wu, Hao A; liming.gao; Justen, Jordan L; Laszlo Ersek; Ard Biesheuvel; Andrew Fish; Ni, Ray
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v6 00/14] Add the VariablePolicy feature

 

Hi Bret,

Sorry for the delayed response.

Some more comments here:

1. Currently I see the LockVaribePolicy is called at ReadyToBoot by variable driver, could we update it to be called at EndOfDxe? We should prevent malicious code registering policy after EndOfDxe for security concern. And could we also add the test case to check the variable policy is locked at EndofDxe?

We could. Right now it’s at ReadyToBoot because it’s just there as a safety net and the platform could lock it earlier. Would it work to have a PCD for which EventGroup GUID the platform should lock on?

2. For patch 4, the SMM communication,  some general guidelines for SMI handler:
a)  Check whether the communication buffer is outside SMM and valid.
For this feature, please double check whether the communication buffer is checked, if all the range in communication buffer has already been checked within existing edk2 core infrastructure, please also add the comments in the code to mention that it has been checked.

I checked this, but I will recheck (since there’ve been a few revisions in the patches) and update the comments.

b) Should copy the communication buffer to SMRAM before checking the data fields to avoid TOC/TOU attac
For this feature, for example, when dump variable policy, if malicious code updates the DumpParams->TotalSize in communication buffer to smaller one to allocate the PaginationCache buffer, and then update it the correct one and dump the variable policy data into the PaginationCache buffer, it will cause buffer overflow in this case.  So please double check the code and copy the communication buffer into SMRAM to avoid such kind issue.

Will also check for this.

3. Did you do any security test for this feature?

Such as? There are both unit tests and integration tests to ensure correct functionality and that the disable and lock interfaces work as expected. I haven’t fuzzed it or anything that involved.

4. Currently, LockVariablePolicy can prevent RegisterVariablePolicy and DisableVariablePolicy. So in SMI hander, could we check the variable policy is locked or not firstly and then decide whether need to check and execution for VAR_CHECK_POLICY_COMMAND_REGISTER and VAR_CHECK_POLICY_COMMAND_DISABLE?

I’ll take a look, but my gut says this may be an unnecessary complication.

5. Since there is the logic when variable policy is disabled, it will permit deletion of auth/protected variables. Could we add some comments in code to mention that variable policy should always be enabled for security concern to avoid giving bad example?

I’m happy to think about how to document this, but I’m not immediately inclined to outright say it shouldn’t be disabled. I’d be happy to say that it shouldn’t be disabled in “normal, production configuration”, but it’s entirely reasonable to be disabled in a Manufacturing or R&R environment and we would actually prefer this be used because it would at least be consistent across platforms, rather than being something done ad hoc by each platform that needs it. Would that be sufficient?

Thanks,
Dandan
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dandan
> Bi
> Sent: Thursday, July 2, 2020 10:14 AM
> To: devel@edk2.groups.io; bret@...
> Cc: Yao, Jiewen <jiewen.yao@...>; Zhang, Chao B
> <chao.b.zhang@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao
> A <hao.a.wu@...>; Gao, Liming <liming.gao@...>; Justen,
> Jordan L <jordan.l.justen@...>; Laszlo Ersek <lersek@...>;
> Ard Biesheuvel <ard.biesheuvel@...>; Andrew Fish
> <afish@...>; Ni, Ray <ray.ni@...>
> Subject: Re: [edk2-devel] [PATCH v6 00/14] Add the VariablePolicy feature
>
> Hi Bret,
>
> Thanks for the contribution.
>
> I have taken an overview of this patch series and have some small comments
> in the related patches, please check in sub-patch.
>
> I will review the patch series more in details and bring more comments back
> if have. Do you have a branch for these patches in GitHub? Which should be
> easy for review.
>
>
> Thanks,
> Dandan
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret
> > Barkelew
> > Sent: Tuesday, June 23, 2020 2:41 PM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao@...>; Zhang, Chao B
> > <chao.b.zhang@...>; Wang, Jian J <jian.j.wang@...>; Wu,
> > Hao A <hao.a.wu@...>; Gao, Liming <liming.gao@...>;
> > Justen, Jordan L <jordan.l.justen@...>; Laszlo Ersek
> > <lersek@...>; Ard Biesheuvel <ard.biesheuvel@...>;
> Andrew
> > Fish <afish@...>; Ni, Ray <ray.ni@...>
> > Subject: [edk2-devel] [PATCH v6 00/14] Add the VariablePolicy feature
> >
> > REF:https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C91eed7c4a0d54e2eff6008d83dfdc4ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637327507431539442&amp;sdata=cLpx7VZsG%2F6r6GCXmiCS4DmmgOH4iKfX3VSAAYUOU00%3D&amp;reserved=0
> >
> > The 14 patches in this series add the VariablePolicy feature to the
> > core, deprecate Edk2VarLock (while adding a compatibility layer to
> > reduce code churn), and integrate the VariablePolicy libraries and
> > protocols into Variable Services.
> >
> > Since the integration requires multiple changes, including adding
> > libraries, a protocol, an SMI communication handler, and
> > VariableServices integration, the patches are broken up by individual
> > library additions and then a final integration. Security-sensitive
> > changes like bypassing Authenticated Variable enforcement are also
> > broken out into individual patches so that attention can be called directly to
> them.
> >
> > Platform porting instructions are described in this wiki entry:
> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C91eed7c4a0d54e2eff6008d83dfdc4ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637327507431539442&amp;sdata=%2FYNgK7yixA5Gi7E9bHw3LIUNAQpZMh9ykTUqCqv2SRY%3D&amp;reserved=0
> > Protocol---Enhanced-Method-for-Managing-Variables#platform-porting
> >
> > Discussion of the feature can be found in multiple places throughout
> > the last year on the RFC channel, staging branches, and in devel.
> >
> > Most recently, this subject was discussed in this thread:
> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7C91eed7c4a0d54e2eff6008d83dfdc4ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637327507431539442&amp;sdata=i7qaO6eZT8%2BzCT0satTptMWwCNspDz%2BS05eJmGGR628%3D&amp;reserved=0
> > (the code branches shared in that discussion are now out of date, but
> > the whitepapers and discussion are relevant).
> >
> > Cc: Jiewen Yao <jiewen.yao@...>
> > Cc: Chao Zhang <chao.b.zhang@...>
> > Cc: Jian J Wang <jian.j.wang@...>
> > Cc: Hao A Wu <hao.a.wu@...>
> > Cc: Liming Gao <liming.gao@...>
> > Cc: Jordan Justen <jordan.l.justen@...>
> > Cc: Laszlo Ersek <lersek@...>
> > Cc: Ard Biesheuvel <ard.biesheuvel@...>
> > Cc: Andrew Fish <afish@...>
> > Cc: Ray Ni <ray.ni@...>
> > Cc: Bret Barkelew <brbarkel@...>
> > Signed-off-by: Bret Barkelew <brbarkel@...>
> >
> > v6 changes:
> > * Fix an issue with uninitialized Status in InitVariablePolicyLib()
> > and
> > DeinitVariablePolicyLib()
> > * Fix GCC building in shell-based functional test
> > * Rebase on latest origin/master
> >
> > v5 changes:
> > * Fix the CONST mismatch in VariablePolicy.h and
> > VariablePolicySmmDxe.c
> > * Fix EFIAPI mismatches in the functional unittest
> > * Rebase on latest origin/master
> >
> > v4 changes:
> > * Remove Optional PcdAllowVariablePolicyEnforcementDisable PCD from
> > platforms
> > * Rebase on master
> > * Migrate to new MmCommunicate2 protocol
> > * Fix an oversight in the default return value for
> > InitMmCommonCommBuffer
> > * Fix in VariablePolicyLib to allow ExtraInitRuntimeDxe to consume
> > variables
> >
> > V3 changes:
> > * Address all non-unittest issues with ECC
> > * Make additional style changes
> > * Include section name in hunk headers in "ini-style" files
> > * Remove requirement for the EdkiiPiSmmCommunicationsRegionTable
> > driver
> >   (now allocates its own buffer)
> > * Change names from VARIABLE_POLICY_PROTOCOL and
> > gVariablePolicyProtocolGuid
> >   to EDKII_VARIABLE_POLICY_PROTOCOL and
> > gEdkiiVariablePolicyProtocolGuid
> > * Fix GCC warning about initializing externs
> > * Add UNI strings for new PCD
> > * Add patches for ArmVirtPkg, OvmfXen, and UefiPayloadPkg
> > * Reorder patches according to Liming's feedback about adding to
> platforms
> >   before changing variable driver
> >
> > V2 changes:
> > * Fixed implementation for RuntimeDxe
> > * Add PCD to block DisableVariablePolicy
> > * Fix the DumpVariablePolicy pagination in SMM
> >
> > Bret Barkelew (14):
> >   MdeModulePkg: Define the VariablePolicy protocol interface
> >   MdeModulePkg: Define the VariablePolicyLib
> >   MdeModulePkg: Define the VariablePolicyHelperLib
> >   MdeModulePkg: Define the VarCheckPolicyLib and SMM interface
> >   OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
> >   EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform
> >   ArmVirtPkg: Add VariablePolicy engine to ArmVirtPkg platform
> >   UefiPayloadPkg: Add VariablePolicy engine to UefiPayloadPkg platform
> >   MdeModulePkg: Connect VariablePolicy business logic to
> >     VariableServices
> >   MdeModulePkg: Allow VariablePolicy state to delete protected variables
> >   SecurityPkg: Allow VariablePolicy state to delete authenticated
> >     variables
> >   MdeModulePkg: Change TCG MOR variables to use VariablePolicy
> >   MdeModulePkg: Drop VarLock from RuntimeDxe variable driver
> >   MdeModulePkg: Add a shell-based functional test for VariablePolicy
> >
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> > |  320 +++
> >
> > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> > |  396 ++++
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
> > |   46 +
> >
> >
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDx
> > e.c               |   85 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> > |  816 +++++++
> >
> >
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePo
> > licyUnitTest.c   | 2440 ++++++++++++++++++++
> >
> >
> MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu
> > ncTestApp.c        | 1978 ++++++++++++++++
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
> > |   52 +-
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
> > |   60 +-
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> > |   49 +-
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> > |   53 +
> >
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock
> > .c                    |   71 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
> > |  642 +++++
> >
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> > c                       |   14 +
> >  SecurityPkg/Library/AuthVariableLib/AuthService.c                                        |
> 22
> > +-
> >  ArmVirtPkg/ArmVirt.dsc.inc                                                               |    4 +
> >  EmulatorPkg/EmulatorPkg.dsc                                                              |    3 +
> >  MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
> |
> > 54 +
> >  MdeModulePkg/Include/Library/VariablePolicyHelperLib.h
> > |  164 ++
> >  MdeModulePkg/Include/Library/VariablePolicyLib.h                                         |
> > 207 ++
> >  MdeModulePkg/Include/Protocol/VariablePolicy.h                                           |
> > 157 ++
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> > |   42 +
> >  MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
> > |   12 +
> >
> > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.i
> > nf
> > |   35 +
> >
> > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.u
> > ni
> > |   12 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> > |   44 +
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
> > |   12 +
> >
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
> > |   51 +
> >
> >
> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePo
> > licyUnitTest.inf |   40 +
> >  MdeModulePkg/MdeModulePkg.ci.yaml                                                        |    4
> +-
> >  MdeModulePkg/MdeModulePkg.dec                                                            |   26 +-
> >  MdeModulePkg/MdeModulePkg.dsc                                                            |   15 +
> >  MdeModulePkg/MdeModulePkg.uni                                                            |    7 +
> >  MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> |
> > 11 +
> >  MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md
> > |   55 +
> >
> >
> MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu
> > ncTestApp.inf      |   42 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > |    5 +
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > |    4 +
> >
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i
> > nf                     |   10 +
> >
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > |    4 +
> >  OvmfPkg/OvmfPkgIa32.dsc                                                                  |    5 +
> >  OvmfPkg/OvmfPkgIa32X64.dsc                                                               |    5 +
> >  OvmfPkg/OvmfPkgX64.dsc                                                                   |    5 +
> >  OvmfPkg/OvmfXen.dsc                                                                      |    4 +
> >  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf                                  |
> > 2 +
> >  UefiPayloadPkg/UefiPayloadPkgIa32.dsc                                                    |    4 +
> >  UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc                                                 |    4 +
> >  47 files changed, 8015 insertions(+), 78 deletions(-)  create mode
> > 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeD
> > x
> > e.c
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/Variable
> > Po
> > licyUnitTest.c
> >  create mode 100644
> >
> MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu
> > ncTestApp.c
> >  create mode 100644
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock
> > .c
> >  create mode 100644
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
> >  create mode 100644 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
> >  create mode 100644
> > MdeModulePkg/Include/Library/VariablePolicyHelperLib.h
> >  create mode 100644 MdeModulePkg/Include/Library/VariablePolicyLib.h
> >  create mode 100644 MdeModulePkg/Include/Protocol/VariablePolicy.h
> >  create mode 100644
> > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> >  create mode 100644
> > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.i
> > nf
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.u
> > ni
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
> >  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/Variable
> > Po
> > licyUnitTest.inf
> >  create mode 100644
> > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md
> >  create mode 100644
> >
> MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu
> > ncTestApp.inf
> >
> > --
> > 2.26.2.windows.1.8.g01c50adf56.20200515075929
> >
> >
> >
>
>
>


 


Re: [edk2-platforms] Platform/Intel: Remove unnecessary comments in file header

Sun, Zailiang
 

Reviewed-by: Zailiang Sun <zailiang.sun@...>

-----Original Message-----
From: Gao, Liming <liming.gao@...>
Sent: Tuesday, August 18, 2020 11:16 AM
To: devel@edk2.groups.io
Cc: Sun, Zailiang <zailiang.sun@...>; Qian, Yi <yi.qian@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Dong, Eric <eric.dong@...>
Subject: [edk2-platforms] Platform/Intel: Remove unnecessary comments in file header

Signed-off-by: Liming Gao <liming.gao@...>
Cc: Zailiang Sun <zailiang.sun@...>
Cc: Yi Qian <yi.qian@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Eric Dong <eric.dong@...>
---
.../PlatformDxe/Observable/Observable.c | 12 ++++--------
.../SecBoardInitLibNull/Ia32/SecBoardInit.nasm | 6 ------
.../PlatformInitPei/PlatformEarlyInit.h | 8 --------
3 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformDxe/Observable/Observable.c b/Platform/Intel/Vlv2TbltDevicePkg/PlatformDxe/Observable/Observable.c
index af466ce96d..eeedfc22ee 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformDxe/Observable/Observable.c
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformDxe/Observable/Observable
+++ .c
@@ -1,16 +1,12 @@
-/*++
- This file contains 'Framework Code' and is licensed as such
- under the terms of your license agreement with Intel or your
- vendor. This file may not be modified, except as allowed by
- additional terms of your license agreement.
---*/
/*++

Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved
-

+
+
SPDX-License-Identifier: BSD-2-Clause-Patent

-

+
+


Module Name:
diff --git a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/SecBoardInitLibNull/Ia32/SecBoardInit.nasm b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/SecBoardInitLibNull/Ia32/SecBoardInit.nasm
index 404c03fe06..ffc738fd49 100644
--- a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/SecBoardInitLibNull/Ia32/SecBoardInit.nasm
+++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/SecBoardInitLib
+++ Null/Ia32/SecBoardInit.nasm
@@ -1,9 +1,3 @@
-;
-; This file contains an 'Intel Peripheral Driver' and is -; licensed for Intel CPUs and chipsets under the terms of your -; license agreement with Intel or your vendor. This file may -; be modified by the user, subject to additional terms of the -; license agreement ;; @file ; This is the code that goes from real-mode to protected mode.
; It consumes the reset vector, calls TempRamInit API from FSP binary.
diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformInitPei/PlatformEarlyInit.h b/Platform/Intel/Vlv2TbltDevicePkg/PlatformInitPei/PlatformEarlyInit.h
index 4c6b0795ec..9d9854abe4 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformInitPei/PlatformEarlyInit.h
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformInitPei/PlatformEarlyInit
+++ .h
@@ -16,14 +16,6 @@ Abstract:

--*/

-/*++
- This file contains an 'Intel Peripheral Driver' and is
- licensed for Intel CPUs and chipsets under the terms of your
- license agreement with Intel or your vendor. This file may
- be modified by the user, subject to additional terms of the
- license agreement
---*/
-
#ifndef _EFI_PLATFORM_EARLY_INIT_H_
#define _EFI_PLATFORM_EARLY_INIT_H_

--
2.27.0.windows.1