Re: [edk2] [BaseTools] PACKAGES_PATH issue?

2016-05-20 Thread Andrew Fish
Liming,

I found one more issues. My ACPI tables are breaking the build. It looks like 
the failure is no input files. What I've figured out in GetFileList() is 
FfsInf.EfiOutputPath is constructed incorrectly. The path does not included the 
prefix from the PACKAGES_PATH and thus the directory does not exist and no 
files are found. It looks like the issue is __GetEFIOutPutPath__() as it is 
using self.InfFileName and that has not been adjusted for PACKAGES_PATH?

Thanks,

Andrew Fish

> On May 20, 2016, at 1:34 AM, Gao, Liming  wrote:
> 
> Andrew:
>  Thanks for your point out. It is bug. Your fix is correct. I will send the 
> patch for it. 
> 
> Thanks
> Liming
>> -Original Message-
>> From: af...@apple.com [mailto:af...@apple.com]
>> Sent: Friday, May 20, 2016 1:25 PM
>> To: Gao, Liming 
>> Cc: edk2-devel 
>> Subject: Re: [edk2] [BaseTools] RuleOverride = BINARY code location?
>> 
>> 
>>> On May 19, 2016, at 8:18 PM, Gao, Liming  wrote:
>>> 
>>> Thanks Andrew. I will continue to investigate it.
>>> 
>> 
> F = 'FileNoExtention'
> os.path.splitext(F)[1]
>> ''
> '' in '.out'
>> True
>> 
>> So I think the bug is the `in` it should be `==`
> '' == '.out'
>> False
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>> 
 -Original Message-
 From: af...@apple.com [mailto:af...@apple.com]
 Sent: Friday, May 20, 2016 10:01 AM
 To: Gao, Liming 
 Cc: edk2-devel 
 Subject: Re: [edk2] [BaseTools] RuleOverride = BINARY code location?
 
 
> On May 18, 2016, at 5:44 PM, Andrew Fish  wrote:
> 
>> 
>> On May 18, 2016, at 5:43 PM, Gao, Liming 
>> wrote:
>> 
>> Andrew:
>> |.bin will search the file with the postfix .bin in INF file directory 
>> and its
 output directory. So, there may be more than .bin files are found. Please
>> see
 the code login in GetFileList() from
 C:\R9Tip\edk2\BaseTools\Source\Python\GenFds\Section.py.
>> 
> 
 
 Thanks for the pointer.
 
 This is the code that adding the OS executables in GetFileList() that is
>> adding
 the OS executables without the .bin extension.
   if os.path.exists(Makefile):
   # Update to search files with suffix in all sub-dirs.
   Tuple = os.walk(FfsInf.EfiOutputPath)
   for Dirpath, Dirnames, Filenames in Tuple:
   for F in Filenames:
   if os.path.splitext(F)[1] in (Suffix):
   FullName = os.path.join(Dirpath, F)
   if os.path.getmtime(FullName) >
>> os.path.getmtime(Makefile):
   FileList.append(FullName)
 
 
 I think the issue is:
  if os.path.splitext(F)[1] in (Suffix):
 
 If the file does not have an extension it matches.
 
 Sorry might be wrong have to go out to a writers reading with my wife so
 have to stop looking right now.
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 
 Thanks,
 
 Andrew Fish
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Structured PCD Proposal

2016-05-20 Thread Andrew Fish

> On May 20, 2016, at 12:34 PM, Kinney, Michael D  
> wrote:
> 
> Andrew,
> 
> I hope there is not a misunderstanding in the include file used by a DEC file 
> for a PCD.
> 
> It is not a parallel include file.  The same include file is used in DEC 
> parsing and 
> by modules/libs that access the same Structured PCD.  This is especially 
> important for
> sharing a .h file between a PCD and VFR NV storage sources to eliminate 
> manual maintenance
> of config value offsets in HII type PCD.
> 

Mike,

Sorry for the confusion I assume .h files are the same in both cases, and that 
is an absolute requirement. My point is the current DSC, INF, Conf/*.txt syntax 
supports figuring out the include pathing and passing the build generated 
compiler flags to the compiler, getting code auto generated, and built. Adding 
the C syntax info the .DEC and .DSC files means you have duplicate this 
mechanism. So I guess what I'm saying is if it is C like syntax use an INF, 
just like the C and ACPI code does today since you are really all sharing the C 
include infrastructure. If you try and duplicate build infrastructure with 
different code it is going to break (not get the same result as the INF path) 
in some subtle way at some point.Ssince it is a different syntax we could hit 
and end case where things are not symmetric (for example the INF files can have 
[BuildOptions] settings and new extensions to DEC/DSC don't support that). Also 
extending the DEC and DSC syntax is more new things for a developer
  to learn. 

Basically if it is a C like syntax (like C and ACPI) then we should use an INF 
file like today is what I'm advocating. 

> I only added syntax for some optional tags in comments for fields in the C 
> structure.  
> I want developer to only enter information (C struct in this case) one time.  
> We also 
> want to focus platform configuration information in the one DSC file for 
> simpler platform
> configuration and generate PCD reports that show PCD configuration settings 
> (including
> all field values in a Structured PCD).
> 
> At this time, bit fields are a must have requirement for Structured PCDs.
> 
> Main use case is register values with bit fields with no translation from PCD 
> to 
> register access.  As a result SIZE_OF and OFFSET_OF are not sufficient.  I 
> have ideas
> on how to get bit offset/bit length from C compiler without using CLANG/AST.  
> It was
> just very easy to get this info from CLANG/AST.
> 
> Additional reason is size reduction of configuration data.  Single bit or 
> BOOLEAN 
> like configuration setting takes up minimum of 8-bits today.  If there are 
> many 
> configuration settings like these, then they can add up.
> 

It is not so much as they are not supported, but you don't get an atomic write. 
You have to read modify write them, so 2 calls and a local variable. Actually 
we "solved that problem" in the IoLib by adding IoBitField* functions So 
that would imply what you need is an AND mask and you can hide the read modify 
write behind the library

Thus if you could generate a local, global or #define with the bit filed value 
all 1's and all the other fields 0's you can automate the read modify write 
into a single call. So maybe it is possible?

Do all the compilers support C99? Named structure initialization makes this is 
easy if you know the type of the bitfield and the member name. Well you have to 
suppress the warning. 

~/work/Compiler>cat bitfield.c
typedef struct {
  unsigned int a:2;
  unsigned int b:4;
  unsigned int c:2;
} TEST;

TEST Test0 = { 0 };
TEST Testa = { .a = 0x };
TEST Testb = { .b = 0x };
TEST Testc = { .c = 0x };
~/work/Compiler>clang -S -Wno-bitfield-constant-conversion bitfield.c
~/work/Compiler>cat bitfield.S
.section__TEXT,__text,regular,pure_instructions
.macosx_version_min 10, 11
.section__DATA,__data
.globl  _Test0  ## @Test0
.align  2
_Test0:
.byte   0   ## 0x0
.space  3

.globl  _Testa  ## @Testa
.align  2
_Testa:
.byte   3   ## 0x3
.space  3

.globl  _Testb  ## @Testb
.align  2
_Testb:
.byte   60  ## 0x3c
.space  3

.globl  _Testc  ## @Testc
.align  2
_Testc:
.byte   192 ## 0xc0
.space  3

OK I think I'm convinced now it is likely we will be able to support bitfields.

Thanks,

Andrew Fish

PS Mike if you look into this bitfield thing I can look at prototyping the INF 
based parsing with an example syntax from my previous mail. It will be easier 
to discuss with a prototype to look at.  

Well assuming I can track down the PACKAGES_PATH bug with ACPI tables. 
FfsInf.EfiOutputPath is missing the PACKAGES_PATH part and so I don't get any 
ACPI tables as the 

Re: [edk2] [PATCH] MdePkg/BaseLib: do not rely on undefined behavior in arithmetic shift

2016-05-20 Thread Felix Poludov
Here is another case of shifting a negative value:
It's CoreRestoreTpl (MdeModulePkg\Core\Dxe\Event\Tpl.c):
while (((-2 << NewTpl) & gEventPending) != 0) {

}

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gao, 
Liming
Sent: Thursday, December 17, 2015 4:29 AM
To: Ard Biesheuvel; edk2-devel@lists.01.org
Cc: dw...@infradead.org
Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: do not rely on undefined behavior 
in arithmetic shift

Ard:
  Have you found other similar cases in EDKII project? Or, it this the only one?

Thanks
Liming
-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Wednesday, December 16, 2015 6:49 PM
To: edk2-devel@lists.01.org; Gao, Liming
Cc: dw...@infradead.org; Ard Biesheuvel
Subject: [PATCH] MdePkg/BaseLib: do not rely on undefined behavior in 
arithmetic shift

The runtime test whether the compiler supports arithmetic shift of negative 
signed numbers currently relies on undefined behavior in C, which means that 
all bets are off regarding whether the condition that follows passes or fails, 
regardless of whether the compiler in fact supports arithmetic shift or not.

Relevant quote from ISO C99 (6.5.7/4)

  The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits
  are filled with zeros. If E1 has an unsigned type, the value of the result
  is E1 × 2^E2, reduced modulo one more than the maximum value representable
  in the result type. If E1 has a signed type and nonnegative value, and
  E1 × 2^E2 is representable in the result type, then that is the resulting
  value; otherwise, the behavior is undefined.

For historic purposes, let's keep the test in place (although it is doubtful we 
actually need it) but rewrite it in a way that prevents compilers from this 
century from doing whacky things with it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---

Starting with version 3.7, Clang warns about the use of a negative left hand 
operand when performing arithmetic shift left, leading to build failures under 
-Werror.

 MdePkg/Library/BaseLib/Math64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/Math64.c b/MdePkg/Library/BaseLib/Math64.c 
index 83d76847213e..9624cf90029f 100644
--- a/MdePkg/Library/BaseLib/Math64.c
+++ b/MdePkg/Library/BaseLib/Math64.c
@@ -86,7 +86,7 @@ InternalMathARShiftU64 (
   //
   // Test if this compiler supports arithmetic shift
   //
-  TestValue = (((-1) << (sizeof (-1) * 8 - 1)) >> (sizeof (-1) * 8 - 1));
+  TestValue = (INTN)((INT64)(1ULL << 63) >> 63);
   if (TestValue == -1) {
 //
 // Arithmetic shift is supported
--
2.5.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/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.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] TCP4: Failure to Acknowledge due to DPC Dispatch Nesting

2016-05-20 Thread Cohen, Eugene

We have isolated a problem where the TCP4 driver fails to acknowledge received 
data under certain circumstances.

Background on DPCs: The DPC mechanism allows functions to be called at a later 
point in time at a requested TPL level.  Functions are queued through the DPC 
Protocol's QueueDpc interface.  DPCs are dispatched anytime a module calls the 
DispatchDpc function in the DPC protocol.  The dispatch process will execute 
all queued DPCs that were registered to execute at or above the caller's TPL 
level (e.g. if caller is at TPL_CALLBACK the DPC dispatch will execute anything 
between CALLBACK and HIGH_LEVEL).

The stack depends on DispatchDpc being called at appropriate preemption points 
to advance packet processing.  The dispatch function is called in multiple 
layers as you can see by searching for DispatchDpc with calls originating from 
ArpDxe, Ip4Dxe, MnpDxe, Udp4Dxe, UefiPxeBcDxe, DnsDxe, HttpDxe, Ip6Dxe and 
Udp6Dxe.

Currently it's possible for DPC dispatching to occur in a nested manner.  
Imagine a case where an upper network stack layer queues a DPC (for example the 
TCP layer's TcpTickingDpc) and in the DPC handler it makes use of lower layers 
(like sending a packet through IP+MNP).  As part of packet processing these 
lower layers will call DispatchDpc resulting in nested DPCs execution.


Here's an example of the DPC nesting, the indent level indicates the level of 
nesting for the DPC

TcpTicking
DpcDispatchDpc TPL=CALLBACK
TcpTickingDpc
TcpTickingDpc Tcb: 0x49a42dd0, DelayedAck=1, CtrlFlag: 0x1019
TcpTickingDpc call TcpSendAck delayed is: 1
TcpSendAck  Seq=158464588 Ack=4152002304
TcpTransmitSegment DelayedAck = 0
Nbuf SendIpPacket: DestPort=62462 Seq=158464588 Ack=4152002304 Window=19840
MnpSyncSendPacket call DispatchDpc
DpcDispatchDpc TPL=CALLBACK
Ip4AccpetFrame call DispatchDpc
DpcDispatchDpc TPL=CALLBACK
Ip4FreeTxToken call DispatchDpc
DpcDispatchDpc TPL=CALLBACK
TcpInput: DestPort=8816 Seq=4152002304 Ack=158464588 Len=1460
TcpClearTimer Tcb: 0x49a42dd0

Notice how the process of sending the TCP ACK via IP then MNP causes another 
DispatchDpc to occur before the TCP segment transmit call returns.  This 
nesting continues on and a whole bunch of code has executed (all at CALLBACK 
TPL).  You can see near the end that we even begin processing another TCP 
packet.


If we look in detail what the TcpSendAck does there are two key steps:

  if (TcpTransmitSegment (Tcb, Nbuf) == 0) {
TCP_CLEAR_FLG (Tcb->CtrlFlag, TCP_CTRL_ACK_NOW);
Tcb->DelayedAck = 0;
  }

It transmits the segment and after the transmit returns it clears the 
DelayedAck counter since the presumption is that we sent an ACK for the most 
recent segment and we are caught up.  But because DPCs are dispatched within 
TcpTransmitSegment the assumption that this transmit was the last one is 
incorrect.


Here is a portion of a trace illustrating the problem:

TcpTicking
DpcDispatchDpc TPL=CALLBACK
TcpTickingDpc
TcpTickingDpc Tcb: 0x49a42dd0, DelayedAck=1, CtrlFlag: 0x1019
TcpTickingDpc call TcpSendAck delayed is: 1
TcpSndAck  Seq=158464588 Ack=4152002304
TcpTransmitSegment DelayedAck = 0
Nbuf SendIpPacket: DestPort=62462 Seq=158464588 Ack=4152002304 Window=19840
MnpSyncSendPacket call DispatchDpc
DpcDispatchDpc TPL=CALLBACK

[snip - a bunch of nested DPC processing removed]

 DpcDispatchDpc Tpl= TPL=CALLBACK
  TcpInput: DestPort=8816 Seq=4152019824 Ack=158464588 Len=1460
  TcpClearTimer Tcb: 0x49a42dd0
  TcpInput Seq=4152019824 Tcb: 0x49a42dd0, Tcb->DupAck = 0
  TcpToSendAck add to delayedack Seq=158464588 Ack=4152021284
  TcpToSendAck add to delayedack Tcb: 0x49a42dd0, Seq=158464588 
Ack=4152021284, DelayedAck=1
  ^^ NOTE: the DelayedAck flag has been set to one indicating that we 
haven't acknowledged yet and need to soon

  [we return from 14 nested DPC calls !!]

TcpSndAck  Tcb->DelayedAck = 0
^^ But when the Dispatch returns from the TcpTransmitSegment we clear 
DelayedAck back to zero such that we never acknowledge the last packet we 
received.

TcpTickingDpc No timer active or expired
TcpTickingDpc Tcb: 0x49918bd0, DelayedAck=0, CtrlFlag: 0x1019
TcpTickingDpc No timer active or expired

In cases where TCP is waiting for more data from the remote endpoint and the 
endpoint is waiting for acknowledgement (like a situation where the sliding 
window is small or closed) the remote endpoint will retransmit the last packet 
but for reasons I don't yet understand duplicate acks aren't sent (I think 
there's another bug around retransmitting acks but I haven't isolated it).   
The result is that the connection is reset by the remote endpoint after a 
timeout period elapses.


This seems kind of like a critical section / locking problem around the 
DelayedAck member.  I'm not sure if the issue 

Re: [edk2] [RFC] Structured PCD Proposal

2016-05-20 Thread Kinney, Michael D
Andrew,

I hope there is not a misunderstanding in the include file used by a DEC file 
for a PCD.

It is not a parallel include file.  The same include file is used in DEC 
parsing and 
by modules/libs that access the same Structured PCD.  This is especially 
important for
sharing a .h file between a PCD and VFR NV storage sources to eliminate manual 
maintenance
of config value offsets in HII type PCD.

I only added syntax for some optional tags in comments for fields in the C 
structure.  
I want developer to only enter information (C struct in this case) one time.  
We also 
want to focus platform configuration information in the one DSC file for 
simpler platform
configuration and generate PCD reports that show PCD configuration settings 
(including
all field values in a Structured PCD).

At this time, bit fields are a must have requirement for Structured PCDs.

Main use case is register values with bit fields with no translation from PCD 
to 
register access.  As a result SIZE_OF and OFFSET_OF are not sufficient.  I have 
ideas
on how to get bit offset/bit length from C compiler without using CLANG/AST.  
It was
just very easy to get this info from CLANG/AST.

Additional reason is size reduction of configuration data.  Single bit or 
BOOLEAN 
like configuration setting takes up minimum of 8-bits today.  If there are many 
configuration settings like these, then they can add up.

Mike

> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Friday, May 20, 2016 12:15 PM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] [RFC] Structured PCD Proposal
> 
> 
> > On May 20, 2016, at 10:48 AM, Kinney, Michael D 
> >  wrote:
> >
> > Andrew,
> >
> > 1) I think the strongest concern is on use of CLANG/AST.  I will look at 
> > your ideas
> and
> >   some other ideas I have thought of as well to use standard C compiler 
> > that is
> already
> >   installed to build FW to support Structured PCD features.
> >
> > 2) The other concern I see here is extending PcdLib to access fields.  You 
> > prefer
> to
> >   not add those APIs and always read/write entire structure.  I have 
> > requests to
> support
> >   field access, so that is a bit of a requirement conflict.
> >
> 
> Mike,
> 
> Given the complexity we ended up with the basic edk2 design, that was also 
> driven by
> complex requirements, I wanted to start the conversation of how some of the
> requirements maybe driving a disproportionate amount of the complexity. I 
> would hate
> for a "nice to have" feature drive 80% of the complexity. I'm still feeling 
> guilty
> about those PCD section names
> 
> I think there are two vectors of complexity:
> 1) Implementation  - Requiring clang/AST
> I think stepping away with the assumption of clang/AST being required helps 
> this
> vector out a lot.
> 
> 2) Workflow - how a developer uses the feature day to day.
> I think my comments about the default values in the .DEC relate to the 
> complexities a
> developer will face. As you point out there is going to be a .DEC grammar 
> extension
> required just to support this feature. Also if I understand correctly 
> changing a
> #define or a structure definition in another package could change the 
> default. We
> currently don't have this behavior and I think that will lead to more 
> confusion and
> hard to track down bugs. Thus I think we should consider sticking with the 
> current
> .DEC syntax for the default variable if it is required. We could advocate 
> comments
> that explain the fields and maybe even output information like that in build 
> log PCD
> section so you can copy paste into the .DEC.
> 
> Using an INF file and new file type for the structured PCD is also trying to 
> manage
> developer complexity by making it symmetric with C and ACPI code. You can 
> make the
> argument that PCDs always worked a certain way, but I will make the a counter
> argument that managing the C include file infrastructure has always worked a 
> certain
> way too, and that is the reason to consider not building a parallel include
> infrastructure definition for the .DSC (and .DEC) files and use an INF file, 
> not to
> mention it is probably easier to implement since it is a solved problem.
> 
> 
> I was also thinking about the PcdLib field accessors. If you don't support 
> bit fields
> and don't need pedantic type safety I think this idea may get you 80% of what 
> you
> want (I'm guessing you might have already thought of this). If the build 
> system knows
> the C type of the structure and places it in the AutoGen.h and you know the 
> structure
> member name in the calling code the Code calling the PcdLib.h can derive the
> structure offset and size of the field.  It may also be possible to verify 
> the size
> of the argument via a sizeof() in a macro.
> 
> I just double checked that nested structures/unions would work, and 

Re: [edk2] [RFC] Structured PCD Proposal

2016-05-20 Thread Andrew Fish

> On May 20, 2016, at 10:48 AM, Kinney, Michael D  
> wrote:
> 
> Andrew,
> 
> 1) I think the strongest concern is on use of CLANG/AST.  I will look at your 
> ideas and 
>   some other ideas I have thought of as well to use standard C compiler that 
> is already
>   installed to build FW to support Structured PCD features.
> 
> 2) The other concern I see here is extending PcdLib to access fields.  You 
> prefer to 
>   not add those APIs and always read/write entire structure.  I have requests 
> to support
>   field access, so that is a bit of a requirement conflict.
> 

Mike,

Given the complexity we ended up with the basic edk2 design, that was also 
driven by complex requirements, I wanted to start the conversation of how some 
of the requirements maybe driving a disproportionate amount of the complexity. 
I would hate for a "nice to have" feature drive 80% of the complexity. I'm 
still feeling guilty about those PCD section names

I think there are two vectors of complexity:
1) Implementation  - Requiring clang/AST
I think stepping away with the assumption of clang/AST being required helps 
this vector out a lot. 

2) Workflow - how a developer uses the feature day to day. 
I think my comments about the default values in the .DEC relate to the 
complexities a developer will face. As you point out there is going to be a 
.DEC grammar extension required just to support this feature. Also if I 
understand correctly changing a #define or a structure definition in another 
package could change the default. We currently don't have this behavior and I 
think that will lead to more confusion and hard to track down bugs. Thus I 
think we should consider sticking with the current .DEC syntax for the default 
variable if it is required. We could advocate comments that explain the fields 
and maybe even output information like that in build log PCD section so you can 
copy paste into the .DEC. 

Using an INF file and new file type for the structured PCD is also trying to 
manage developer complexity by making it symmetric with C and ACPI code. You 
can make the argument that PCDs always worked a certain way, but I will make 
the a counter argument that managing the C include file infrastructure has 
always worked a certain way too, and that is the reason to consider not 
building a parallel include infrastructure definition for the .DSC (and .DEC) 
files and use an INF file, not to mention it is probably easier to implement 
since it is a solved problem. 


I was also thinking about the PcdLib field accessors. If you don't support bit 
fields and don't need pedantic type safety I think this idea may get you 80% of 
what you want (I'm guessing you might have already thought of this). If the 
build system knows the C type of the structure and places it in the AutoGen.h 
and you know the structure member name in the calling code the Code calling the 
PcdLib.h can derive the structure offset and size of the field.  It may also be 
possible to verify the size of the argument via a sizeof() in a macro. 

I just double checked that nested structures/unions would work, and clang looks 
happy. 

#include 

#define OFFSET_OF(TYPE, Field)  __builtin_offsetof(TYPE, Field)
#define SIZE_OF(TYPE, Field) (sizeof(((TYPE *)0)->Field))

typedef struct {
  int a;
  int b;
  int c;
  int leaf;
} STRUCT_LEAF;

typedef struct {
  STRUCT_LEAF   Leaf;
  int   i;
} STRUCT;

int
main ()
{
  STRUCT S;

  printf ("Offset 0x%lx size 0x%lx\n", OFFSET_OF(STRUCT, Leaf.leaf), 
SIZE_OF(STRUCT, Leaf.leaf));
  return 0;
}

So maybe the complexity tradeoff is how you support bitfields (union with a 
real type and granularity of access is that type) and how pedantic the type 
checking (C is not type safe, so this is no different than a cast in the C 
code) is going to be for the PcdLib structure field accessor. 

Thanks,

Andrew Fish

> More responses below.
> 
> Mike
> 
> 
>> -Original Message-
>> From: af...@apple.com [mailto:af...@apple.com]
>> Sent: Thursday, May 19, 2016 3:25 PM
>> To: Kinney, Michael D 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] [RFC] Structured PCD Proposal
>> 
>> 
>>> On May 19, 2016, at 12:16 PM, Kinney, Michael D 
>>>  wrote:
>>> 
>>> Andrew,
>>> 
>>> My responses embedded below.
>>> 
>> 
>> Mike,
>> 
>> Likewise.
>> 
>>> Mike
>>> 
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
 Andrew Fish
 Sent: Thursday, May 19, 2016 10:53 AM
 To: Kinney, Michael D ; 
 edk2-devel@lists.01.org >>> de...@ml01.01.org>
 Subject: Re: [edk2] [RFC] Structured PCD Proposal
 
 
> On May 19, 2016, at 2:26 AM, Laszlo Ersek  wrote:
> 
> On 05/19/16 01:28, Kinney, Michael D wrote:
> 
> [snip]
> 
>> ## C Structure Syntax Proposal
>> * Use C 

Re: [edk2] [PATCH] MdePkg/WSMT.h: update header comment to use official URL link.

2016-05-20 Thread El-Haj-Mahmoud, Samer
Thanks for fixing it.. I was wondering what that URL was, and had to ping MS 
about it.

Reviewed-by: Samer El-Haj-Mahmoud 



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiewen 
Yao
Sent: Thursday, May 19, 2016 7:58 PM
To: edk2-devel@lists.01.org
Cc: Michael D Kinney ; Liming Gao 

Subject: [edk2] [PATCH] MdePkg/WSMT.h: update header comment to use official 
URL link.

Update WSMT table link to official MSDN URL.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h 
b/MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h
index bfbcf81..2b0a644 100644
--- a/MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h
+++ b/MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h
@@ -1,6 +1,6 @@
 /** @file
   Defines Windows SMM Security Mitigation Table
-  @ 
http://download.microsoft.com/download/1/8/A/18A21244-EB67-4538-BAA2-1A54E0E490B6/WSMT.docx
+  @ 
https://msdn.microsoft.com/windows/hardware/drivers/bringup/acpi-system-description-tables#wsmt
 
   Copyright (c) 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials 
-- 
2.7.4.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.

2016-05-20 Thread El-Haj-Mahmoud, Samer
Yes this is probably better than HTTP drivers directly printing on console



-Original Message-
From: Fu, Siyuan [mailto:siyuan...@intel.com] 
Sent: Thursday, May 19, 2016 9:04 PM
To: El-Haj-Mahmoud, Samer ; Zhang, Lubo 
; edk2-devel@lists.01.org
Cc: Ye, Ting ; Wu, Jiaxin 
Subject: RE: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.

Hi, Samer

I think in PXE this issue was solved by the 
EFI_PXE_BASE_CODE_CALLBACK_PROTOCOL, which allows the platform owner to provide 
an callback implementation which will take precedence over the default 
implementation in PXE driver. The callback protocol allows the platform owner 
to display the process messages or even abort the PXE boot. Do you think we 
could reference this practice in HTTP boot?

Best Regards
Siyuan

> -Original Message-
> From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Thursday, May 19, 2016 9:46 PM
> To: Zhang, Lubo ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; 
> Wu, Jiaxin 
> Subject: RE: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.
> 
> Thanks Zhang.
> 
> I know this makes sense (and does help during lengthy download), and I 
> also know that PXE does something similar. I am only concerned about 
> this happening after BGRT logo is displayed on the screen (if PXE/HTTP 
> boot options are tried after a failed HDD boot option for instance). 
> The result is that the messages will be printed on top of the already 
> displayed logo, which is also reported in ACPI BGRT table. It may not 
> be a big issue, but it did result in some user complaints at times. What do 
> you think?
> 
> Other than that, I would improve the messaging a bit. See feedback below.
> 
> 
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Zhang Lubo
> Sent: Wednesday, May 18, 2016 10:53 PM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting ; Fu Siyuan ; Wu 
> Jiaxin 
> Subject: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.
> 
> When downloading a big image as ram disk iso,we can print the progress 
> message on screen to enhance the user experience.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c  | 29
> +
> NetworkPkg/HttpBootDxe/HttpBootSupport.c |  4 ++--
>  2 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 46cf9ca..9b2a8bd 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -753,10 +753,12 @@ HttpBootGetBootFile (
>HTTP_BOOT_CACHE_CONTENT*Cache;
>UINT8  *Block;
>CHAR16 *Url;
>BOOLEANIdentityMode;
>UINTN  ReceivedSize;
> +  UINTN  Ratio;
> +  UINTN  NewRatio;
> 
>ASSERT (Private != NULL);
>ASSERT (Private->HttpCreated);
> 
>if (BufferSize == NULL || ImageType == NULL) { @@ -992,10 +994,13 
> @@ HttpBootGetBootFile (
>  //
>  // 3.4.2, start the message-body download, the identity and 
> chunked transfer-coding
>  // is handled in different path here.
>  //
>  ZeroMem (, sizeof (HTTP_IO_RESPONSE_DATA));
> +AsciiPrint ("\n");
> +AsciiPrint ("\n  Downloading NBP file... ");
> 
> 
> [Samer] How about printing the URL where the file is being downloaded 
> from so the user knows that this is happening from HTTP Boot and not 
> PXE (since the messages are identical)
> 
> 
> 
> +Ratio = 0;
>  if (IdentityMode) {
>//
>// In identity transfer-coding there is no need to parse the message 
> body,
>// just download the message body to the user provided buffer directly.
>//
> @@ -1010,10 +1015,25 @@ HttpBootGetBootFile (
> );
>  if (EFI_ERROR (Status)) {
>goto ERROR_6;
>  }
>  ReceivedSize += ResponseBody.BodyLength;
> +//
> +// Display the download progress.
> +//
> +NewRatio = (ReceivedSize * 100) / ContentLength;
> +if (NewRatio != Ratio) {
> +  if (Ratio !=0) {
> +if (Ratio >=10) {
> +  AsciiPrint ("\b\b\b");
> +} else {
> +  AsciiPrint ("\b\b");
> +}
> +  }
> +  Ratio = NewRatio;
> +  AsciiPrint ("%d%%", NewRatio);
> +}
>}
>  } else {
>//
>// In "chunked" 

Re: [edk2] [RFC] Structured PCD Proposal

2016-05-20 Thread Kinney, Michael D
Andrew,

1) I think the strongest concern is on use of CLANG/AST.  I will look at your 
ideas and 
   some other ideas I have thought of as well to use standard C compiler that 
is already
   installed to build FW to support Structured PCD features.

2) The other concern I see here is extending PcdLib to access fields.  You 
prefer to 
   not add those APIs and always read/write entire structure.  I have requests 
to support
   field access, so that is a bit of a requirement conflict.

More responses below.

Mike


> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Thursday, May 19, 2016 3:25 PM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] [RFC] Structured PCD Proposal
> 
> 
> > On May 19, 2016, at 12:16 PM, Kinney, Michael D 
> >  wrote:
> >
> > Andrew,
> >
> > My responses embedded below.
> >
> 
> Mike,
> 
> Likewise.
> 
> > Mike
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> >> Andrew Fish
> >> Sent: Thursday, May 19, 2016 10:53 AM
> >> To: Kinney, Michael D ; 
> >> edk2-devel@lists.01.org  >> de...@ml01.01.org>
> >> Subject: Re: [edk2] [RFC] Structured PCD Proposal
> >>
> >>
> >>> On May 19, 2016, at 2:26 AM, Laszlo Ersek  wrote:
> >>>
> >>> On 05/19/16 01:28, Kinney, Michael D wrote:
> >>>
> >>> [snip]
> >>>
>  ## C Structure Syntax Proposal
>  * Use C typedef syntax to describe complex C structures
>  * Use a C language parser with support for struct/union/array/bit 
>  fields/pack
>  * Recommend use of libclang C language parser into Abstract Syntax Tree 
>  (AST)
>    - Included with LLVM release
>    - http://llvm.org/releases/download.html
>    - http://clang.llvm.org/doxygen/group__CINDEX.html
>  * Recommend use of Python bindings for CLANG for EDK II Python based 
>  BaseTools
>    - pip install clang
> >>>
> >>> What versions are necessary? On RHEL-7, the following versions are
> >>> available (from EPEL only; RHEL does not provide clang):
> >>>
> >>> clang-devel: 3.4.2-8.el7
> >>> python-pip: 7.1.0-1.el7
> >>>
> >>
> >> Will it be possible to check everything into the edk2 git tree? Currently 
> >> we only
> >> require the developer to have Xcode installed and everything else is in 
> >> the git
> tree.
> >> I'm not sure it is possible to run pip on our production build servers?
> >
> > Is clang and clang libs included in your environment?
> 
> clang is the compiler for Xcode so we have that for sure. I'm not sure about 
> the libs
> but they are likely accessible.
> 
> >  That is not edk2 git today.
> > The only other component is the Python wrapper on top of the clang lib and 
> > that
> > is available from another github project if you want to pull python sources 
> > to it.
> >
> 
> If is just Python code it should not be too complex?
> 
> >>
> >>
> 
>  ## DEC File Extensions
>  * Declare link between PCD and C data structure and token range for 
>  fields
>  * @Include path to include file with C typedef [Required]
>  * Replace |VOID*| with name of C typedef
>  * @TokenMap path to file with token number assignments [Optional]
>  * Range of token numbers to assign to fields in C typedef [Required]
>  * Example:
> 
>  ```
>  # @Include  Include/Pcd/AcpiLowerPowerIdleTable.h
>  # @TokenMap Include/Pcd/AcpiLowerPowerIdleTable.map
> 
> >>
> gPlatformTokenSpaceGuid.AcpiLowPowerIdleTable|{}|ACPI_LOW_POWER_IDLE_TABLE|0x00010080
> >> |0x0E000200-0x0E0002FF
>  ```
> >>>
> >>> What does 0x00010080 mean here?
> >>>
> 
>  * Recommended File Paths
>    - /Include/Pcd/.h
>    - /Include/Pcd/.map  [Optional]
>    - /Include/Pcd/.uni  [Optional]
>  * C Pre-Processor Support
>    - Use of #include, #define, #if supported
>    - #include limited to files in same package
>    - Including files from other packages being evaluated
> 
> >>
> >> I think you need to share the C headers with the edk2 C and ASL code, so 
> >> you
> should
> >> have full access including all the things set in the [BuildOptions] flags 
> >> in the
> DSC.
> >> So use something like PP_FLAGS or ASLPP_FLAGS and allow customization via 
> >> the
> >> Conf/build_rule.txt. I'd rather we not makeup a new thing and just use one 
> >> of the
> >> existing pre processor schemes.
> >
> > I agree.  Any flags set up in [BuildOptions] or -D flags on command line 
> > would be
> > passed in, so the .h file associated with the Structured PCD is treated the 
> > same
> > as any other .h file used to build modules/libs.  There is no intention to 
> > create
> > any new build rules to support Structured PCDs.
> 
> OK I was confused by this statement. "Including files from other packages 
> being
> evaluated". The more I think about it it is 

Re: [edk2] Driver dependency on boot

2016-05-20 Thread Andrew Fish

> On May 20, 2016, at 12:36 AM, Iru Cai  wrote:
> 
> It's a good idea, but I still have some problems. My ccidboot driver has
> different behaviors depending on whether a CCID card is plugged in, so I
> think I also need a timer event to check if gEfiCcidProtocolGuid is
> installed to a handle in time.

No that is a bad idea. It sounds to me like you have some kind of fundamental 
design flaw. If I had to guess it has to do with understanding the EFI Driver 
Model. 

This is a good reference on writing an UEFI Driver: 
https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide

>  However, I need the code executed before
> the boot manager starts.

Why? 

> In my case in OVMF, if I set a time too long in
> the timer event, its notification function will run after the EFI shell
> starts, and this is not what I want.
> 
> So now I need this in my ccidboot: when a CCID card is plugged in, it can
> wait for gEfiCcidProtocolGuid installed to a handle, and if no card is
> plugged in before BDS phase, it'll run other code.
> 

I'm guessing the issue is  you are trying to mix UEFI Driver Model code with 
non UEFI Driver Model code. 

A UEFI Driver Model driver registers a Driver Binding Protocol. So basically 
when a driver is loaded all it does is install a few protocos and returns. This 
is very different than the legacy BIOS as the option ROM basically takes over 
the system. When we designed EFI we got a lot of feedback from option ROM 
writers that they had to put in work arounds for the BIOS, the BIOS vendors 
mentioned they had to put in work arounds for option ROMs, and the big OEMs 
complained all these work arounds caused chaos, and at times to make the 
platforms work correctly, or follow the correct platform policy, they would 
have to get a custom version of the option ROM produces. Given the state of 
BIOS we decided the best approach would be to make the option ROM (UEFI Driver) 
as simple as possible, and move all the policy into the platform. 

The BDS (Boot Manager in UEFI Spec speak, BDS is a PI Spec thing) is executing 
the platform policy and it is the one Starting the drivers (via 
gBS->ConnectController()). What devices gets started is platform policy, so for 
a fast boot only boot devices may be started. The devices are connected 
recursively so if you connect a Block IO device, that partition driver will get 
connected and produce child Block Io handles, and the file system driver will 
also get connected. 

I'd also point out given the UEFI design is to concentrate the the policy in 
platform you may need to customize the BDS if you are trying to change some 
kind of fundamental platform policy. 

Thanks,

Andrew Fish

> Thanks
> Iru
> 
> On Thu, May 19, 2016 at 3:22 PM, Gao, Liming  wrote:
> 
>> You can create ProtocolNotification function for gEfiCcidProtocolGuid in
>> ccidboot driver. If so, you don't need add it into [Depex].
>> UefiLib EfiCreateProtocolNotifyEvent() API can help to create it.
>> 
>> Thanks
>> Liming
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Iru
>>> Cai
>>> Sent: Thursday, May 19, 2016 2:35 PM
>>> To: edk2-devel@lists.01.org
>>> Subject: [edk2] Driver dependency on boot
>>> 
>>> Hi,
>>> 
>>> I'm having some dependency problem when developing on edk2. I've
>>> written a
>>> CCID device driver which provides a protocol EFI_CCID_PROTOCOL with GUID
>>> gEfiCcidProtocolGuid. And I have another application called ccidboot to
>> be
>>> run on boot that uses gEfiCcidProtocolGuid, I set the module type as
>>> DXE_DRIVER.
>>> 
>>> If I add gEfiCcidProtocolGuid to the [Depex] of ccidboot, it works good
>>> when I have my CCID card plugged, but it'll not run if the card is
>>> unplugged. If I don't add this dependency, the ccidboot module will run
>>> before the driver starts the card. I don't know what I should do to solve
>>> this problem.
>>> 
>>> Thanks,
>>> Iru
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Structured PCD Proposal

2016-05-20 Thread Kinney, Michael D
Laszlo,

Your feedback is consistent with Andrews.  

I will see if we can implement the Structured PCD feature without using 
CLANG frontend and Python bindings.  CLANG/Python was a very valuable 
toolset to prototype and evaluate a number of design options quickly.

Andrew has suggested maximizing the use of the compiler that is used to
build the EDK II modules to extract the information required, and I think
that is the best option to explore based on the feedback received so far.

Thanks,

Mike


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Friday, May 20, 2016 2:01 AM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] [RFC] Structured PCD Proposal
> 
> On 05/19/16 19:39, Kinney, Michael D wrote:
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> >> Laszlo
> Ersek
> >> Sent: Thursday, May 19, 2016 2:26 AM
> >> To: Kinney, Michael D 
> >> Cc: edk2-devel@lists.01.org 
> >> Subject: Re: [edk2] [RFC] Structured PCD Proposal
> >>
> >> On 05/19/16 01:28, Kinney, Michael D wrote:
> >>
> >> [snip]
> >>
> >>> ## C Structure Syntax Proposal
> >>>   * Use C typedef syntax to describe complex C structures
> >>>   * Use a C language parser with support for struct/union/array/bit 
> >>> fields/pack
> >>>   * Recommend use of libclang C language parser into Abstract Syntax Tree 
> >>> (AST)
> >>> - Included with LLVM release
> >>> - http://llvm.org/releases/download.html
> >>> - http://clang.llvm.org/doxygen/group__CINDEX.html
> >>>   * Recommend use of Python bindings for CLANG for EDK II Python based 
> >>> BaseTools
> >>> - pip install clang
> >>
> >> What versions are necessary? On RHEL-7, the following versions are
> >> available (from EPEL only; RHEL does not provide clang):
> >>
> >> clang-devel: 3.4.2-8.el7
> >> python-pip: 7.1.0-1.el7
> >
> > I have not tried the prototype on RHEL-7 yet.  For Fedora 23, we have used:
> >
> >   $ sudo dnf install clang
> 
> 
> Up to and including Fedora 23, clang had been built from the source
> package called "llvm", so Koji and pkgdb must be searched for "llvm",
> when someone is looking for Fedora 23 packages on the web (i.e., not
> with "dnf" or "yum"). In Fedora 24, "clang" got its own SRPM:
> 
> http://thread.gmane.org/gmane.linux.redhat.fedora.devel/216194
> https://bugzilla.redhat.com/show_bug.cgi?id=1300945
> 
> So, looking for "llvm" packages, it seems to me that the last Fedora
> releases that shipped 3.4-based clang binaries were Fedora 21, and
> Fedora 22:
> 
> http://koji.fedoraproject.org/koji/buildinfo?buildID=552862
> http://koji.fedoraproject.org/koji/buildinfo?buildID=552863
> 
> (However, Fedora 22 later moved to the 3.5 series.)
> 
> Why this matters: on RHEL-7 I'll only be able to use 3.4.2-based clang
> (from EPEL-7). I think testing the BaseTools feature in a fully updated
> Fedora 21 installation (3.4-based clang) would allow you to extrapolate
> the results to EPEL-7:
> 
> Fedora 23: 3.7.0-4   works
> Fedora 21: 3.4-15?
> EPEL-7:3.4.2-8   ?
> 
> Could you please test such a configuration?
> 
> Alternatively, if you have a public branch, and testing instructions,
> I'd be happy to check them on my RHEL-7 system, using clang 3.4.2-8
> (from EPEL-7).
> 
> >   $ git clone -b release_37 https://github.com/trolldbois/python-clang
> >   $ cd python-clang
> >   $ sudo python setup.py install
> 
> Unfortunately, this looks very bad. I shouldn't have to install, as
> root, a Python package from a random github repository, in order to
> build edk2 platforms.
> 
> At this point, quite a few downstreams (e.g., Linux distributions) build
> OVMF packages. If the Structured PCD Feature is put to use in such
> generic edk2 modules that OVMF pulls in (which I think will likely
> happen), then all these downstreams are going to require the python
> bindings for clang as native packages.
> 
> ... Actually, the README file at
>  says:
> 
> OBSOLETE. LLVM-CLANG NOW PUBLISHES PYTHON PACKAGE. JUST ADD THE
> OFFICIAL llvm-3.7 repo in your apt.
> 
> If the above statement -- about llvm-3.7 including the python bindings
> for clang -- is correct, then Fedora 23 and later might be all set,
> because their llvm and clang stuff does not predate 3.7. (See Fedora 23
> above, for example: 3.7.0-4).
> 
> However, this dependency leaves a number of Linux distributions in the
> cold; distros that are still supported
> - by their vendors,
> - and by edk2 in general (for example through the now-antique GCC44
>   toolchain).
> 
> I think if clang and its python bindings become an unconditional
> dependency for building edk2, then:
> 
> (1) Some Linux distributions that can currently build edk2 platforms
> (through the older GCC toolchains) 

Re: [edk2] [Patch] BaseTools: Fix GenFds issue to wrongly get file without postfix.

2016-05-20 Thread Andrew Fish
Reviewed-by: Andrew FIsh 

Thanks you for the quick fix.

Andrew Fish

> On May 20, 2016, at 1:42 AM, Liming Gao  wrote:
> 
> GenFds GenSection will search the output file based on the file extension.
> If the output file has no extension, it should be skip.
> 
> Cc: Andrew Fish 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
> BaseTools/Source/Python/GenFds/Section.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/Section.py 
> b/BaseTools/Source/Python/GenFds/Section.py
> index fc25447..942dd5c 100644
> --- a/BaseTools/Source/Python/GenFds/Section.py
> +++ b/BaseTools/Source/Python/GenFds/Section.py
> @@ -154,7 +154,7 @@ class Section (SectionClassObject):
> Tuple = os.walk(FfsInf.EfiOutputPath)
> for Dirpath, Dirnames, Filenames in Tuple:
> for F in Filenames:
> -if os.path.splitext(F)[1] in (Suffix):
> +if os.path.splitext(F)[1] == Suffix:
> FullName = os.path.join(Dirpath, F)
> if os.path.getmtime(FullName) > 
> os.path.getmtime(Makefile):
> FileList.append(FullName)
> -- 
> 2.8.0.windows.1
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] IntelFsp2WrapperPkg: Update gFspWrapperTokenSpaceGuid to gIntelFsp2WrapperTokenSpaceGuid.

2016-05-20 Thread Jiewen Yao
We updated gIntelFspPkgTokenSpaceGuid to gIntelFsp2PkgTokenSpaceGuid
in IntelFsp2Pkg, but we miss the update in IntelFsp2WrapperPkg.
This patch fixed the issue and made them consistent.

Cc: Giri P Mudusuru 
Cc: Satya P Yarlagadda 
Cc: Maurice Ma 
Cc: Ravi P Rangarajan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
 |  2 +-
 IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
 |  2 +-
 IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
 |  2 +-
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
 | 22 ++--
 IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFspWrapperApiLib.inf  
 |  4 ++--
 
IntelFsp2WrapperPkg/Library/PeiFspWrapperHobProcessLibSample/PeiFspWrapperHobProcessLibSample.inf
   |  4 ++--
 
IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecFspWrapperPlatformSecLibSample.inf
 | 14 ++---
 7 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf 
b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
index 9d50922..d8af0aa 100644
--- a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
+++ b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
@@ -60,7 +60,7 @@
   gFspHobGuid   ## CONSUMES ## HOB
 
 [Pcd]
-  gFspWrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
 
 [Depex]
   TRUE
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf 
b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index df735e1..0901a14 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -59,7 +59,7 @@
   IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
 
 [Pcd]
-  gFspWrapperTokenSpaceGuid.PcdFspmBaseAddress## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress  ## CONSUMES
 
 [Sources]
   FspmWrapperPeim.c
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf 
b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index 05914f3..261278f 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -66,7 +66,7 @@
   gEfiPeiMemoryDiscoveredPpiGuid## PRODUCES
 
 [Pcd]
-  gFspWrapperTokenSpaceGuid.PcdFspsBaseAddress## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
 
 [Guids]
   gFspHobGuid   ## CONSUMES ## HOB
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec 
b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
index b76e611..ac30e76 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -36,7 +36,7 @@
   #
   # GUID defined in package
   #
-  gFspWrapperTokenSpaceGuid = { 0xa34cf082, 0xf50, 0x4f0d,  { 
0x89, 0x8a, 0x3d, 0x39, 0x30, 0x2b, 0xc5, 0x1e } }
+  gIntelFsp2WrapperTokenSpaceGuid   = { 0xa34cf082, 0xf50, 0x4f0d,  { 
0x89, 0x8a, 0x3d, 0x39, 0x30, 0x2b, 0xc5, 0x1e } }
   gFspApiPerformanceGuid= { 0xc9122295, 0x56ed, 0x4d4e, { 
0x06, 0xa6, 0x50, 0x8d, 0x89, 0x4d, 0x3e, 0x40 } }
   gFspHobGuid   = { 0x6d86fb36, 0xba90, 0x472c, { 
0xb5, 0x83, 0x3f, 0xbe, 0xd3, 0xfb, 0x20, 0x9a } }
 
@@ -56,22 +56,22 @@
 

 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Provides the memory mapped base address of the BIOS CodeCache Flash 
Device.
-  
gFspWrapperTokenSpaceGuid.PcdFlashCodeCacheAddress|0xFFE0|UINT32|0x1001
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress|0xFFE0|UINT32|0x1001
   ## Provides the size of the BIOS Flash Device.
-  gFspWrapperTokenSpaceGuid.PcdFlashCodeCacheSize|0x0020|UINT32|0x1002
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize|0x0020|UINT32|0x1002
 
   ## Indicates the base address of the first Microcode Patch in the Microcode 
Region
-  gFspWrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x1005
-  
gFspWrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize|0x0|UINT64|0x1006
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x1005
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize|0x0|UINT64|0x1006
   ## Indicates the offset of the Cpu Microcode.
-  gFspWrapperTokenSpaceGuid.PcdFlashMicrocodeOffset|0x90|UINT32|0x1007
+ 

Re: [edk2] [PATCH] MdePkg: Follow PI1.4a to update the comments of EndOfDxe and SmmReadyToLock

2016-05-20 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Friday, May 20, 2016 2:42 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming
> 
> Subject: [PATCH] MdePkg: Follow PI1.4a to update the comments of
> EndOfDxe and SmmReadyToLock
> 
> Cc: Jiewen Yao 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Include/Protocol/DxeSmmReadyToLock.h | 26
> +++---
>  MdePkg/Include/Protocol/SmmEndOfDxe.h   | 14 +-
>  MdePkg/Include/Protocol/SmmReadyToLock.h| 21
> +
>  3 files changed, 41 insertions(+), 20 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> b/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> index 27045578aa44..a52a8a2c34c6 100644
> --- a/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> +++ b/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> @@ -1,11 +1,23 @@
>  /** @file
> -  DXE SMM Ready To Lock protocol as defined in the PI 1.2 specification.
> -
> -  This UEFI protocol indicates that SMM is about to be locked.
> -  This protocol is a mandatory protocol published by a DXE driver prior to
> invoking the
> -  EFI_SMM_ACCESS2_PROTOCOL.Lock() function to lock SMM.
> -
> -  Copyright (c) 2009, Intel Corporation. All rights reserved.
> +  DXE SMM Ready To Lock protocol introduced in the PI 1.2 specification.
> +
> +  According to PI 1.4a specification, this UEFI protocol indicates that
> +  resources and services that should not be used by the third party code
> +  are about to be locked.
> +  This protocol is a mandatory protocol published by PI platform code.
> +  This protocol in tandem with the End of DXE Event facilitates transition
> +  of the platform from the environment where all of the components are
> +  under the authority of the platform manufacturer to the environment
> where
> +  third party extensible modules such as UEFI drivers and UEFI applications
> +  are executed. The protocol is published immediately after signaling of
> the
> +  End of DXE Event. PI modules that need to lock or protect their resources
> +  in anticipation of the invocation of 3rd party extensible modules should
> +  register for notification on installation of this protocol and effect the
> +  appropriate protections in their notification handlers. For example, PI
> +  platform code may choose to use notification handler to lock SMM by
> invoking
> +  EFI_SMM_ACCESS2_PROTOCOL.Lock() function.
> +
> +  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the
> BSD License
>which accompanies this distribution.  The full text of the license may be
> found at
> diff --git a/MdePkg/Include/Protocol/SmmEndOfDxe.h
> b/MdePkg/Include/Protocol/SmmEndOfDxe.h
> index 4637ca1df372..e92ce4afbf51 100644
> --- a/MdePkg/Include/Protocol/SmmEndOfDxe.h
> +++ b/MdePkg/Include/Protocol/SmmEndOfDxe.h
> @@ -1,11 +1,15 @@
>  /** @file
> -  SMM End Of Dxe protocol as defined in the PI 1.2.1 specification.
> +  SMM End Of Dxe protocol introduced in the PI 1.2.1 specification.
> 
> -  This protocol is a mandatory protocol published by the PI platform code
> prior to invoking any
> -  3rd party content, including options ROM's and UEFI executables that are
> not from the platform manufacturer.
> -  There is an associated event GUID that is signaled for the DXE drivers
> called EFI_END_OF_DXE_EVENT_GUID.
> +  According to PI 1.4a specification, this protocol indicates end of the
> +  execution phase when all of the components are under the authority of
> +  the platform manufacturer.
> +  This protocol is a mandatory protocol published by SMM Foundation
> code.
> +  This protocol is an SMM counterpart of the End of DXE Event.
> +  This protocol prorogates End of DXE notification into SMM environment.
> +  This protocol is installed prior to installation of the SMM Ready to Lock
> Protocol.
> 
> -  Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the
> BSD License
>which accompanies this distribution.  The full text of the license may be
> found at
> diff --git a/MdePkg/Include/Protocol/SmmReadyToLock.h
> b/MdePkg/Include/Protocol/SmmReadyToLock.h
> index 59c92327a941..edf171923a38 100644
> --- a/MdePkg/Include/Protocol/SmmReadyToLock.h
> +++ b/MdePkg/Include/Protocol/SmmReadyToLock.h
> @@ -1,12 +1,17 @@
>  /** @file
> -  SMM Ready To Lock protocol as defined in the PI 1.2 specification.
> -
> -  This SMM protocol indicates that SMM is about to be locked.
> -  This protocol is a mandatory protocol 

[edk2] [PATCH] IntelFsp2WrapperPkg: Update gFspWrapperTokenSpaceGuid to gIntelFsp2WrapperTokenSpaceGuid.

2016-05-20 Thread Jiewen Yao
We updated gIntelFspPkgTokenSpaceGuid to gIntelFsp2PkgTokenSpaceGuid
in IntelFsp2Pkg, but we miss the update in IntelFsp2WrapperPkg.
This patch fixed the issue and made them consistent.

Cc: Giri P Mudusuru 
Cc: Satya P Yarlagadda 
Cc: Maurice Ma 
Cc: Ravi P Rangarajan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
 |  2 +-
 IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
 |  2 +-
 IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
 |  2 +-
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
 | 22 ++--
 IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFspWrapperApiLib.inf  
 |  4 ++--
 
IntelFsp2WrapperPkg/Library/PeiFspWrapperHobProcessLibSample/PeiFspWrapperHobProcessLibSample.inf
   |  4 ++--
 
IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecFspWrapperPlatformSecLibSample.inf
 | 14 ++---
 7 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf 
b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
index 9d50922..d8af0aa 100644
--- a/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
+++ b/IntelFsp2WrapperPkg/FspWrapperNotifyDxe/FspWrapperNotifyDxe.inf
@@ -60,7 +60,7 @@
   gFspHobGuid   ## CONSUMES ## HOB
 
 [Pcd]
-  gFspWrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
 
 [Depex]
   TRUE
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf 
b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index df735e1..0901a14 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -59,7 +59,7 @@
   IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
 
 [Pcd]
-  gFspWrapperTokenSpaceGuid.PcdFspmBaseAddress## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress  ## CONSUMES
 
 [Sources]
   FspmWrapperPeim.c
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf 
b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index 05914f3..261278f 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -66,7 +66,7 @@
   gEfiPeiMemoryDiscoveredPpiGuid## PRODUCES
 
 [Pcd]
-  gFspWrapperTokenSpaceGuid.PcdFspsBaseAddress## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress  ## CONSUMES
 
 [Guids]
   gFspHobGuid   ## CONSUMES ## HOB
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec 
b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
index b76e611..ac30e76 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -36,7 +36,7 @@
   #
   # GUID defined in package
   #
-  gFspWrapperTokenSpaceGuid = { 0xa34cf082, 0xf50, 0x4f0d,  { 
0x89, 0x8a, 0x3d, 0x39, 0x30, 0x2b, 0xc5, 0x1e } }
+  gIntelFsp2WrapperTokenSpaceGuid   = { 0xa34cf082, 0xf50, 0x4f0d,  { 
0x89, 0x8a, 0x3d, 0x39, 0x30, 0x2b, 0xc5, 0x1e } }
   gFspApiPerformanceGuid= { 0xc9122295, 0x56ed, 0x4d4e, { 
0x06, 0xa6, 0x50, 0x8d, 0x89, 0x4d, 0x3e, 0x40 } }
   gFspHobGuid   = { 0x6d86fb36, 0xba90, 0x472c, { 
0xb5, 0x83, 0x3f, 0xbe, 0xd3, 0xfb, 0x20, 0x9a } }
 
@@ -56,22 +56,22 @@
 

 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Provides the memory mapped base address of the BIOS CodeCache Flash 
Device.
-  
gFspWrapperTokenSpaceGuid.PcdFlashCodeCacheAddress|0xFFE0|UINT32|0x1001
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress|0xFFE0|UINT32|0x1001
   ## Provides the size of the BIOS Flash Device.
-  gFspWrapperTokenSpaceGuid.PcdFlashCodeCacheSize|0x0020|UINT32|0x1002
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize|0x0020|UINT32|0x1002
 
   ## Indicates the base address of the first Microcode Patch in the Microcode 
Region
-  gFspWrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x1005
-  
gFspWrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize|0x0|UINT64|0x1006
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x1005
+  
gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize|0x0|UINT64|0x1006
   ## Indicates the offset of the Cpu Microcode.
-  gFspWrapperTokenSpaceGuid.PcdFlashMicrocodeOffset|0x90|UINT32|0x1007
+ 

Re: [edk2] [Patch] NetworkPkg: Need update Http token status while timeout happened

2016-05-20 Thread Gary Lin
On Fri, May 20, 2016 at 03:24:29PM +0800, Jiaxin Wu wrote:
> Http token status should be updated to EFI_TIMEOUT while timeout
> happened by any abruptly interrupted (e.g. network disconnection,
> cable plug/unplug...). Otherwise, HttpBootDxe driver will continue
> treat it as no error happened, and its ReceivedSize will be updated
> to ContentLength directly. Moreover, If download image type is RAM
> Disk, the corresponding info will be registered to system.
> 
Hi Jiaxin,

I just tested the patch. It works well for me.

Here is how the patch was tested.
I started OVMF to connect to the host with a tap interface with IP
address 192.168.111.1 and then started the dhcp and http server on the
host. A new HTTP boot entry was created from HII to download a 2.9 GB
iso file. When OVMF was downloading the iso, I changed the IP to
192.168.100.1 to make the timeout deliberately. After applying the
patch, OVMF went back to the BDS HII as expected. What's interesting is
that, before applying the patch, OVMF just loaded the unfinished iso,
created a RAMDISK, and loaded the bootloader. It's probably because that
HttpTcpReceiveBody always returned EFI_SUCCESS.

So, the patch does a good work :)
Just one nitpicking thing: the line below HttpCloseConnection introdued
trailing whitespaces.

Reviewed-by: Gary Lin  and Tested-by: Gary Lin 

Cheers,

Gary Lin

> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Gary Lin 
> Cc: Samer El-Haj-Mahmoud 
> Cc: Hegde Nagaraj P 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c  |  3 +++
>  NetworkPkg/HttpDxe/HttpProto.c | 16 +++-
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index f4ae28a..05a96e9 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1259,10 +1259,13 @@ Exit:
>  
>  Error2:
>NetMapInsertHead (>TxTokens, ValueInItem->HttpToken, 
> ValueInItem);
>  
>  Error:
> +
> +  HttpCloseConnection (HttpInstance);
> +  
>HttpTcpTokenCleanup (Wrap);
>
>if (HttpHeaders != NULL) {
>  FreePool (HttpHeaders);
>}
> diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
> index afa7fe4..c3608c0 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.c
> +++ b/NetworkPkg/HttpDxe/HttpProto.c
> @@ -1,9 +1,9 @@
>  /** @file
>Miscellaneous routines for HttpDxe driver.
>  
> -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD 
> License
>  which accompanies this distribution.  The full text of the license may be 
> found at
>  http://opensource.org/licenses/bsd-license.php
> @@ -1605,10 +1605,11 @@ HttpTcpReceiveHeader (
>while (!HttpInstance->IsRxDone && ((Timeout == NULL) || EFI_ERROR 
> (gBS->CheckEvent (Timeout {
>  Tcp4->Poll (Tcp4);
>}
>  
>if (!HttpInstance->IsRxDone) {
> +Tcp4->Cancel (Tcp4, >CompletionToken);
>  gBS->CloseEvent (Rx4Token->CompletionToken.Event);
>  Rx4Token->CompletionToken.Status = EFI_TIMEOUT;
>}
>
>Status = Rx4Token->CompletionToken.Status;
> @@ -1671,10 +1672,11 @@ HttpTcpReceiveHeader (
>while (!HttpInstance->IsRxDone && ((Timeout == NULL) || EFI_ERROR 
> (gBS->CheckEvent (Timeout {
>  Tcp6->Poll (Tcp6);
>}
>  
>if (!HttpInstance->IsRxDone) {
> +Tcp6->Cancel (Tcp6, >CompletionToken);
>  gBS->CloseEvent (Rx6Token->CompletionToken.Event);
>  Rx6Token->CompletionToken.Status = EFI_TIMEOUT;
>}
>
>Status = Rx6Token->CompletionToken.Status;
> @@ -1745,11 +1747,12 @@ HttpTcpReceiveBody (
>HTTP_PROTOCOL *HttpInstance;
>EFI_TCP6_PROTOCOL *Tcp6;
>EFI_TCP6_IO_TOKEN *Rx6Token;
>EFI_TCP4_PROTOCOL *Tcp4;
>EFI_TCP4_IO_TOKEN *Rx4Token;
> -  
> +
> +  Status = EFI_SUCCESS;
>HttpInstance   = Wrap->HttpInstance;
>Tcp4 = HttpInstance->Tcp4;
>Tcp6 = HttpInstance->Tcp6;
>Rx4Token   = NULL;
>Rx6Token   = NULL;
> @@ -1776,14 +1779,15 @@ HttpTcpReceiveBody (
>  while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR 
> (gBS->CheckEvent (Timeout {
>Tcp6->Poll (Tcp6);
>  }
>  
>  if (!Wrap->TcpWrap.IsRxDone) {
> +  Tcp6->Cancel (Tcp6, >CompletionToken);
>gBS->CloseEvent (Rx6Token->CompletionToken.Event);
> +  Rx6Token->CompletionToken.Event = NULL;
>Rx6Token->CompletionToken.Status = EFI_TIMEOUT;
>Wrap->HttpToken->Status = 

Re: [edk2] [RFC] Structured PCD Proposal

2016-05-20 Thread Laszlo Ersek
On 05/19/16 19:39, Kinney, Michael D wrote:
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Laszlo Ersek
>> Sent: Thursday, May 19, 2016 2:26 AM
>> To: Kinney, Michael D 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] [RFC] Structured PCD Proposal
>>
>> On 05/19/16 01:28, Kinney, Michael D wrote:
>>
>> [snip]
>>
>>> ## C Structure Syntax Proposal
>>>   * Use C typedef syntax to describe complex C structures
>>>   * Use a C language parser with support for struct/union/array/bit 
>>> fields/pack
>>>   * Recommend use of libclang C language parser into Abstract Syntax Tree 
>>> (AST)
>>> - Included with LLVM release
>>> - http://llvm.org/releases/download.html
>>> - http://clang.llvm.org/doxygen/group__CINDEX.html
>>>   * Recommend use of Python bindings for CLANG for EDK II Python based 
>>> BaseTools
>>> - pip install clang
>>
>> What versions are necessary? On RHEL-7, the following versions are
>> available (from EPEL only; RHEL does not provide clang):
>>
>> clang-devel: 3.4.2-8.el7
>> python-pip: 7.1.0-1.el7
> 
> I have not tried the prototype on RHEL-7 yet.  For Fedora 23, we have used:
>  
>   $ sudo dnf install clang


Up to and including Fedora 23, clang had been built from the source
package called "llvm", so Koji and pkgdb must be searched for "llvm",
when someone is looking for Fedora 23 packages on the web (i.e., not
with "dnf" or "yum"). In Fedora 24, "clang" got its own SRPM:

http://thread.gmane.org/gmane.linux.redhat.fedora.devel/216194
https://bugzilla.redhat.com/show_bug.cgi?id=1300945

So, looking for "llvm" packages, it seems to me that the last Fedora
releases that shipped 3.4-based clang binaries were Fedora 21, and
Fedora 22:

http://koji.fedoraproject.org/koji/buildinfo?buildID=552862
http://koji.fedoraproject.org/koji/buildinfo?buildID=552863

(However, Fedora 22 later moved to the 3.5 series.)

Why this matters: on RHEL-7 I'll only be able to use 3.4.2-based clang
(from EPEL-7). I think testing the BaseTools feature in a fully updated
Fedora 21 installation (3.4-based clang) would allow you to extrapolate
the results to EPEL-7:

Fedora 23: 3.7.0-4   works
Fedora 21: 3.4-15?
EPEL-7:3.4.2-8   ?

Could you please test such a configuration?

Alternatively, if you have a public branch, and testing instructions,
I'd be happy to check them on my RHEL-7 system, using clang 3.4.2-8
(from EPEL-7).

>   $ git clone -b release_37 https://github.com/trolldbois/python-clang
>   $ cd python-clang
>   $ sudo python setup.py install

Unfortunately, this looks very bad. I shouldn't have to install, as
root, a Python package from a random github repository, in order to
build edk2 platforms.

At this point, quite a few downstreams (e.g., Linux distributions) build
OVMF packages. If the Structured PCD Feature is put to use in such
generic edk2 modules that OVMF pulls in (which I think will likely
happen), then all these downstreams are going to require the python
bindings for clang as native packages.

... Actually, the README file at
 says:

OBSOLETE. LLVM-CLANG NOW PUBLISHES PYTHON PACKAGE. JUST ADD THE
OFFICIAL llvm-3.7 repo in your apt.

If the above statement -- about llvm-3.7 including the python bindings
for clang -- is correct, then Fedora 23 and later might be all set,
because their llvm and clang stuff does not predate 3.7. (See Fedora 23
above, for example: 3.7.0-4).

However, this dependency leaves a number of Linux distributions in the
cold; distros that are still supported
- by their vendors,
- and by edk2 in general (for example through the now-antique GCC44
  toolchain).

I think if clang and its python bindings become an unconditional
dependency for building edk2, then:

(1) Some Linux distributions that can currently build edk2 platforms
(through the older GCC toolchains) will lose that capability, because
they may not have (fresh enough) clang. This could be okay, but then we
should declare it as a fact (= removal of compatibility), and remove old
GCC toolchains as well.

(2) The python bindings for clang, as an official part of LLVM, look
like a bleeding edge feature, in edk2 terms (according to Wikipedia,
LLVM 3.7.0 was released on 1 September 2015). If the python bindings
become a hard requirement for building edk2, then I think BaseTools
should embed them (it's "just Python", I guess). Otherwise only bleeding
edge Linux distros will be able to build edk2.

(I removed the rest of the context, but I do thank you for your other
answers!)

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch] BaseTools: Fix GenFds issue to wrongly get file without postfix.

2016-05-20 Thread Liming Gao
GenFds GenSection will search the output file based on the file extension.
If the output file has no extension, it should be skip.

Cc: Andrew Fish 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 BaseTools/Source/Python/GenFds/Section.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/GenFds/Section.py 
b/BaseTools/Source/Python/GenFds/Section.py
index fc25447..942dd5c 100644
--- a/BaseTools/Source/Python/GenFds/Section.py
+++ b/BaseTools/Source/Python/GenFds/Section.py
@@ -154,7 +154,7 @@ class Section (SectionClassObject):
 Tuple = os.walk(FfsInf.EfiOutputPath)
 for Dirpath, Dirnames, Filenames in Tuple:
 for F in Filenames:
-if os.path.splitext(F)[1] in (Suffix):
+if os.path.splitext(F)[1] == Suffix:
 FullName = os.path.join(Dirpath, F)
 if os.path.getmtime(FullName) > 
os.path.getmtime(Makefile):
 FileList.append(FullName)
-- 
2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [BaseTools] RuleOverride = BINARY code location?

2016-05-20 Thread Gao, Liming
Andrew:
  Thanks for your point out. It is bug. Your fix is correct. I will send the 
patch for it. 

Thanks
Liming
> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Friday, May 20, 2016 1:25 PM
> To: Gao, Liming 
> Cc: edk2-devel 
> Subject: Re: [edk2] [BaseTools] RuleOverride = BINARY code location?
> 
> 
> > On May 19, 2016, at 8:18 PM, Gao, Liming  wrote:
> >
> > Thanks Andrew. I will continue to investigate it.
> >
> 
> >>> F = 'FileNoExtention'
> >>> os.path.splitext(F)[1]
> ''
> >>> '' in '.out'
> True
> 
> So I think the bug is the `in` it should be `==`
> >>> '' == '.out'
> False
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> >> -Original Message-
> >> From: af...@apple.com [mailto:af...@apple.com]
> >> Sent: Friday, May 20, 2016 10:01 AM
> >> To: Gao, Liming 
> >> Cc: edk2-devel 
> >> Subject: Re: [edk2] [BaseTools] RuleOverride = BINARY code location?
> >>
> >>
> >>> On May 18, 2016, at 5:44 PM, Andrew Fish  wrote:
> >>>
> 
>  On May 18, 2016, at 5:43 PM, Gao, Liming 
> wrote:
> 
>  Andrew:
>  |.bin will search the file with the postfix .bin in INF file directory 
>  and its
> >> output directory. So, there may be more than .bin files are found. Please
> see
> >> the code login in GetFileList() from
> >> C:\R9Tip\edk2\BaseTools\Source\Python\GenFds\Section.py.
> 
> >>>
> >>
> >> Thanks for the pointer.
> >>
> >> This is the code that adding the OS executables in GetFileList() that is
> adding
> >> the OS executables without the .bin extension.
> >>if os.path.exists(Makefile):
> >># Update to search files with suffix in all sub-dirs.
> >>Tuple = os.walk(FfsInf.EfiOutputPath)
> >>for Dirpath, Dirnames, Filenames in Tuple:
> >>for F in Filenames:
> >>if os.path.splitext(F)[1] in (Suffix):
> >>FullName = os.path.join(Dirpath, F)
> >>if os.path.getmtime(FullName) >
> os.path.getmtime(Makefile):
> >>FileList.append(FullName)
> >>
> >>
> >> I think the issue is:
> >>   if os.path.splitext(F)[1] in (Suffix):
> >>
> >> If the file does not have an extension it matches.
> >>
> >> Sorry might be wrong have to go out to a writers reading with my wife so
> >> have to stop looking right now.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v2] BaseTools: Add error condition for the path in PACKAGES_PATH env

2016-05-20 Thread Liming Gao
From: "Zhu, Yonghong" 

This patch adds two error conditions:
1) if one path in PACKAGES_PATH doesn't exist.
2) if the space exists in the PACKAGES_PATH.

In V2, highlight one path in PACKAGES_PATH env doesn't exist.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
Reviewed-by: Liming Gao 
Reviewed by: Andrew Fish 
---
 BaseTools/Source/Python/build/build.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index 07891da..4f859bf 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -110,6 +110,12 @@ def CheckEnvVariable():
 # set multiple workspace
 PackagesPath = os.getenv("PACKAGES_PATH")
 mws.setWs(WorkspaceDir, PackagesPath)
+if mws.PACKAGES_PATH:
+for Path in mws.PACKAGES_PATH:
+if not os.path.exists(Path):
+EdkLogger.error("build", FILE_NOT_FOUND, "One Path in 
PACKAGES_PATH doesn't exist", ExtraData="%s" % Path)
+elif ' ' in Path:
+EdkLogger.error("build", FORMAT_NOT_SUPPORTED, "No space is 
allowed in PACKAGES_PATH", ExtraData=Path)
 
 #
 # Check EFI_SOURCE (Edk build convention). EDK_SOURCE will always point to 
ECP
-- 
2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Driver dependency on boot

2016-05-20 Thread Gao, Liming
You can try creating two event notification functions. One is to notify 
gEfiCcidProtocolGuid to run the logic with device plug-in, another is notify 
ReadyToBoot event to run the logic without device plug-in.

Thanks
Liming
From: Iru Cai [mailto:mytbk920...@gmail.com]
Sent: Friday, May 20, 2016 3:37 PM
To: Gao, Liming 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] Driver dependency on boot

It's a good idea, but I still have some problems. My ccidboot driver has 
different behaviors depending on whether a CCID card is plugged in, so I think 
I also need a timer event to check if gEfiCcidProtocolGuid is installed to a 
handle in time.  However, I need the code executed before the boot manager 
starts. In my case in OVMF, if I set a time too long in the timer event, its 
notification function will run after the EFI shell starts, and this is not what 
I want.
So now I need this in my ccidboot: when a CCID card is plugged in, it can wait 
for gEfiCcidProtocolGuid installed to a handle, and if no card is plugged in 
before BDS phase, it'll run other code.
Thanks
Iru

On Thu, May 19, 2016 at 3:22 PM, Gao, Liming 
> wrote:
You can create ProtocolNotification function for gEfiCcidProtocolGuid in 
ccidboot driver. If so, you don't need add it into [Depex].
UefiLib EfiCreateProtocolNotifyEvent() API can help to create it.

Thanks
Liming
> -Original Message-
> From: edk2-devel 
> [mailto:edk2-devel-boun...@lists.01.org]
>  On Behalf Of Iru
> Cai
> Sent: Thursday, May 19, 2016 2:35 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Driver dependency on boot
>
> Hi,
>
> I'm having some dependency problem when developing on edk2. I've
> written a
> CCID device driver which provides a protocol EFI_CCID_PROTOCOL with GUID
> gEfiCcidProtocolGuid. And I have another application called ccidboot to be
> run on boot that uses gEfiCcidProtocolGuid, I set the module type as
> DXE_DRIVER.
>
> If I add gEfiCcidProtocolGuid to the [Depex] of ccidboot, it works good
> when I have my CCID card plugged, but it'll not run if the card is
> unplugged. If I don't add this dependency, the ccidboot module will run
> before the driver starts the card. I don't know what I should do to solve
> this problem.
>
> Thanks,
> Iru
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] HTTP Boot crashed when downloading remote image

2016-05-20 Thread Wu, Jiaxin
Gary,

Timeout case could happened by any abruptly interrupted (e.g. network 
disconnection, cable plug/unplug...).

Some exception behavior may happened while Timeout triggered. The rootcause as 
I descripted below:
If TCP CompletionToken is not signaled, it should not be closed directly by 
calling CloseEvent after timeout happened (Still in the TCP RcvTokenList). If 
not, any exception behavior may be triggered. We should cancel it by calling 
Tcp->Cancel(), then close it. but unfortunately, Tcp->Cancel() is unsupported 
currently.

However, we can flush the tokens in TCP token list(what we want to do) by 
closing the existing TCP connection(Actually we should!). It's the safe 
operation.

Please help to verify the corresponding patch in edk2 mailing list. Title as 
below:
[edk2] [Patch] NetworkPkg: Need update Http token status while timeout happened 

Thanks.
Jiaxin

> -Original Message-
> From: Gary Lin [mailto:g...@suse.com]
> Sent: Thursday, May 19, 2016 12:11 PM
> To: Wu, Jiaxin 
> Cc: El-Haj-Mahmoud, Samer ; Laszlo
> Ersek ; Hegde, Nagaraj P  p.he...@hpe.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] HTTP Boot crashed when downloading remote image
> 
> On Thu, May 19, 2016 at 02:11:34AM +, Wu, Jiaxin wrote:
> > Thanks Gary. We also found that if TCP CompletionToken is not signaled, it
> should not be closed directly by calling CloseEvent after timeout
> happened(Still in the TCP RcvTokenList). If not, any exception behavior may
> be triggered while more packets continue received but only timeout
> happened (do you root cause? I'm glad to know.). We should cancel it by
> calling Tcp4->Cancel(), then close it. but unfortunately, Tcp4->Cancel() is
> unsupported currently.
> >
> Hi Jiaxin,
> 
> The case I encountered is that HttpTcpReceiveNotifyDpc freed Wrap
> mistakenly and caused the instability of the system (just sent the patch).
> Actually the timeout shouldn't show in my case since I used a VM to connect
> to the host to download a small EFI file.
> 
> Anyway, the problem you mentioned looks valid to me and maybe I should
> try to make a timeout test.
> 
> Thanks,
> 
> Gary Lin
> 
> > Thanks.
> > Jiaxin
> >
> > > -Original Message-
> > > From: Gary Lin [mailto:g...@suse.com]
> > > Sent: Thursday, May 19, 2016 10:06 AM
> > > To: El-Haj-Mahmoud, Samer ; Laszlo
> > > Ersek 
> > > Cc: edk2-devel@lists.01.org; Wu, Jiaxin 
> > > Subject: Re: [edk2] HTTP Boot crashed when downloading remote image
> > >
> > > On Wed, May 18, 2016 at 01:54:24PM +, El-Haj-Mahmoud, Samer
> wrote:
> > > > Gary,
> > > >
> > > > The EDK2 list blocked the wireshark attachment. Can you put it on
> > > > a share
> > > and send a link please? We are trying to look at this internally as well.
> > > Hi Samer and Laszlo,
> > >
> > > I've found the root cause. I'll send a patch later and Cc you guys.
> > >
> > > Thanks,
> > >
> > > Gary Lin
> > >
> > > >
> > > > Thanks,
> > > > --Samer
> > > >
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > Behalf Of Gary Lin
> > > > Sent: Tuesday, May 17, 2016 11:29 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Jiaxin Wu 
> > > > Subject: Re: [edk2] HTTP Boot crashed when downloading remote
> > > > image
> > > >
> > > > On Thu, May 12, 2016 at 06:26:36PM +0800, Gary Lin wrote:
> > > > > Hi,
> > > > >
> > > > > I was testing HTTP Boot with the latest OMVF and found that it
> > > > > crashed when downloading the remote image from the http server.
> > > > > Here is my bisect result:
> > > > >
> > > > > commit b347a22aecbfac9aac47831fee9a30aa810d6d0b
> > > > > NetworkPkg: Avoid the indefinite wait case in HttpDxe
> > > > >
> > > > > Actually, it sometimes worked but was broken for the most of time.
> > > > > Reverting this patch makes HTTP Boot always work.
> > > > >
> > > > > It seems the IP4 driver tried to remove a mnp event, but the
> > > > > event list was empty so the assert was triggered.
> > > > >
> > > > Some findings:
> > > >
> > > > 1. OVMF could crash without b347a22ae in a slight different way.
> > > >Several "TcpInput: Discard a packet" showed without the assert.
> > > >
> > > > 2. Rx4Token->CompletionToken in HttpTcpReceiveBody() corrupted
> after
> > > >the timeout. I saved Rx4Token->CompletionToken.Event before the
> > > >loop of Tcp4->Poll() and set a assert like this:
> > > >
> > > >ASSERT(Rx4Token->CompletionToken.Event == event);
> > > >
> > > >right after "if (!Wrap->TcpWrap.IsRxDone)". The assert was raised
> > > >after the timeout.
> > > >
> > > > 3. I attached wireshark to the tap interface. The first few packets were
> > > >alright, and then the window size from ACK of OVMF decreased
> rapidly
> > > >and "TCP ZeroWindow" showed right before the crash. I added debug
> > > >message to 

Re: [edk2] Driver dependency on boot

2016-05-20 Thread Iru Cai
It's a good idea, but I still have some problems. My ccidboot driver has
different behaviors depending on whether a CCID card is plugged in, so I
think I also need a timer event to check if gEfiCcidProtocolGuid is
installed to a handle in time.  However, I need the code executed before
the boot manager starts. In my case in OVMF, if I set a time too long in
the timer event, its notification function will run after the EFI shell
starts, and this is not what I want.

So now I need this in my ccidboot: when a CCID card is plugged in, it can
wait for gEfiCcidProtocolGuid installed to a handle, and if no card is
plugged in before BDS phase, it'll run other code.

Thanks
Iru

On Thu, May 19, 2016 at 3:22 PM, Gao, Liming  wrote:

> You can create ProtocolNotification function for gEfiCcidProtocolGuid in
> ccidboot driver. If so, you don't need add it into [Depex].
> UefiLib EfiCreateProtocolNotifyEvent() API can help to create it.
>
> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Iru
> > Cai
> > Sent: Thursday, May 19, 2016 2:35 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] Driver dependency on boot
> >
> > Hi,
> >
> > I'm having some dependency problem when developing on edk2. I've
> > written a
> > CCID device driver which provides a protocol EFI_CCID_PROTOCOL with GUID
> > gEfiCcidProtocolGuid. And I have another application called ccidboot to
> be
> > run on boot that uses gEfiCcidProtocolGuid, I set the module type as
> > DXE_DRIVER.
> >
> > If I add gEfiCcidProtocolGuid to the [Depex] of ccidboot, it works good
> > when I have my CCID card plugged, but it'll not run if the card is
> > unplugged. If I don't add this dependency, the ccidboot module will run
> > before the driver starts the card. I don't know what I should do to solve
> > this problem.
> >
> > Thanks,
> > Iru
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdePkg: Clarification to the return status of EFI_PEIM_NOTIFY_ENTRY_POINT

2016-05-20 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, May 18, 2016 5:08 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [PATCH] MdePkg: Clarification to the return status of
> EFI_PEIM_NOTIFY_ENTRY_POINT
> 
> In Previous PI spec (< PI1.4a) Volume 1, Section 7.4.1, the callback
> EFI_PEIM_NOTIFY_ENTRY_POINT is defined. A description for the arguments
> are provided but not for the EFI_STATUS return value.
> 
> PI1.4a updated EFI_PEIM_NOTIFY_ENTRY_POINT definition to include a new
> paragraph with this sentence after the arguments:
> 
> "The status code returned from this function is ignored"
> 
> This patch is to follow PI1.4a spec to update the code.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Include/Pi/PiPeiCis.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
> index f57f3331b14a..7cd525512f3d 100644
> --- a/MdePkg/Include/Pi/PiPeiCis.h
> +++ b/MdePkg/Include/Pi/PiPeiCis.h
> @@ -1,7 +1,7 @@
>  /** @file
>PI PEI master include file. This file should match the PI spec.
> 
> -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -12,7 +12,7 @@ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE
> ON AN "AS IS" BASIS,
>  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
>@par Revision Reference:
> -  PI Version 1.4.
> +  PI Version 1.4a.
> 
>  **/
> 
> @@ -73,6 +73,7 @@ EFI_STATUS
>@param  Ppi  Address of the PPI that was installed.
> 
>@return Status of the notification.
> +  The status code returned from this function is ignored.
>  **/
>  typedef
>  EFI_STATUS
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 0/2] Follow PI1.4a to fix artificial limitation of PCD SkuId range

2016-05-20 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Friday, May 20, 2016 2:49 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] Follow PI1.4a to fix artificial limitation of PCD
> SkuId range
> 
> There is absolutely no reason to artificially limit the SKU range to 1-255.
> PI1.4a spec fixed the artificial limitation.
> 
> The patches are to follow PI1.4a spec to remove the sentence
> "The valid SkuId range is 1 to 255." from SetSku function comments,
> PCD_MAX_SKU_ID definition, the check to PCD_MAX_SKU_ID and the
> comments
> describes the limitation.
> 
> Star Zeng (2):
>   MdeModulePkg PCD: Follow PI1.4a to fix artificial limitation of SkuId
> range
>   MdePkg: Follow PI1.4a to fix artificial limitation of SkuId range
> 
>  MdeModulePkg/Universal/PCD/Dxe/Pcd.c   |  6 +++---
>  MdeModulePkg/Universal/PCD/Pei/Pcd.c   |  4 ++--
>  MdePkg/Include/Library/PcdLib.h|  7 +--
>  MdePkg/Library/BasePcdLibNull/PcdLib.c |  6 +-
>  MdePkg/Library/DxePcdLib/DxePcdLib.c   | 14 ++
>  MdePkg/Library/PeiPcdLib/PeiPcdLib.c   | 15 ++-
>  6 files changed, 11 insertions(+), 41 deletions(-)
> 
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch] NetworkPkg: Need update Http token status while timeout happened

2016-05-20 Thread Jiaxin Wu
Http token status should be updated to EFI_TIMEOUT while timeout
happened by any abruptly interrupted (e.g. network disconnection,
cable plug/unplug...). Otherwise, HttpBootDxe driver will continue
treat it as no error happened, and its ReceivedSize will be updated
to ContentLength directly. Moreover, If download image type is RAM
Disk, the corresponding info will be registered to system.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Gary Lin 
Cc: Samer El-Haj-Mahmoud 
Cc: Hegde Nagaraj P 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpImpl.c  |  3 +++
 NetworkPkg/HttpDxe/HttpProto.c | 16 +++-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index f4ae28a..05a96e9 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1259,10 +1259,13 @@ Exit:
 
 Error2:
   NetMapInsertHead (>TxTokens, ValueInItem->HttpToken, 
ValueInItem);
 
 Error:
+
+  HttpCloseConnection (HttpInstance);
+  
   HttpTcpTokenCleanup (Wrap);
   
   if (HttpHeaders != NULL) {
 FreePool (HttpHeaders);
   }
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index afa7fe4..c3608c0 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1,9 +1,9 @@
 /** @file
   Miscellaneous routines for HttpDxe driver.
 
-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
 (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
@@ -1605,10 +1605,11 @@ HttpTcpReceiveHeader (
   while (!HttpInstance->IsRxDone && ((Timeout == NULL) || EFI_ERROR 
(gBS->CheckEvent (Timeout {
 Tcp4->Poll (Tcp4);
   }
 
   if (!HttpInstance->IsRxDone) {
+Tcp4->Cancel (Tcp4, >CompletionToken);
 gBS->CloseEvent (Rx4Token->CompletionToken.Event);
 Rx4Token->CompletionToken.Status = EFI_TIMEOUT;
   }
   
   Status = Rx4Token->CompletionToken.Status;
@@ -1671,10 +1672,11 @@ HttpTcpReceiveHeader (
   while (!HttpInstance->IsRxDone && ((Timeout == NULL) || EFI_ERROR 
(gBS->CheckEvent (Timeout {
 Tcp6->Poll (Tcp6);
   }
 
   if (!HttpInstance->IsRxDone) {
+Tcp6->Cancel (Tcp6, >CompletionToken);
 gBS->CloseEvent (Rx6Token->CompletionToken.Event);
 Rx6Token->CompletionToken.Status = EFI_TIMEOUT;
   }
   
   Status = Rx6Token->CompletionToken.Status;
@@ -1745,11 +1747,12 @@ HttpTcpReceiveBody (
   HTTP_PROTOCOL *HttpInstance;
   EFI_TCP6_PROTOCOL *Tcp6;
   EFI_TCP6_IO_TOKEN *Rx6Token;
   EFI_TCP4_PROTOCOL *Tcp4;
   EFI_TCP4_IO_TOKEN *Rx4Token;
-  
+
+  Status = EFI_SUCCESS;
   HttpInstance   = Wrap->HttpInstance;
   Tcp4 = HttpInstance->Tcp4;
   Tcp6 = HttpInstance->Tcp6;
   Rx4Token   = NULL;
   Rx6Token   = NULL;
@@ -1776,14 +1779,15 @@ HttpTcpReceiveBody (
 while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR 
(gBS->CheckEvent (Timeout {
   Tcp6->Poll (Tcp6);
 }
 
 if (!Wrap->TcpWrap.IsRxDone) {
+  Tcp6->Cancel (Tcp6, >CompletionToken);
   gBS->CloseEvent (Rx6Token->CompletionToken.Event);
+  Rx6Token->CompletionToken.Event = NULL;
   Rx6Token->CompletionToken.Status = EFI_TIMEOUT;
   Wrap->HttpToken->Status = Rx6Token->CompletionToken.Status;
-  gBS->SignalEvent (Wrap->HttpToken->Event);
 }
   } else {
 Rx4Token = >TcpWrap.Rx4Token;
 Rx4Token->Packet.RxData->DataLength = (UINT32) HttpMsg->BodyLength;
 Rx4Token->Packet.RxData->FragmentTable[0].FragmentLength = (UINT32) 
HttpMsg->BodyLength;
@@ -1799,19 +1803,21 @@ HttpTcpReceiveBody (
 while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR 
(gBS->CheckEvent (Timeout {
   Tcp4->Poll (Tcp4);
 }
 
 if (!Wrap->TcpWrap.IsRxDone) {
+  Tcp4->Cancel (Tcp4, >CompletionToken);
   gBS->CloseEvent (Rx4Token->CompletionToken.Event);
+  Rx4Token->CompletionToken.Event = NULL;
   Rx4Token->CompletionToken.Status = EFI_TIMEOUT;
   Wrap->HttpToken->Status = Rx4Token->CompletionToken.Status;
-  gBS->SignalEvent (Wrap->HttpToken->Event);
 }
   }
 
-  return EFI_SUCCESS;
+  Status = Wrap->HttpToken->Status;
 
+  return Status;
 }
 
 /**
   Clean up Tcp Tokens while the Tcp transmission error occurs.
 
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdePkg: Follow PI1.4a to update the comments of EndOfDxe and SmmReadyToLock

2016-05-20 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Zeng, Star
> Sent: Friday, May 20, 2016 2:42 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming
> 
> Subject: [PATCH] MdePkg: Follow PI1.4a to update the comments of
> EndOfDxe and SmmReadyToLock
> 
> Cc: Jiewen Yao 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Include/Protocol/DxeSmmReadyToLock.h | 26
> +++---
>  MdePkg/Include/Protocol/SmmEndOfDxe.h   | 14 +-
>  MdePkg/Include/Protocol/SmmReadyToLock.h| 21 +---
> -
>  3 files changed, 41 insertions(+), 20 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> b/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> index 27045578aa44..a52a8a2c34c6 100644
> --- a/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> +++ b/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
> @@ -1,11 +1,23 @@
>  /** @file
> -  DXE SMM Ready To Lock protocol as defined in the PI 1.2 specification.
> -
> -  This UEFI protocol indicates that SMM is about to be locked.
> -  This protocol is a mandatory protocol published by a DXE driver prior to
> invoking the
> -  EFI_SMM_ACCESS2_PROTOCOL.Lock() function to lock SMM.
> -
> -  Copyright (c) 2009, Intel Corporation. All rights reserved.
> +  DXE SMM Ready To Lock protocol introduced in the PI 1.2 specification.
> +
> +  According to PI 1.4a specification, this UEFI protocol indicates that
> +  resources and services that should not be used by the third party code
> +  are about to be locked.
> +  This protocol is a mandatory protocol published by PI platform code.
> +  This protocol in tandem with the End of DXE Event facilitates transition
> +  of the platform from the environment where all of the components are
> +  under the authority of the platform manufacturer to the environment
> where
> +  third party extensible modules such as UEFI drivers and UEFI applications
> +  are executed. The protocol is published immediately after signaling of the
> +  End of DXE Event. PI modules that need to lock or protect their resources
> +  in anticipation of the invocation of 3rd party extensible modules should
> +  register for notification on installation of this protocol and effect the
> +  appropriate protections in their notification handlers. For example, PI
> +  platform code may choose to use notification handler to lock SMM by
> invoking
> +  EFI_SMM_ACCESS2_PROTOCOL.Lock() function.
> +
> +  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> diff --git a/MdePkg/Include/Protocol/SmmEndOfDxe.h
> b/MdePkg/Include/Protocol/SmmEndOfDxe.h
> index 4637ca1df372..e92ce4afbf51 100644
> --- a/MdePkg/Include/Protocol/SmmEndOfDxe.h
> +++ b/MdePkg/Include/Protocol/SmmEndOfDxe.h
> @@ -1,11 +1,15 @@
>  /** @file
> -  SMM End Of Dxe protocol as defined in the PI 1.2.1 specification.
> +  SMM End Of Dxe protocol introduced in the PI 1.2.1 specification.
> 
> -  This protocol is a mandatory protocol published by the PI platform code
> prior to invoking any
> -  3rd party content, including options ROM's and UEFI executables that are
> not from the platform manufacturer.
> -  There is an associated event GUID that is signaled for the DXE drivers 
> called
> EFI_END_OF_DXE_EVENT_GUID.
> +  According to PI 1.4a specification, this protocol indicates end of the
> +  execution phase when all of the components are under the authority of
> +  the platform manufacturer.
> +  This protocol is a mandatory protocol published by SMM Foundation code.
> +  This protocol is an SMM counterpart of the End of DXE Event.
> +  This protocol prorogates End of DXE notification into SMM environment.
> +  This protocol is installed prior to installation of the SMM Ready to Lock
> Protocol.
> 
> -  Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> diff --git a/MdePkg/Include/Protocol/SmmReadyToLock.h
> b/MdePkg/Include/Protocol/SmmReadyToLock.h
> index 59c92327a941..edf171923a38 100644
> --- a/MdePkg/Include/Protocol/SmmReadyToLock.h
> +++ b/MdePkg/Include/Protocol/SmmReadyToLock.h
> @@ -1,12 +1,17 @@
>  /** @file
> -  SMM Ready To Lock protocol as defined in the PI 1.2 specification.
> -
> -  This SMM protocol indicates that SMM is about to be locked.
> -  This protocol is a 

Re: [edk2] [PATCH] MdePkg: Update EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE to 0x00080000

2016-05-20 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, May 18, 2016 2:02 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [PATCH] MdePkg: Update
> EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE to 0x0008
> 
> Previous PI spec (< PI1.4a) has EFI_RESOURCE_ATTRIBUTE_PERSISTENT and
> EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE with same value
> 0x0080.
> 
> To resolve the conflict, PI1.4a updated
> EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE to 0x0008, this
> patch
> is to follow PI1.4a spec to update the code.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Include/Pi/PiHob.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Include/Pi/PiHob.h b/MdePkg/Include/Pi/PiHob.h
> index 8581d7c74782..29467e79d59c 100644
> --- a/MdePkg/Include/Pi/PiHob.h
> +++ b/MdePkg/Include/Pi/PiHob.h
> @@ -1,7 +1,7 @@
>  /** @file
>HOB related definitions in PI.
> 
> -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -11,7 +11,7 @@ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE
> ON AN "AS IS" BASIS,
>  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
>@par Revision Reference:
> -  PI Version 1.4
> +  PI Version 1.4a
> 
>  **/
> 
> @@ -293,7 +293,7 @@ typedef UINT32 EFI_RESOURCE_ATTRIBUTE_TYPE;
>  #define EFI_RESOURCE_ATTRIBUTE_PERSISTABLE  0x0100
> 
>  #define EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTED  0x0004
> -#define EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE
> 0x0080
> +#define EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE
> 0x0008
> 
>  //
>  // Physical memory relative reliability attribute. This
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] UEFI HTTP Boot Device Path and DNS

2016-05-20 Thread Ye, Ting
Hi Michael,

Sorry I missed your previous email.
First,  I don't use OVMF platform, though I think the PXE and HTTP should be 
able to co-work together. It means that the PXE boot option and HTTP boot 
option are both available in the boot manager of BDS. I might not fully 
understand your usage, though.
Second, the UEFI spec defines EFI_IP4_CONFIG2_PROTOCOL for user to configure 
and retrieve DNS server addresses. You could use GetData with data type 
"Ip4Config2DataTypeDnsServer" to retrieve the info. 
Also, you can use EFI_DNS4/6_PROTOCOL to translate the host name to IP address 
directly. In the case you don't know the DNS server address, just configure the 
DnsServerList to NULL when configuring the DNS protocol instance; the protocol 
will try to retrieve the DNS server address from DHCP server.

Thanks,
Ye Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Michael 
Chang
Sent: Friday, May 20, 2016 12:57 PM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] UEFI HTTP Boot Device Path and DNS

Gently ping ..

Could anyone help to provide insights for this? In short the URI is absoultely 
legit to have host name in it but current device path couldn't provide any dns 
server to help in resolving it. How to deal with this problem ?

Thanks,
Michael

On Tue, Apr 19, 2016 at 06:18:35PM +0800, Michael Chang wrote:
> Background (And also Question):
> The PXE Base Code Protocol is no longer produced if selecting to boot 
> from UEFI HTTP device from boot menu. Does anyone here know why or is 
> it's just a problem of my environment (I'm using OVMF from openSUSE) ?
> 
> That made the work to support it in grub2 more complicated than it 
> should be, becuase it has to extract DHCP information from dhcp 
> packets cached in EFI_PXE_BASE_CODE_MODE structure to setup it's own 
> network interface. That's how it works before for TFTP, and at that 
> time booted TFTP device path seems to provide meaningless values.
> 
> Question:
> Now that we have to get it work by taking another approach to obtain 
> the DHCP information from the booted (HTTP) URI device path if the PXE 
> BC protocol cannot be located. But then we also bumped into another 
> issue that DNS infomation is missing from within the device path nodes 
> like
> (MAC(..)/IPv4(...)/URI(..)) so that server name in URI cannot work but 
> can only use IP address (v4 or v6). Is this a spec issue or intended 
> to leave behind DNS info ?
> 
> Could anyone please help to provide more insights ?
> 
> Thanks. :)
> Michael
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 0/2] Follow PI1.4a to fix artificial limitation of PCD SkuId range

2016-05-20 Thread Star Zeng
There is absolutely no reason to artificially limit the SKU range to 1-255.
PI1.4a spec fixed the artificial limitation.

The patches are to follow PI1.4a spec to remove the sentence
"The valid SkuId range is 1 to 255." from SetSku function comments,
PCD_MAX_SKU_ID definition, the check to PCD_MAX_SKU_ID and the comments
describes the limitation.

Star Zeng (2):
  MdeModulePkg PCD: Follow PI1.4a to fix artificial limitation of SkuId
range
  MdePkg: Follow PI1.4a to fix artificial limitation of SkuId range

 MdeModulePkg/Universal/PCD/Dxe/Pcd.c   |  6 +++---
 MdeModulePkg/Universal/PCD/Pei/Pcd.c   |  4 ++--
 MdePkg/Include/Library/PcdLib.h|  7 +--
 MdePkg/Library/BasePcdLibNull/PcdLib.c |  6 +-
 MdePkg/Library/DxePcdLib/DxePcdLib.c   | 14 ++
 MdePkg/Library/PeiPcdLib/PeiPcdLib.c   | 15 ++-
 6 files changed, 11 insertions(+), 41 deletions(-)

-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 2/2] MdePkg: Follow PI1.4a to fix artificial limitation of SkuId range

2016-05-20 Thread Star Zeng
There is absolutely no reason to artificially limit the SKU range to 1-255.
PI1.4a spec fixed the artificial limitation.

This patch is to follow PI1.4a spec to remove PCD_MAX_SKU_ID definition,
the check to PCD_MAX_SKU_ID and the comments describes the limitation.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdePkg/Include/Library/PcdLib.h|  7 +--
 MdePkg/Library/BasePcdLibNull/PcdLib.c |  6 +-
 MdePkg/Library/DxePcdLib/DxePcdLib.c   | 14 ++
 MdePkg/Library/PeiPcdLib/PeiPcdLib.c   | 15 ++-
 4 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/MdePkg/Include/Library/PcdLib.h b/MdePkg/Include/Library/PcdLib.h
index ad8a70082acb..4def32886294 100644
--- a/MdePkg/Include/Library/PcdLib.h
+++ b/MdePkg/Include/Library/PcdLib.h
@@ -14,7 +14,7 @@
   There are no restrictions on the use of FeaturePcd(), FixedPcdGetXX(),
   PatchPcdGetXX(), and PatchPcdSetXX().
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -28,8 +28,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef __PCD_LIB_H__
 #define __PCD_LIB_H__
 
-#define PCD_MAX_SKU_ID   0x100
-
 
 /**
   Retrieves a token number based on a token name.
@@ -1074,7 +1072,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
   This function provides a means by which SKU support can be established in 
the PCD infrastructure.
 
   Sets the current SKU in the PCD database to the value specified by SkuId.  
SkuId is returned.
-  If SkuId >= PCD_MAX_SKU_ID, then ASSERT(). 
 
   @param  SkuId   The SKU value that will be used when the PCD service 
retrieves and sets values
   associated with a PCD token.
@@ -2250,8 +2247,6 @@ LibPcdGetInfoEx (
 /**
   Retrieve the currently set SKU Id.
 
-  If the sku id got >= PCD_MAX_SKU_ID, then ASSERT().
-
   @return   The currently set SKU Id. If the platform has not set at a SKU Id, 
then the
 default SKU Id value of 0 is returned. If the platform has set a 
SKU Id, then the currently set SKU
 Id is returned.
diff --git a/MdePkg/Library/BasePcdLibNull/PcdLib.c 
b/MdePkg/Library/BasePcdLibNull/PcdLib.c
index 5d914ada561c..4fc3672b7a36 100644
--- a/MdePkg/Library/BasePcdLibNull/PcdLib.c
+++ b/MdePkg/Library/BasePcdLibNull/PcdLib.c
@@ -1,7 +1,7 @@
 /** @file
   A emptry template implementation of PCD Library.
 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -26,8 +26,6 @@
 
   @param[in]  SkuId The SKU value that will be used when the PCD service will 
retrieve and 
 set values associated with a PCD token.
-
-  If SkuId >= 0x100, then ASSERT().  
 
   @return Return the SKU ID that just be set.
 
@@ -1465,8 +1463,6 @@ LibPcdGetInfoEx (
 /**
   Retrieve the currently set SKU Id.
 
-  If the sku id got >= PCD_MAX_SKU_ID, then ASSERT().
-
   @return   The currently set SKU Id. If the platform has not set at a SKU Id, 
then the
 default SKU Id value of 0 is returned. If the platform has set a 
SKU Id, then the currently set SKU
 Id is returned.
diff --git a/MdePkg/Library/DxePcdLib/DxePcdLib.c 
b/MdePkg/Library/DxePcdLib/DxePcdLib.c
index 79f769557b80..371166818a34 100644
--- a/MdePkg/Library/DxePcdLib/DxePcdLib.c
+++ b/MdePkg/Library/DxePcdLib/DxePcdLib.c
@@ -1,7 +1,7 @@
 /** @file
 Implementation of PcdLib class library for DXE phase.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials  
 are licensed and made available under the terms and conditions of the BSD 
License 
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -125,7 +125,6 @@ GetPcdInfoProtocolPointer (
   This function provides a means by which SKU support can be established in 
the PCD infrastructure.
 
   Sets the current SKU in the PCD database to the value specified by SkuId.  
SkuId is returned.
-  If SkuId >= PCD_MAX_SKU_ID, then ASSERT(). 
 
   @param  SkuId   The SKU value that will be used when the PCD service 
retrieves and sets values
   associated with a PCD token.
@@ -139,8 +138,6 @@ 

[edk2] [PATCH 1/2] MdeModulePkg PCD: Follow PI1.4a to fix artificial limitation of SkuId range

2016-05-20 Thread Star Zeng
There is absolutely no reason to artificially limit the SKU range to 1-255.
PI1.4a spec fixed the artificial limitation.

This patch is to follow PI1.4a spec to remove the sentence
"The valid SkuId range is 1 to 255." from SetSku function comments.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 6 +++---
 MdeModulePkg/Universal/PCD/Pei/Pcd.c | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c 
b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
index 14d47849d85b..b9cf9e4e7646 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
@@ -1,9 +1,9 @@
 /** @file
   PCD DXE driver manage all PCD entry initialized in PEI phase and DXE phase, 
and
   produce the implementation of native PCD protocol and EFI_PCD_PROTOCOL 
defined in
-  PI 1.2 Vol3.
+  PI 1.4a Vol3.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -252,7 +252,7 @@ DxeGetPcdInfoGetSku (
   or multiple values, where each value is associated with a specific SKU Id. 
Items with multiple, 
   SKU-specific values are called SKU enabled. 
   
-  The SKU Id of zero is reserved as a default. The valid SkuId range is 1 to 
255.  
+  The SKU Id of zero is reserved as a default.
   For tokens that are not SKU enabled, the system ignores any set SKU Id and 
works with the 
   single value for that token. For SKU-enabled tokens, the system will use the 
SKU Id set by the 
   last call to SetSku(). If no SKU Id is set or the currently set SKU Id isn't 
valid for the specified token, 
diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.c 
b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
index 15f1924f898b..4d0caed5a94e 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
@@ -1,7 +1,7 @@
 /** @file 
   All Pcd Ppi services are implemented here.
   
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials  
 are licensed and made available under the terms and conditions of the BSD 
License 
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -239,7 +239,7 @@ PeiGetPcdInfoGetSku (
   or multiple values, where each value is associated with a specific SKU Id. 
Items with multiple, 
   SKU-specific values are called SKU enabled. 
   
-  The SKU Id of zero is reserved as a default. The valid SkuId range is 1 to 
255.  
+  The SKU Id of zero is reserved as a default.
   For tokens that are not SKU enabled, the system ignores any set SKU Id and 
works with the 
   single value for that token. For SKU-enabled tokens, the system will use the 
SKU Id set by the 
   last call to SetSku(). If no SKU Id is set or the currently set SKU Id isn't 
valid for the specified token, 
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] MdePkg: Follow PI1.4a to update the comments of EndOfDxe and SmmReadyToLock

2016-05-20 Thread Star Zeng
Cc: Jiewen Yao 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdePkg/Include/Protocol/DxeSmmReadyToLock.h | 26 +++---
 MdePkg/Include/Protocol/SmmEndOfDxe.h   | 14 +-
 MdePkg/Include/Protocol/SmmReadyToLock.h| 21 +
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/MdePkg/Include/Protocol/DxeSmmReadyToLock.h 
b/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
index 27045578aa44..a52a8a2c34c6 100644
--- a/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
+++ b/MdePkg/Include/Protocol/DxeSmmReadyToLock.h
@@ -1,11 +1,23 @@
 /** @file
-  DXE SMM Ready To Lock protocol as defined in the PI 1.2 specification.
-
-  This UEFI protocol indicates that SMM is about to be locked.
-  This protocol is a mandatory protocol published by a DXE driver prior to 
invoking the 
-  EFI_SMM_ACCESS2_PROTOCOL.Lock() function to lock SMM. 
-
-  Copyright (c) 2009, Intel Corporation. All rights reserved.
+  DXE SMM Ready To Lock protocol introduced in the PI 1.2 specification.
+
+  According to PI 1.4a specification, this UEFI protocol indicates that
+  resources and services that should not be used by the third party code
+  are about to be locked.
+  This protocol is a mandatory protocol published by PI platform code.
+  This protocol in tandem with the End of DXE Event facilitates transition
+  of the platform from the environment where all of the components are
+  under the authority of the platform manufacturer to the environment where
+  third party extensible modules such as UEFI drivers and UEFI applications
+  are executed. The protocol is published immediately after signaling of the
+  End of DXE Event. PI modules that need to lock or protect their resources
+  in anticipation of the invocation of 3rd party extensible modules should
+  register for notification on installation of this protocol and effect the
+  appropriate protections in their notification handlers. For example, PI
+  platform code may choose to use notification handler to lock SMM by invoking
+  EFI_SMM_ACCESS2_PROTOCOL.Lock() function.
+
+  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
diff --git a/MdePkg/Include/Protocol/SmmEndOfDxe.h 
b/MdePkg/Include/Protocol/SmmEndOfDxe.h
index 4637ca1df372..e92ce4afbf51 100644
--- a/MdePkg/Include/Protocol/SmmEndOfDxe.h
+++ b/MdePkg/Include/Protocol/SmmEndOfDxe.h
@@ -1,11 +1,15 @@
 /** @file
-  SMM End Of Dxe protocol as defined in the PI 1.2.1 specification.
+  SMM End Of Dxe protocol introduced in the PI 1.2.1 specification.
 
-  This protocol is a mandatory protocol published by the PI platform code 
prior to invoking any
-  3rd party content, including options ROM's and UEFI executables that are not 
from the platform manufacturer.
-  There is an associated event GUID that is signaled for the DXE drivers 
called EFI_END_OF_DXE_EVENT_GUID.
+  According to PI 1.4a specification, this protocol indicates end of the
+  execution phase when all of the components are under the authority of
+  the platform manufacturer.
+  This protocol is a mandatory protocol published by SMM Foundation code.
+  This protocol is an SMM counterpart of the End of DXE Event.
+  This protocol prorogates End of DXE notification into SMM environment.
+  This protocol is installed prior to installation of the SMM Ready to Lock 
Protocol.
 
-  Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
diff --git a/MdePkg/Include/Protocol/SmmReadyToLock.h 
b/MdePkg/Include/Protocol/SmmReadyToLock.h
index 59c92327a941..edf171923a38 100644
--- a/MdePkg/Include/Protocol/SmmReadyToLock.h
+++ b/MdePkg/Include/Protocol/SmmReadyToLock.h
@@ -1,12 +1,17 @@
 /** @file
-  SMM Ready To Lock protocol as defined in the PI 1.2 specification.
-
-  This SMM protocol indicates that SMM is about to be locked.
-  This protocol is a mandatory protocol published by the SMM Foundation code 
when the system is 
-  preparing to lock SMM. This protocol should be installed immediately after
-  EFI_END_OF_DXE_EVENT_GROUP_GUID with no intervening modules dispatched.
-
-  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+  SMM Ready To Lock protocol introduced in the PI 1.2 specification.
+
+  According to PI 1.4a specification, this SMM protocol indicates that
+  SMM resources and services that should not be used by the third party
+  code are 

Re: [edk2] [Patch] ShellPkg: Add argument to set block size for tftp command.

2016-05-20 Thread Ard Biesheuvel
On 20 May 2016 at 08:24, Fu, Siyuan  wrote:
> Hi, Ard
>
>
>
> Thanks for you reminder. I’m sorry that I don’t have the ARM build
> environment, will try to setup one later. For the current build failure, do
> you mean the explicit casting in attached patch could fix it?
>

Yes. I tried your patch and it fixes the build on ARM

Reviewed-by: Ard Biesheuvel 

Thanks,
Ard.


>
>
> Best Regards
>
> Siyuan
>
>
>
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Friday, May 20, 2016 2:05 PM
> To: Fu, Siyuan 
> Cc: edk2-devel@lists.01.org; Qiu, Shumin ; Carsey,
> Jaben 
> Subject: Re: [edk2] [Patch] ShellPkg: Add argument to set block size for
> tftp command.
>
>
>
> This patch has broken the build on ARM
>
> In function 'DownloadFile':
> :938:22:
> error: pointer targets in assignment differ in signedness
> [-Werror=pointer-sign]
>  ReqOpt.OptionStr = "blksize";
>   ^
> :939:5:
> error: pointer targets in passing argument 1 of 'AsciiSPrint' differ
> in signedness [-Werror=pointer-sign]
>  AsciiSPrint (OptBuf, sizeof (OptBuf), "%d", BlockSize);
>
> Please don't mix CHAR8, INT8 and UINT8: they are all distinct types,
> and require explicit casting when using one in place of the other.
>
> Thanks,
> Ard,
>
>
> On 6 May 2016 at 19:46, Carsey, Jaben  wrote:
>> Why write the function UintnToAscDec to convert UINTN to ascii string?
>> PrintLib can do that for you.
>>
>> Reviewed-by: Jaben Carsey 
>>
>>> -Original Message-
>>> From: Fu, Siyuan
>>> Sent: Thursday, May 05, 2016 7:33 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Carsey, Jaben ; Qiu, Shumin
>>> 
>>> Subject: [Patch] ShellPkg: Add argument to set block size for tftp
>>> command.
>>> Importance: High
>>>
>>> TFTP block size has a big impact on the transmit performance, this patch
>>> is to
>>> add new argument [-s ] for shell "tftp" command to configure
>>> the
>>> block size for file download.
>>>
>>> Cc: Jaben Carsey 
>>> Cc: Shumin Qiu 
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Fu Siyuan 
>>> ---
>>>  ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c| 81
>>> +-
>>>  .../UefiShellTftpCommandLib.uni|  6 +-
>>>  2 files changed, 83 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
>>> b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
>>> index 831b9c3..e72e6f6 100644
>>> --- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
>>> +++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
>>> @@ -2,7 +2,7 @@
>>>The implementation for the 'tftp' Shell command.
>>>
>>>Copyright (c) 2015, ARM Ltd. All rights reserved.
>>> -  Copyright (c) 2015, Intel Corporation. All rights reserved. 
>>> +  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>>> 
>>>(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>>>
>>>This program and the accompanying materials
>>> @@ -163,6 +163,7 @@ GetFileSize (
>>>@param[in]   FilePath   Path of the file, Unicode encoded
>>>@param[in]   AsciiFilePath  Path of the file, ASCII encoded
>>>@param[in]   FileSize   Size of the file in number of bytes
>>> +  @param[in]   BlockSize  Value of the TFTP blksize option
>>>@param[out]  Data   Address where to store the address of the
>>> buffer
>>>where the data of the file were downloaded
>>> in
>>>case of success.
>>> @@ -180,6 +181,7 @@ DownloadFile (
>>>IN   CONST CHAR16 *FilePath,
>>>IN   CONST CHAR8  *AsciiFilePath,
>>>IN   UINTNFileSize,
>>> +  IN   UINT16   BlockSize,
>>>OUT  VOID **Data
>>>);
>>>
>>> @@ -223,9 +225,15 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>>>{L"-r", TypeValue},
>>>{L"-c", TypeValue},
>>>{L"-t", TypeValue},
>>> +  {L"-s", TypeValue},
>>>{NULL , TypeMax}
>>>};
>>>
>>> +///
>>> +/// The default block size (512) of tftp is defined in the RFC1350.
>>> +///
>>> +#define MTFTP_DEFAULT_BLKSIZE  512
>>> +
>>>  /**
>>>Function for 'tftp' command.
>>>
>>> @@ -271,6 +279,7 @@ ShellCommandRunTftp (
>>>UINTN   FileSize;
>>>VOID*Data;
>>>SHELL_FILE_HANDLE   FileHandle;
>>> +  UINT16  BlockSize;
>>>
>>>ShellStatus = SHELL_INVALID_PARAMETER;
>>>ProblemParam= NULL;