Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zachary Bobroff zacha...@ami.com<mailto:zacha...@ami.com>
From: Andrew Fish [mailto:af...@apple.com]
Sent: Thursday, June 04, 2015 12:30 PM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] Problem with PiSmmCore Dispatcher.c
On Jun 3, 2015, at 11:39 AM, Zachary Bobroff
<zacha...@ami.com<mailto:zacha...@ami.com>> wrote:
Yao,
Find attached patch file. Also, note that I had my thought process wrong for
C99 vs C89 and moved the variable declarations for SmmTypeIndex and
AprioriIndex to the top of the function to make the compiler more happy. Let
me know if you have any comments/questions.
Zach,
You need to add:
Contributed-under: TianoCore Contribution Agreement 1.0
Thanks,
Andrew Fish
Best Regards,
Zach
From: Zachary Bobroff
Sent: Tuesday, June 02, 2015 6:32 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: RE: Problem with PiSmmCore Dispatcher.c
Yao,
Thanks. Sure, I will create a patch file tomorrow and attach it here.
Best Regards,
Zach
From: Yao, Jiewen [mailto:jiewen....@intel.com]
Sent: Tuesday, June 02, 2015 6:22 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] Problem with PiSmmCore Dispatcher.c
Thanks.
Yes, this is an issue, and should be fixed.
Using SmmTypeIndex and AprioriIndex seems good way for 2nd and 3rd loop.
How about use “HandleIndex” for the 1st loop?
Will you generate a patch? Or we can help generate patch.
Thank you
Yao Jiewen
From: Zachary Bobroff [mailto:zacha...@ami.com]
Sent: Wednesday, June 03, 2015 3:34 AM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: [edk2] Problem with PiSmmCore Dispatcher.c
All,
I hope I am proving this issue the right way, please correct me if I am not. I
also am not sure if this has been already reported. I have come across a
problem in Dispatcher.c within PiSmmCore of MdeModulePkg linked here:
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
The issue is in function “SmmDriverDispatchHandler” on line 1196. Within this
function is a for loop fairly early on line 1235 defined like so:
for (Index = 0; Index < HandleCount; Index++) {
Then on line 1271 there is a another for loop inside the first for loop defined
like so:
for (Index = 0; Index < sizeof (mSmmFileTypes)/sizeof (EFI_FV_FILETYPE);
Index++) {
Then on line 1318 you will find another for loop inside the first for loop (but
outside the second for loop) defined like so:
for (Index = 0; Index < AprioriEntryCount; Index++) {
You will note that all three for loops use the same Index variable defined on
line 1218 like so:
UINTN Index;
The only way I can guess that this has ever worked is that because of the last
for loop Index is set back to 0. Also, most people are not using Apriori for
SMM drivers at this time. Since this is the case, when it loops back around,
it hits the if statement checking if this FV was already processed, if so, it
continues to the next one. The same process would complete for any subsequent
FVs as well.
A proposed fix would be to use an alternate Index variable for each of the
other two for loops. Since these are both only used locally they could be
defined locally like:
for (UINTN SmmTypeIndex = 0; SmmTypeIndex < sizeof (mSmmFileTypes)/sizeof
(EFI_FV_FILETYPE); SmmTypeIndex ++) {
and
for (UINTN AprioriIndex = 0; AprioriIndex < AprioriEntryCount; AprioriIndex ++)
{
Another minor improvement would be to give the overall Index some better name
than just Index for the main loop too.
Best Regards,
Zach
The information contained in this message may be confidential and proprietary
to American Megatrends, Inc. This communication is intended to be read only by
the individual or entity to whom it is addressed or by their designee. If the
reader of this message is not the intended recipient, you are on notice that
any distribution of this message, in any form, is strictly prohibited. Please
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.
The information contained in this message may be confidential and proprietary
to American Megatrends, Inc. This communication is intended to be read only by
the individual or entity to whom it is addressed or by their designee. If the
reader of this message is not the intended recipient, you are on notice that
any distribution of this message, in any form, is strictly prohibited. Please
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.
<patch.txt>------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
The information contained in this message may be confidential and proprietary
to American Megatrends, Inc. This communication is intended to be read only by
the individual or entity to whom it is addressed or by their designee. If the
reader of this message is not the intended recipient, you are on notice that
any distribution of this message, in any form, is strictly prohibited. Please
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.
--- Dispatcher_org.c Wed Jun 3 10:43:06 2015
+++ Dispatcher.c Wed Jun 3 14:08:23 2015
@@ -1257,7 +1257,9 @@
EFI_SMM_DRIVER_ENTRY *DriverEntry;
EFI_GUID *AprioriFile;
UINTN AprioriEntryCount;
- UINTN Index;
+ UINTN HandleIndex;
+ UINTN SmmTypeIndex;
+ UINTN AprioriIndex;
LIST_ENTRY *Link;
UINT32 AuthenticationStatus;
UINTN SizeOfBuffer;
@@ -1274,8 +1276,8 @@
return EFI_NOT_FOUND;
}
- for (Index = 0; Index < HandleCount; Index++) {
- FvHandle = HandleBuffer[Index];
+ for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
+ FvHandle = HandleBuffer[HandleIndex];
if (FvHasBeenProcessed (FvHandle)) {
//
@@ -1310,13 +1312,13 @@
// Discover Drivers in FV and add them to the Discovered Driver List.
// Process EFI_FV_FILETYPE_SMM type and then
EFI_FV_FILETYPE_COMBINED_SMM_DXE
//
- for (Index = 0; Index < sizeof (mSmmFileTypes)/sizeof (EFI_FV_FILETYPE);
Index++) {
+ for (SmmTypeIndex = 0; SmmTypeIndex < sizeof (mSmmFileTypes)/sizeof
(EFI_FV_FILETYPE); SmmTypeIndex++) {
//
// Initialize the search key
//
Key = 0;
do {
- Type = mSmmFileTypes[Index];
+ Type = mSmmFileTypes[SmmTypeIndex];
GetNextFileStatus = Fv->GetNextFile (
Fv,
&Key,
@@ -1357,10 +1359,10 @@
// is only valid for the FV that it resided in.
//
- for (Index = 0; Index < AprioriEntryCount; Index++) {
+ for (AprioriIndex = 0; AprioriIndex < AprioriEntryCount; AprioriIndex++) {
for (Link = mDiscoveredList.ForwardLink; Link != &mDiscoveredList; Link
= Link->ForwardLink) {
DriverEntry = CR(Link, EFI_SMM_DRIVER_ENTRY, Link,
EFI_SMM_DRIVER_ENTRY_SIGNATURE);
- if (CompareGuid (&DriverEntry->FileName, &AprioriFile[Index]) &&
+ if (CompareGuid (&DriverEntry->FileName, &AprioriFile[AprioriIndex]) &&
(FvHandle == DriverEntry->FvHandle)) {
DriverEntry->Dependent = FALSE;
DriverEntry->Scheduled = TRUE;
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
