Re: [edk2] [PATCH 0/6] implement standalone MM versions of the variable runtime drivers

[Zeng, Star]

We'd better have a bugzilla to track this change.
And since it will require platform change in platform dsc to add the new 
library mapping, we need add notes in 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Notes.



Thanks,
Star

On 2019/1/4 2:28, Ard Biesheuvel wrote:

This series proposed an alternative approach to the series sent out by
Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the
special PCD, as well as some other if() conditionals.

The primary difference is that this series defines and implements
MmServicesTableLib in such a way that the traditional SMM drivers
can use it as well. This is appropriate, considering that the PI
spec has rebranded traditional SMM as one implementation of the generic
MM framework.

Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib
library class, but for all SMM flavours, not only for standalone MM.

Patch #2 implements MmServicesTableLib for traditional SMM implementations.

Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM
driver that invoke boot services are separated from the core SMM pieces.

Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment.

Patches #5 and #6 do the same, respectively, for the variable runtime driver.

This approach minimizes the delta, and thus the maintenance burden, between
the traditional SMM and standalone MM drivers, while not resorting to runtime
checks or other conditionals in the code to implement logic that should be
decided at build time.

Note that this series only covers part of the work contributed by Jagadeesh.
This series focuses on the MdePkg and MdeModulePkg changes that affect shared
code.

Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Jagadeesh Ujja 
Cc: Achin Gupta 
Cc: Thomas Panakamattam Abraham 
Cc: Sami Mujawar 

Ard Biesheuvel (5):
   MdePkg: implement MmServicesTableLib based on traditional SMM
   MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses
   MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version
   MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses
   MdeModulePkg/VariableRuntimeDxe: implement standalone MM version

Jagadeesh Ujja (1):
   MdePkg/Include: add MmServicesTableLib header file

  MdeModulePkg/MdeModulePkg.dsc |   1 +
  .../FaultTolerantWrite.h  |  22 ++-
  .../FaultTolerantWriteDxe.c   |  31 
  .../FaultTolerantWriteSmm.c   |  54 +++
  .../FaultTolerantWriteSmm.inf |   5 +-
  .../FaultTolerantWriteSmmCommon.h |  31 
  .../FaultTolerantWriteSmmDxe.c|   1 +
  .../FaultTolerantWriteStandaloneMm.c  |  70 +
  .../FaultTolerantWriteStandaloneMm.inf|  90 
  .../FaultTolerantWriteTraditionalMm.c |  94 
  .../UpdateWorkingBlock.c  |  10 +-
  .../Variable/RuntimeDxe/TcgMorLockSmm.c   |  18 +--
  .../Universal/Variable/RuntimeDxe/Variable.h  |  50 +++
  .../Variable/RuntimeDxe/VariableSmm.c |  59 +++-
  .../Variable/RuntimeDxe/VariableSmm.inf   |   5 +-
  .../RuntimeDxe/VariableStandaloneMm.c |  69 +
  .../RuntimeDxe/VariableStandaloneMm.inf   | 135 ++
  .../RuntimeDxe/VariableTraditionalMm.c| 114 +++
  MdePkg/Include/Library/MmServicesTableLib.h   |  25 
  .../MmServicesTableLib/MmServicesTableLib.c   |  63 
  .../MmServicesTableLib/MmServicesTableLib.inf |  45 ++
  .../MmServicesTableLib/MmServicesTableLib.uni |  22 +++
  MdePkg/MdePkg.dec |   4 +
  MdePkg/MdePkg.dsc |   1 +
  24 files changed, 916 insertions(+), 103 deletions(-)
  create mode 100644 
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
  create mode 100644 
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
  create mode 100644 
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c
  create mode 100644 
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c
  create mode 100644 
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
  create mode 100644 
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c
  create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h
  create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c
  create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
  create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni



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

Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

[Zeng, Star]

On 2019/1/10 15:33, Ard Biesheuvel wrote:

On Thu, 10 Jan 2019 at 08:30, Zeng, Star  wrote:


Hi Ard,

Another minor feedback.

On 2019/1/10 14:47, Zeng, Star wrote:

Hi Ard,

Some minor feedback added inline.

On 2019/1/4 2:28, Ard Biesheuvel wrote:

Implement a new version of the fault tolerant write driver that can
be used in the context of a standalone MM implementation.

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

MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
| 70 +++

MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
| 90 
   2 files changed, 160 insertions(+)


Please add it into MdeModulePkg.dsc for package build verification.



Hello Star,

Thanks for all the feedback. I will respond in more detail later.

However, to the point raised here: it is not possible to add these
drivers to MdeModulePkg.dsc unless we add a dummy implementation of
StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
do that?


Oh, good information.
To have full code building coverage for the package, personally I think 
we can move StandaloneMmDriverEntryPoint library class and instance into 
MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should 
be generic enough.


I do not want to block this patch set because of this. So let's discuss 
this in parallel as separated topic.


Mike, Liming, Laszlo, Jian and Hao,\
What's your opinion?


Thanks,
Star






diff --git
a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

new file mode 100644
index ..b6fbf6c64f8a
--- /dev/null
+++
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

@@ -0,0 +1,70 @@
+/** @file
+
+  Parts of the SMM/MM implementation that are specific to standalone MM
+
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018, Linaro, Ltd. 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
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
IMPLIED.
+
+**/
+
+#include 
+#include 
+#include "FaultTolerantWrite.h"
+#include "FaultTolerantWriteSmmCommon.h"
+
+BOOLEAN
+FtwSmmIsBufferOutsideSmmValid (
+  IN EFI_PHYSICAL_ADDRESS  Buffer,
+  IN UINT64Length
+  )
+{
+  return TRUE;
+}


Please add function comment header for it, otherwise some coding style
tool may report error.


+
+/**
+  Internal implementation of CRC32. Depending on the execution context
+  (standalone SMM or DXE vs standalone MM), this function is implemented
+  via a call to the CalculateCrc32 () boot service, or via a library
+  call.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param[in]  Buffer   A pointer to the buffer on which the
32-bit CRC is to be computed.
+  @param[in]  Length   The number of bytes in the buffer Data.
+
+  @retval Crc32The 32-bit CRC was computed for the data
buffer.
+
+**/
+UINT32
+FtwCalculateCrc32 (
+  IN  VOID *Buffer,
+  IN  UINTNLength
+  )
+{
+  return CalculateCrc32 (Buffer, Length);
+}


Please add function comment header for it, otherwise some coding style
tool may report error.


+
+VOID
+FtwNotifySmmReady (
+  VOID
+  )
+{
+}


Please add function comment header for it, otherwise some coding style
tool may report error.

Thanks,
Star


+
+EFI_STATUS
+EFIAPI
+StandaloneMmFaultTolerantWriteInitialize (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
+  )
+{
+  return MmFaultTolerantWriteInitialize ();
+}
diff --git
a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

new file mode 100644
index ..99bd62ad5ceb
--- /dev/null
+++
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

@@ -0,0 +1,90 @@
+ ## @file
+#   Fault Tolerant Write Smm Driver.
+#
+#   This driver installs SMM Fault Tolerant Write (FTW) protocol,
which provides fault
+#   tolerant write capability in SMM environment for block devices.
Its implementation
+#   depends on the full functionality SMM FVB protocol that support
read, write/erase
+#   flash access.
+#
+# Copyright (c) 2010 - 2018, 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 

Re: [edk2] Conditional Compilation support in INF file

[Laszlo Ersek]

On 01/10/19 07:03, karunakarpoosapa...@dell.com wrote:
> Hi All,
> 
> I agree with providing the support like "FixedAtBuild PCD in INF". And we 
> need to modify or provide support in BaseTools to support this feature.
> 
> There are more use cases or flexibility to developer if we support 
> Conditional compilation support in INF.
> As we're providing support in BaseTools for FixedAtBuild PCD support in INF, 
> Is there any challenges or drawbacks in  providing conditional compilation 
> support in INF?  

This is not for me to say authoritatively, but I'm unaware of any
specific use case that cannot be solved without this feature addition,
and any further complexity to BaseTools should be strongly justified.
"More convenient" is too vague for me, and the BaseTools code is already
hard to read and debug.

That's just my opinion, again.

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

Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

[Laszlo Ersek]

On 01/10/19 08:59, Zeng, Star wrote:
> On 2019/1/10 15:33, Ard Biesheuvel wrote:
>> On Thu, 10 Jan 2019 at 08:30, Zeng, Star  wrote:
>>>
>>> Hi Ard,
>>>
>>> Another minor feedback.
>>>
>>> On 2019/1/10 14:47, Zeng, Star wrote:
 Hi Ard,

 Some minor feedback added inline.

 On 2019/1/4 2:28, Ard Biesheuvel wrote:
> Implement a new version of the fault tolerant write driver that can
> be used in the context of a standalone MM implementation.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>
> | 70 +++
>
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>
> | 90 
>    2 files changed, 160 insertions(+)
>>>
>>> Please add it into MdeModulePkg.dsc for package build verification.
>>>
>>
>> Hello Star,
>>
>> Thanks for all the feedback. I will respond in more detail later.
>>
>> However, to the point raised here: it is not possible to add these
>> drivers to MdeModulePkg.dsc unless we add a dummy implementation of
>> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
>> do that?
> 
> Oh, good information.
> To have full code building coverage for the package, personally I think
> we can move StandaloneMmDriverEntryPoint library class and instance into
> MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should
> be generic enough.
> 
> I do not want to block this patch set because of this. So let's discuss
> this in parallel as separated topic.
> 
> Mike, Liming, Laszlo, Jian and Hao,\
> What's your opinion?

It should be possible to build all library instances in a central
Package (well, all Packages really), using the Package's DSC file. To my
understanding, libraries built like this are not expected to be used in
actual (shipped) drivers / applications, nor is their indiscriminate
distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull
library instance in binary form seems quite useless.

With that in mind, I think a Null instance for the entry point in
question makes sense, under MdeModulePkg.

Thanks
Laszlo


> 
> 
> Thanks,
> Star
> 
>>
>>
>
> diff --git
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>
>
> new file mode 100644
> index ..b6fbf6c64f8a
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>
>
> @@ -0,0 +1,70 @@
> +/** @file
> +
> +  Parts of the SMM/MM implementation that are specific to
> standalone MM
> +
> +Copyright (c) 2010 - 2018, Intel Corporation. All rights
> reserved.
> +Copyright (c) 2018, Linaro, Ltd. 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
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include "FaultTolerantWrite.h"
> +#include "FaultTolerantWriteSmmCommon.h"
> +
> +BOOLEAN
> +FtwSmmIsBufferOutsideSmmValid (
> +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> +  IN UINT64    Length
> +  )
> +{
> +  return TRUE;
> +}

 Please add function comment header for it, otherwise some coding style
 tool may report error.

> +
> +/**
> +  Internal implementation of CRC32. Depending on the execution
> context
> +  (standalone SMM or DXE vs standalone MM), this function is
> implemented
> +  via a call to the CalculateCrc32 () boot service, or via a library
> +  call.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then
> ASSERT().
> +
> +  @param[in]  Buffer   A pointer to the buffer on which the
> 32-bit CRC is to be computed.
> +  @param[in]  Length   The number of bytes in the buffer Data.
> +
> +  @retval Crc32    The 32-bit CRC was computed for the data
> buffer.
> +
> +**/
> +UINT32
> +FtwCalculateCrc32 (
> +  IN  VOID *Buffer,
> +  IN  UINTN    Length
> +  )
> +{
> +  return CalculateCrc32 (Buffer, Length);
> +}

 Please add function comment header for it, otherwise some coding style
 tool may report error.

> +
> +VOID
> 

Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

[Wang, Jian J]

Star,

I think moving it to MdePkg would be better, just like UefiDriverEntryPoint. A 
dummy
one may be not necessary.

Regards,
Jian


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zeng,
> Star
> Sent: Thursday, January 10, 2019 4:00 PM
> To: Ard Biesheuvel 
> Cc: Wu, Hao A ; edk2-devel@lists.01.org; Gao, Liming
> ; Kinney, Michael D ;
> Laszlo Ersek ; Zeng, Star 
> Subject: Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe:
> implement standalone MM version
> 
> On 2019/1/10 15:33, Ard Biesheuvel wrote:
> > On Thu, 10 Jan 2019 at 08:30, Zeng, Star  wrote:
> >>
> >> Hi Ard,
> >>
> >> Another minor feedback.
> >>
> >> On 2019/1/10 14:47, Zeng, Star wrote:
> >>> Hi Ard,
> >>>
> >>> Some minor feedback added inline.
> >>>
> >>> On 2019/1/4 2:28, Ard Biesheuvel wrote:
>  Implement a new version of the fault tolerant write driver that can
>  be used in the context of a standalone MM implementation.
> 
>  Contributed-under: TianoCore Contribution Agreement 1.1
>  Signed-off-by: Ard Biesheuvel 
>  ---
> 
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
> eMm.c
>  | 70 +++
> 
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon
> eMm.inf
>  | 90 
> 2 files changed, 160 insertions(+)
> >>
> >> Please add it into MdeModulePkg.dsc for package build verification.
> >>
> >
> > Hello Star,
> >
> > Thanks for all the feedback. I will respond in more detail later.
> >
> > However, to the point raised here: it is not possible to add these
> > drivers to MdeModulePkg.dsc unless we add a dummy implementation of
> > StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
> > do that?
> 
> Oh, good information.
> To have full code building coverage for the package, personally I think
> we can move StandaloneMmDriverEntryPoint library class and instance into
> MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should
> be generic enough.
> 
> I do not want to block this patch set because of this. So let's discuss
> this in parallel as separated topic.
> 
> Mike, Liming, Laszlo, Jian and Hao,\
> What's your opinion?
> 
> 
> Thanks,
> Star
> 
> >
> >
> 
>  diff --git
> 
> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.c
> 
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.c
> 
>  new file mode 100644
>  index ..b6fbf6c64f8a
>  --- /dev/null
>  +++
> 
> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal
> oneMm.c
> 
>  @@ -0,0 +1,70 @@
>  +/** @file
>  +
>  +  Parts of the SMM/MM implementation that are specific to standalone
> MM
>  +
>  +Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
>  +Copyright (c) 2018, Linaro, Ltd. 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
>  +http://opensource.org/licenses/bsd-license.php
>  +
>  +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
>  +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR
>  IMPLIED.
>  +
>  +**/
>  +
>  +#include 
>  +#include 
>  +#include "FaultTolerantWrite.h"
>  +#include "FaultTolerantWriteSmmCommon.h"
>  +
>  +BOOLEAN
>  +FtwSmmIsBufferOutsideSmmValid (
>  +  IN EFI_PHYSICAL_ADDRESS  Buffer,
>  +  IN UINT64Length
>  +  )
>  +{
>  +  return TRUE;
>  +}
> >>>
> >>> Please add function comment header for it, otherwise some coding style
> >>> tool may report error.
> >>>
>  +
>  +/**
>  +  Internal implementation of CRC32. Depending on the execution context
>  +  (standalone SMM or DXE vs standalone MM), this function is
> implemented
>  +  via a call to the CalculateCrc32 () boot service, or via a library
>  +  call.
>  +
>  +  If Buffer is NULL, then ASSERT().
>  +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
>  +
>  +  @param[in]  Buffer   A pointer to the buffer on which the
>  32-bit CRC is to be computed.
>  +  @param[in]  Length   The number of bytes in the buffer Data.
>  +
>  +  @retval Crc32The 32-bit CRC was computed for the data
>  buffer.
>  +
>  +**/
>  +UINT32
>  +FtwCalculateCrc32 (
>  +  IN  VOID *Buffer,
>  +  IN  UINTNLength
>  +  )
>  +{
>  +  return CalculateCrc32 (Buffer, Length);
>  +}
> >>>
> >>> Please add function comment header for it, otherwise some coding style
> >>> tool 

Re: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiService.

[Laszlo Ersek]

On 01/10/19 05:51, Ni, Ray wrote:
> 
> 
>> -Original Message-
>> From: Laszlo Ersek 
>> Sent: Wednesday, January 9, 2019 6:35 PM
>> To: Dong, Eric 
>> Cc: Brian J. Johnson ; Yao, Jiewen
>> ; Ni, Ray ; edk2-
>> de...@lists.01.org; Kinney, Michael D 
>> Subject: Re: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid
>> AP calls PeiService.
>>
>> Hi Eric,
>>
>> On 01/09/19 06:26, Dong, Eric wrote:
>>> Hi all,
>>>
>>> We got some feedback about this BZ. Someone think this timeout is
>> valuable for the debug purpose, and oppose to remove it.
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=1419
>>>
>>> So I'm back to here and want to still use this change. I not use "update
>> PcdSpinLockTimeout to 0 in platform" solution because I think core driver
>> depends on platform policy is not a good design.
>>>
>>> Do you guys have any other concern?
>>
>> sorry, I don't understand.
>>
>> (1) In , where
>> currently comment #2 is the last comment, I don't see any request for
>> keeping the timeout facility.
> 
> The comment "timeout is valuable for debugging" is raised in bug scrub 
> meeting.
> (Sorry that's not a public meeting yet. I think there is effort to make it 
> public, not sure).
> 
>>
>> (2) On the mailing list as well, you seem to have received comments only in
>> favor of removing the timeout facility.
>>
>> "Someone think this timeout is valuable for the debug purpose" doesn't cut
>> it, for open development. I don't care about the identity of the person that
>> wants to preserve the feature, but I certainly care about their use case. You
>> shouldn't have to mediate and describe their use case for them, on the list;
>> that's always a lossy process.
> 
> Use case of the timeout:
> When someone mis-uses the spin lock that causes dead lock, timeout assertion
> debug message can help the one to know something wrong happens.
> Instead of the system just hangs without any debug message.
> 
> But I doubt whether the benefit of timeout is bigger than the potential issue 
> it
> brings. Potential issues are:
> 1. User may mis-use time lib in AP procedure causing assertion.
> 2. Not sure HPE Brian's issue is similar to #1. @Brian
> 
> Basically, I also don' t like the idea that a BASE Synchronization library 
> depends on
> a non-BASE timer lib. It makes the Synchronization library a non-BASE lib in 
> most
> of the case. Or platform developer needs some change to make it BASE. Changes
> could be:
> 1. Setting the timeout PCD to 0
> 2. Link to a NULL timer lib.
> 
> But for the purpose of avoiding AP calls PeiService, I agree with Eric's 
> change.
> I don't think the timer lib dependency blocks Eric's change.
> Agree?

Yes, I do. Replacing the AcquireSpinLock() call with a looped
AcquireSpinLockOrFail() call amounts to acquiring the spinlock, but
without depending on the timeout PCD or on the TimerLib class.

Thanks,
Laszlo

>> Regarding the PCD: I think zeroing "PcdSpinLockTimeout" to disable the
>> timeout case is a valid approach, it's just that we should change the default
>> value in the DEC file to zero. Then the PCD setting will become a burden only
>> for those platforms and those use cases that want to use the timeout
>> feature (such as for debugging).
>>
>> In general, PCD default values in DEC files have to be considered carefully,
>> but in some cases, such changes are the right thing. Another example was
>> 509f8425b75d ("UefiCpuPkg: change PcdCpuSmmStackGuard default to
>> TRUE", 2016-06-06).
>>
>> (You made the same point at the end of
>> .)
>>
>>
>> In addition to changing the default value to zero, I'd suggest moving
>> "PcdSpinLockTimeout" from section
>>
>> [PcdsFixedAtBuild,PcdsPatchableInModule]
>>
>> to section
>>
>> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>>
>> so that platforms can enable the "debug" feature (i.e. set a nonzero
>> value) more flexibly.
>>
>> Thanks
>> Laszlo
>>
 -Original Message-
 From: Brian J. Johnson [mailto:brian.john...@hpe.com]
 Sent: Friday, December 21, 2018 12:36 AM
 To: Yao, Jiewen ; Ni, Ruiyu
 ; Dong, Eric ;
 edk2-devel@lists.01.org
 Cc: Laszlo Ersek 
 Subject: Re: [edk2] [Patch 1/3] UefiCpuPkg/RegisterCpuFeaturesLib:
 Avoid AP calls PeiService.

 Agreed.  We've seen issues on real platforms with timed-out spinlocks
 in DXE causing calls to GetPerformanceCounter and DebugAssert.  (DXE
 has the same code, with the same issues.)

 Note that it's possible to set PcdSpinLockTimeout=0 to work around
 the issue on a particular platform, or in a particular module.  But
 if you have to do that for every module which uses APs, and hence
 could contend for a spinlock, it kind of defeats the point  We're 
 better
>> off removing the timeout code.

 Thanks,
 Brian

 On 12/19/18 8:08 PM, Yao, Jiewen wrote:

[edk2] [PATCH v2] MdePkg/BaseLib: Add Base64Encode() and Base64Decode()

[Shenglei Zhang]

Introduce public functions Base64Encode and Base64Decode.
https://bugzilla.tianocore.org/show_bug.cgi?id=1370

v2:1.Remove some white space.
   2.Add unit test with test vectors in RFC 4648.
 https://github.com/shenglei10/edk2/tree/encode_test
 https://github.com/shenglei10/edk2/tree/decode_test

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang 
---
 MdePkg/Include/Library/BaseLib.h |  56 +
 MdePkg/Library/BaseLib/String.c  | 345 +++
 2 files changed, 401 insertions(+)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 1eb842384e..9317888a7e 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -2720,6 +2720,62 @@ AsciiStrnToUnicodeStrS (
   OUT UINTN *DestinationLength
   );
 
+/**
+  Convert binary data to a Base64 encoded ascii string based on RFC4648.
+
+  Produce a Null-terminated Ascii string in the output buffer specified by 
AsciiPtr and AsciiSize.
+  The Ascii string is produced by converting the data string specified by 
DataPtr and DataLen.
+
+  @param DataPtr Input UINT8 data
+  @param DataLenNumber of UINT8 bytes of data
+  @param AsciiPtr  Pointer to output string buffer
+  @param AsciiSize   Size of ascii buffer. Set to 0 to get the size needed.
+   Caller is responsible for passing in buffer of 
AsciiSize
+
+  @retval RETURN_SUCCESSWhen ascii buffer is filled in.
+  @retval RETURN_INVALID_PARAMETER   If DataPtr is NULL or AsciiSize is NULL.
+  @retval RETURN_INVALID_PARAMETER   If DataLen or AsciiSize is too big.
+  @retval RETURN_BUFFER_TOO_SMALL   If DataLen is 0 and AsciiSize is <1.
+  @retval RETURN_BUFFER_TOO_SMALL   If AsciiPtr is NULL or too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+Base64Encode (
+  IN  CONST UINT8  *DataPtr,
+  INUINTN   DataLen,
+  OUT   CHAR8  *AsciiPtr  OPTIONAL,
+  IN OUTUINTN  *AsciiSize
+  );
+
+/**
+  Convert Base64 ascii string to binary data based on RFC4648.
+
+  Produce Null-terminated binary data in the output buffer specified by BinPtr 
and BinSize.
+  The binary data is produced by converting the Base64 ascii string specified 
by DataPtr and DataLen.
+
+  @param DataPtr  Input ASCII characters
+  @param DataLen Number of ASCII characters
+  @param BinPtrPointer to output buffer
+  @param BinSize  Caller is responsible for passing in buffer of at least 
BinSize.
+Set 0 to get the size needed. Set to bytes 
stored on return.
+
+  @retval RETURN_SUCCESSWhen binary buffer is filled in.
+  @retval RETURN_INVALID_PARAMETER   If DataPtr is NULL or BinSize is NULL.
+  @retval RETURN_INVALID_PARAMETER   If DataLen or BinSize is too big.
+  @retval RETURN_INVALID_PARAMETER   If BinPtr NULL and BinSize != 0.
+  @retval RETURN_INVALID_PARAMETER   If there is any Invalid character in 
input stream.
+  @retval RETURN_BUFFER_TOO_SMALL   If Buffer length is too small.
+ **/
+RETURN_STATUS
+EFIAPI
+Base64Decode (
+  IN  CONST CHAR8  *DataPtr,
+  INUINTN   DataLen,
+  OUT   UINT8  *BinPtr  OPTIONAL,
+  IN OUTUINTN  *BinSize
+  );
+
 /**
   Converts an 8-bit value to an 8-bit BCD value.
 
diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
index e6df12797d..761ff6830a 100644
--- a/MdePkg/Library/BaseLib/String.c
+++ b/MdePkg/Library/BaseLib/String.c
@@ -1763,6 +1763,351 @@ AsciiStrToUnicodeStr (
 
 #endif
 
+//
+// The basis for Base64 encoding is RFC 4686 
https://tools.ietf.org/html/rfc4648
+//
+// RFC 4686 has a number of MAY and SHOULD cases.  This implementation chooses
+// the more restrictive versions for security concerns (see RFC 4686 section 
3.3).
+//
+// A invalid character, if encountered during the decode operation, causes the 
data
+// to be rejected. In addition, the '=' padding character is only allowed at 
the end
+// of the Base64 encoded string.
+//
+#define BAD_V  99
+
+STATIC CHAR8 Encoding_table[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+"abcdefghijklmnopqrstuvwxyz"
+"0123456789+/";
+
+STATIC UINT8 Decoding_table[] = {
+  //
+  // Valid characters 
ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/
+  // Also, set '=' as a zero for decoding
+  // 0  ,1,   2,   3,4,   5,   
 6,   7,   8,9,   a,b,  
  c,   d,e,f
+  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  
BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,   //   0
+  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  
BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,   //  10
+  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  BAD_V,  

Re: [edk2] EDK II Network Stack Issue

[Karin Willers]

On 2019-01-07 20:27, Laszlo Ersek wrote:

On 01/04/19 15:02, Karin Willers wrote:

G'Day!

I'm trying to get networking under edk2 up and running. I tried
AppPkg/Applications/Sockets/RawIp4Tx
under OVMF. The raw packet is sent out on the network, but the
application never returns from the
socket close routine.

I'm currently using UDK2017 with the latest security patches 
(downloaded

December 18 2018). The network
driver under OVMF is the e1000 driver.

The effect that the socket close never returns is also visible when
running RawIp4Tx on real hardware,
so I think the behavior has nothing to do with OVMF or the UEFI 
itself.


Does anyone see similar effects? Any hints on setting up networking
under edk2 correctly?


The socket (= libc-level networking) APIs are not the most robust ones
in edk2, in my -- admittedly limited -- experience. I'd suggest using
applications and shell commands that use UEFI protocols instead, for
networking. (I understand that could be a challenge if you are porting 
a

standard C program to UEFI.)

Thanks
Laszlo


The reason for this undertaking initially was to compile a Python.efi 
that
supports sockets. We wanted that to be able to run the Chipsec suite 
with

external network test partners under UEFI. The Chipsec guys do provide
pre-compiled Pyhon executables, but these do not include the network 
stack.


I think, I have to debug the issue myself ...

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

Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation

[Gao, Liming]

Pushed them at 0290fca..15666b

> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Thursday, January 10, 2019 11:33 PM
> To: Gao, Liming ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin 
> Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Thanks Liming. I have files BZ: 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1444 to update UEFI drivers to 
> use new APIs. I have not
> assigned it to anyone as there are many drivers across packages that need to 
> be looked at. I would try to fix the ones I hit an issue with.
> 
> Over the weekend Siyuan approved the patch from PATCH v2 which is exactly 
> same as in PATCH v4.
> 
> Thanks
> Ashish
> 
> -Original Message-
> From: Gao, Liming 
> Sent: Thursday, January 10, 2019 8:23 AM
> To: Ashish Singhal ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin 
> Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Ashish:
>   The MdePkg change is good to me. Reviewed-by: Liming Gao 
> 
>   Please help submit another BZ to update UefiDriver to uninstall protocol 
> when failure with new APIs.
> 
>   If Siyuan/Jiaxin has no other comments, I will help push this patch set.
> 
> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ashish Singhal
> > Sent: Thursday, January 10, 2019 9:19 AM
> > To: Kinney, Michael D ;
> > edk2-devel@lists.01.org
> > Cc: Fu, Siyuan ; Wu, Jiaxin
> > ; Gao, Liming 
> > Subject: Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for
> > protocol uninstallation
> >
> > Thanks Mike. Hope to see the patches merged soon. Please let me know if you 
> > want me to file the BZ.
> >
> > Hi Liming,
> >
> > Please let me know if you need me to take care of anything in the patch 
> > before you push it.
> >
> > Hi Siyuan/Jiaxin,
> >
> > I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. 
> > Please let me know if you have any issues with this going
> in.
> >
> > Thanks
> > Ashish
> >
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Wednesday, January 9, 2019 5:56 PM
> > To: Ashish Singhal ; edk2-devel@lists.01.org;
> > Kinney, Michael D 
> > Cc: Gao, Liming ; Fu, Siyuan
> > ; Wu, Jiaxin 
> > Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol
> > uninstallation
> >
> > Hi Ashish,
> >
> > This V4 version of the patch produces the expected size results for 
> > platform and driver builds.
> >
> > There are some very minor issues with some extra carriage returns, but
> > those can be handled by Liming when the patch series is committed.
> >
> > I may be good to have an additional BZ to use these new APIs from all
> > UEFI Driver Model drivers that have failure paths in their entry point or 
> > support the unload feature.
> > Those updates can be done later.
> >
> > Thanks,
> >
> > Mike
> >
> > > -Original Message-
> > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> > > Sent: Wednesday, January 9, 2019 12:59 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Kinney, Michael D ; Gao, Liming
> > > ; Fu, Siyuan ; Wu, Jiaxin
> > > ; Ashish Singhal 
> > > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol
> > > uninstallation
> > >
> > > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup
> > > after initialization failure was not done right. Bug 1428 was filed
> > > in this regard.
> > > As per discussions with Mike, it was also discussed that having
> > > UEFILib provide protocol uninstallation abstraction would help to
> > > avoid these issues in the future. Bug 1429 was found to track this.
> > > These 2 patches
> > > take care of this.
> > >
> > >
> > > Ashish Singhal (2):
> > >   MdePkg/UefiLib: Abstract driver model protocol uninstallation
> > >   NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
> > >
> > >  MdePkg/Include/Library/UefiLib.h | 103 
> > >  MdePkg/Library/UefiLib/UefiDriverModel.c | 972
> > > ++-
> > >  NetworkPkg/IScsiDxe/IScsiDriver.c|  31 +-
> > >  3 files changed, 1085 insertions(+), 21 deletions(-)
> > >
> > > --
> > > 2.7.4
> >
> > --
> > - This email message is for the sole use of the intended
> > recipient(s) and may contain confidential information.  Any
> > unauthorized review, use, disclosure or distribution is prohibited.
> > If you are not the intended recipient, please contact the sender by
> > reply email and destroy all copies of the original message.
> > --
> > - ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Conditional Compilation support in INF file

[Gao, Liming]

I have same question. What flexibility is expected in INF? I see one request in 
[Depex] section. So, PCD support in [Depex] is added. 

Edk2 INF is used to describe the source code behavior. If the source uses 
Ppi/Protocol/Guid/Pcd, these information are always required to be described in 
INF file. The compiler can optimize the code and remove the unused 
Ppi/Protocol/Guid/Pcd. It doesn't need developer specify the conditional 
statement. 

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, January 10, 2019 8:54 PM
> To: karunakarpoosapa...@dell.com; Gao, Liming ; 
> edk2-devel@lists.01.org
> Cc: sumanth.vidyadh...@dell.com; sriramkumar.r...@dell.com
> Subject: Re: [edk2] Conditional Compilation support in INF file
> 
> On 01/10/19 07:03, karunakarpoosapa...@dell.com wrote:
> > Hi All,
> >
> > I agree with providing the support like "FixedAtBuild PCD in INF". And we 
> > need to modify or provide support in BaseTools to support
> this feature.
> >
> > There are more use cases or flexibility to developer if we support 
> > Conditional compilation support in INF.
> > As we're providing support in BaseTools for FixedAtBuild PCD support in 
> > INF, Is there any challenges or drawbacks in  providing
> conditional compilation support in INF?
> 
> This is not for me to say authoritatively, but I'm unaware of any
> specific use case that cannot be solved without this feature addition,
> and any further complexity to BaseTools should be strongly justified.
> "More convenient" is too vague for me, and the BaseTools code is already
> hard to read and debug.
> 
> That's just my opinion, again.
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Conditional Compilation support in INF file

[Kinney, Michael D]

There is one additional constraint for adding conditionals
to INF files.

The UEFI Distribution Packaging Specification defines a 
format to share package and module content in a standard
format and uses an XML schema for the metadata.  We need to
be able to convert between INF <--> XML and DEC <--> XML.

http://www.uefi.org/sites/default/files/resources/Dist_Package_Spec_1_1.pdf

If we define extensions to INF or DEC files, we need to
make sure these transforms are still supported.  If an
extension prevents these transforms, then we either need
to change the extension to be compatible or work on an
update to the UDP spec to support the extension in the XML.

Best regards,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Gao, Liming
> Sent: Thursday, January 10, 2019 7:48 AM
> To: Laszlo Ersek ;
> karunakarpoosapa...@dell.com; edk2-devel@lists.01.org
> Cc: sumanth.vidyadh...@dell.com; Gao, Liming
> ; sriramkumar.r...@dell.com
> Subject: Re: [edk2] Conditional Compilation support in
> INF file
> 
> I have same question. What flexibility is expected in
> INF? I see one request in [Depex] section. So, PCD
> support in [Depex] is added.
> 
> Edk2 INF is used to describe the source code behavior.
> If the source uses Ppi/Protocol/Guid/Pcd, these
> information are always required to be described in INF
> file. The compiler can optimize the code and remove the
> unused Ppi/Protocol/Guid/Pcd. It doesn't need developer
> specify the conditional statement.
> 
> Thanks
> Liming
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Thursday, January 10, 2019 8:54 PM
> > To: karunakarpoosapa...@dell.com; Gao, Liming
> ; edk2-devel@lists.01.org
> > Cc: sumanth.vidyadh...@dell.com;
> sriramkumar.r...@dell.com
> > Subject: Re: [edk2] Conditional Compilation support in
> INF file
> >
> > On 01/10/19 07:03, karunakarpoosapa...@dell.com wrote:
> > > Hi All,
> > >
> > > I agree with providing the support like
> "FixedAtBuild PCD in INF". And we need to modify or
> provide support in BaseTools to support
> > this feature.
> > >
> > > There are more use cases or flexibility to developer
> if we support Conditional compilation support in INF.
> > > As we're providing support in BaseTools for
> FixedAtBuild PCD support in INF, Is there any challenges
> or drawbacks in  providing
> > conditional compilation support in INF?
> >
> > This is not for me to say authoritatively, but I'm
> unaware of any
> > specific use case that cannot be solved without this
> feature addition,
> > and any further complexity to BaseTools should be
> strongly justified.
> > "More convenient" is too vague for me, and the
> BaseTools code is already
> > hard to read and debug.
> >
> > That's just my opinion, again.
> >
> > Thanks
> > Laszlo
> ___
> 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 v4 0/2] Provide UEFILib functions for protocol uninstallation

[Ashish Singhal]

Thanks Liming. I have files BZ: 
https://bugzilla.tianocore.org/show_bug.cgi?id=1444 to update UEFI drivers to 
use new APIs. I have not assigned it to anyone as there are many drivers across 
packages that need to be looked at. I would try to fix the ones I hit an issue 
with.

Over the weekend Siyuan approved the patch from PATCH v2 which is exactly same 
as in PATCH v4.

Thanks
Ashish

-Original Message-
From: Gao, Liming  
Sent: Thursday, January 10, 2019 8:23 AM
To: Ashish Singhal ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Wu, Jiaxin 
Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
uninstallation

Ashish:
  The MdePkg change is good to me. Reviewed-by: Liming Gao 

  Please help submit another BZ to update UefiDriver to uninstall protocol when 
failure with new APIs. 
  
  If Siyuan/Jiaxin has no other comments, I will help push this patch set. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ashish Singhal
> Sent: Thursday, January 10, 2019 9:19 AM
> To: Kinney, Michael D ; 
> edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin 
> ; Gao, Liming 
> Subject: Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for 
> protocol uninstallation
> 
> Thanks Mike. Hope to see the patches merged soon. Please let me know if you 
> want me to file the BZ.
> 
> Hi Liming,
> 
> Please let me know if you need me to take care of anything in the patch 
> before you push it.
> 
> Hi Siyuan/Jiaxin,
> 
> I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. 
> Please let me know if you have any issues with this going in.
> 
> Thanks
> Ashish
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, January 9, 2019 5:56 PM
> To: Ashish Singhal ; edk2-devel@lists.01.org; 
> Kinney, Michael D 
> Cc: Gao, Liming ; Fu, Siyuan 
> ; Wu, Jiaxin 
> Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Hi Ashish,
> 
> This V4 version of the patch produces the expected size results for platform 
> and driver builds.
> 
> There are some very minor issues with some extra carriage returns, but 
> those can be handled by Liming when the patch series is committed.
> 
> I may be good to have an additional BZ to use these new APIs from all 
> UEFI Driver Model drivers that have failure paths in their entry point or 
> support the unload feature.
> Those updates can be done later.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> > Sent: Wednesday, January 9, 2019 12:59 PM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D ; Gao, Liming 
> > ; Fu, Siyuan ; Wu, Jiaxin 
> > ; Ashish Singhal 
> > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> > uninstallation
> >
> > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup 
> > after initialization failure was not done right. Bug 1428 was filed 
> > in this regard.
> > As per discussions with Mike, it was also discussed that having 
> > UEFILib provide protocol uninstallation abstraction would help to 
> > avoid these issues in the future. Bug 1429 was found to track this.
> > These 2 patches
> > take care of this.
> >
> >
> > Ashish Singhal (2):
> >   MdePkg/UefiLib: Abstract driver model protocol uninstallation
> >   NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
> >
> >  MdePkg/Include/Library/UefiLib.h | 103 
> >  MdePkg/Library/UefiLib/UefiDriverModel.c | 972
> > ++-
> >  NetworkPkg/IScsiDxe/IScsiDriver.c|  31 +-
> >  3 files changed, 1085 insertions(+), 21 deletions(-)
> >
> > --
> > 2.7.4
> 
> --
> - This email message is for the sole use of the intended 
> recipient(s) and may contain confidential information.  Any 
> unauthorized review, use, disclosure or distribution is prohibited.  
> If you are not the intended recipient, please contact the sender by 
> reply email and destroy all copies of the original message.
> --
> - ___
> 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 v4 0/2] Provide UEFILib functions for protocol uninstallation

[Ashish Singhal]

Thanks everyone.

-Original Message-
From: Gao, Liming  
Sent: Thursday, January 10, 2019 8:39 AM
To: Ashish Singhal ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Wu, Jiaxin 
Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
uninstallation

Pushed them at 0290fca..15666b

> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Thursday, January 10, 2019 11:33 PM
> To: Gao, Liming ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin 
> Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Thanks Liming. I have files BZ: 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1444 to update UEFI drivers to 
> use new APIs. I have not
> assigned it to anyone as there are many drivers across packages that need to 
> be looked at. I would try to fix the ones I hit an issue with.
> 
> Over the weekend Siyuan approved the patch from PATCH v2 which is exactly 
> same as in PATCH v4.
> 
> Thanks
> Ashish
> 
> -Original Message-
> From: Gao, Liming 
> Sent: Thursday, January 10, 2019 8:23 AM
> To: Ashish Singhal ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin 
> Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Ashish:
>   The MdePkg change is good to me. Reviewed-by: Liming Gao 
> 
>   Please help submit another BZ to update UefiDriver to uninstall protocol 
> when failure with new APIs.
> 
>   If Siyuan/Jiaxin has no other comments, I will help push this patch set.
> 
> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ashish Singhal
> > Sent: Thursday, January 10, 2019 9:19 AM
> > To: Kinney, Michael D ;
> > edk2-devel@lists.01.org
> > Cc: Fu, Siyuan ; Wu, Jiaxin
> > ; Gao, Liming 
> > Subject: Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for
> > protocol uninstallation
> >
> > Thanks Mike. Hope to see the patches merged soon. Please let me know if you 
> > want me to file the BZ.
> >
> > Hi Liming,
> >
> > Please let me know if you need me to take care of anything in the patch 
> > before you push it.
> >
> > Hi Siyuan/Jiaxin,
> >
> > I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. 
> > Please let me know if you have any issues with this going
> in.
> >
> > Thanks
> > Ashish
> >
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Wednesday, January 9, 2019 5:56 PM
> > To: Ashish Singhal ; edk2-devel@lists.01.org;
> > Kinney, Michael D 
> > Cc: Gao, Liming ; Fu, Siyuan
> > ; Wu, Jiaxin 
> > Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol
> > uninstallation
> >
> > Hi Ashish,
> >
> > This V4 version of the patch produces the expected size results for 
> > platform and driver builds.
> >
> > There are some very minor issues with some extra carriage returns, but
> > those can be handled by Liming when the patch series is committed.
> >
> > I may be good to have an additional BZ to use these new APIs from all
> > UEFI Driver Model drivers that have failure paths in their entry point or 
> > support the unload feature.
> > Those updates can be done later.
> >
> > Thanks,
> >
> > Mike
> >
> > > -Original Message-
> > > From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> > > Sent: Wednesday, January 9, 2019 12:59 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Kinney, Michael D ; Gao, Liming
> > > ; Fu, Siyuan ; Wu, Jiaxin
> > > ; Ashish Singhal 
> > > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol
> > > uninstallation
> > >
> > > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup
> > > after initialization failure was not done right. Bug 1428 was filed
> > > in this regard.
> > > As per discussions with Mike, it was also discussed that having
> > > UEFILib provide protocol uninstallation abstraction would help to
> > > avoid these issues in the future. Bug 1429 was found to track this.
> > > These 2 patches
> > > take care of this.
> > >
> > >
> > > Ashish Singhal (2):
> > >   MdePkg/UefiLib: Abstract driver model protocol uninstallation
> > >   NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
> > >
> > >  MdePkg/Include/Library/UefiLib.h | 103 
> > >  MdePkg/Library/UefiLib/UefiDriverModel.c | 972
> > > ++-
> > >  NetworkPkg/IScsiDxe/IScsiDriver.c|  31 +-
> > >  3 files changed, 1085 insertions(+), 21 deletions(-)
> > >
> > > --
> > > 2.7.4
> >
> > --
> > - This email message is for the sole use of the intended
> > recipient(s) and may contain confidential information.  Any
> > unauthorized review, use, disclosure or distribution is prohibited.
> > If you are not the intended recipient, please contact the sender by
> > reply email and destroy all copies of the original message.
> > 

Re: [edk2] A question about shell-application's argument make system blocked;

[Carsey, Jaben]

Is this in a script file?  I don't remember how "comments" work on raw command 
lines where the user types them.

-Jaben


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> krishnaLee
> Sent: Wednesday, January 09, 2019 10:13 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] A question about shell-application's argument make system
> blocked;
> Importance: High
> 
> Hi everybody,
> I meet a question,a special arg can make system blocked,follow is my steps.
> 1,go to uefi shell v2.2(uefi v2.70),run this application in QEMU-ovmf:
> testapp.efi
> 2,the output is "index:0,string:FS0:\testapp.efi"
> 
> 
> 3,testapp.efi #abc.
> 4,the output is same as step 2.  ///< I had read the uefi shell specification
> 2.2,the '#' is a comment remark,so I think it is ok.
> 
> 
> 5 testapp.efi "#abc"
> 6,the system blocked(dead).  ///< I think it is a bug.
> 
> 
> //follow is the testapp.efi source code:
> EFI_STATUS
> EFIAPI
> UefiMain (
> IN EFI_HANDLE ImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> EFI_STATUS status;
> EFI_SHELL_PARAMETERS_PROTOCOL* param;
> status=SystemTable->BootServices-
> >HandleProtocol(ImageHandle,,);
> if(status!=EFI_SUCCESS)
> {
> return0;
> }
> 
> 
> for(UINTN i=0;i< param->Argc;i++)
> {
> Print(L"index:%d,string:%s\n",i,param->Argv[i]);
> }
> 
> 
> return EFI_SUCCESS;
> }
> 
> 
> //test environment:
> //QEMU v2.10.95 + edk2-2018-ovmf-x64.
> //shell command line:
> //"D:\qemu\qemu-system-x86_64.exe" -machine pc-q35-2.9 -pflash
> "D:\qemu\bios\OVMF_x64_debug.fd" -serial stdio -hda fat:rw:G:\temp -net
> none
> //end
> 
> 
> 
> 
> 
> 
> thanks,
> krishna.
> 
> 
> 
> ___
> 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 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

[Ard Biesheuvel]

On Thu, 10 Jan 2019 at 14:03, Laszlo Ersek  wrote:
>
> On 01/10/19 08:59, Zeng, Star wrote:
> > On 2019/1/10 15:33, Ard Biesheuvel wrote:
> >> On Thu, 10 Jan 2019 at 08:30, Zeng, Star  wrote:
> >>>
> >>> Hi Ard,
> >>>
> >>> Another minor feedback.
> >>>
> >>> On 2019/1/10 14:47, Zeng, Star wrote:
>  Hi Ard,
> 
>  Some minor feedback added inline.
> 
>  On 2019/1/4 2:28, Ard Biesheuvel wrote:
> > Implement a new version of the fault tolerant write driver that can
> > be used in the context of a standalone MM implementation.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >
> > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
> >
> > | 70 +++
> >
> > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> >
> > | 90 
> >2 files changed, 160 insertions(+)
> >>>
> >>> Please add it into MdeModulePkg.dsc for package build verification.
> >>>
> >>
> >> Hello Star,
> >>
> >> Thanks for all the feedback. I will respond in more detail later.
> >>
> >> However, to the point raised here: it is not possible to add these
> >> drivers to MdeModulePkg.dsc unless we add a dummy implementation of
> >> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
> >> do that?
> >
> > Oh, good information.
> > To have full code building coverage for the package, personally I think
> > we can move StandaloneMmDriverEntryPoint library class and instance into
> > MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should
> > be generic enough.
> >
> > I do not want to block this patch set because of this. So let's discuss
> > this in parallel as separated topic.
> >
> > Mike, Liming, Laszlo, Jian and Hao,\
> > What's your opinion?
>
> It should be possible to build all library instances in a central
> Package (well, all Packages really), using the Package's DSC file. To my
> understanding, libraries built like this are not expected to be used in
> actual (shipped) drivers / applications, nor is their indiscriminate
> distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull
> library instance in binary form seems quite useless.
>
> With that in mind, I think a Null instance for the entry point in
> question makes sense, under MdeModulePkg.
>

I will look into this a bit deeper next week. I think it makes sense
for the core PI architected pieces to all live in MdePkg rather than
StandaloneMmPkg. For instance, MmServicesTableLib for standalone MM
should live there, MmEntryPoint should live there (and have
traditional and standalone MM implementation) and perhaps some other
core pieces as well.

This may be a slippery slope, so I will dedicate some time to look
into this carefully, at least with the goal to make the
FaultTolerantWrite and Variable driver buildable from within
MdeModulePkg.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] ShellPkg/TftpDynamicCommand: Change file writing method in tftp

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Li, Songpeng
> Sent: Wednesday, January 09, 2019 6:54 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ni, Ray ;
> Wu, Jiaxin 
> Subject: [PATCH v2] ShellPkg/TftpDynamicCommand: Change file writing
> method in tftp
> Importance: High
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1433
> 
> v2: Remove an unused variable.
> 
> Current logic of shell tftp download was writing file after tftp
> download finished, when the file is large, it looks like the shell
> tftp command hanged after download was finished. To improve
> end-user experience, the solution is using split file writing
> instead.
> 
> This patch update the code to open and close file inside
> DownloadFile(), and save each packet to file within callback
> function CheckPacket().
> 
> Since AllocatePage() is no-longer needed, This patch can also
> remove the memory limitation. The download file can be larger
> than system free memory now.
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Cc: Wu Jiaxin 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Songpeng Li 
> ---
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c | 154 -
> ---
>  1 file changed, 64 insertions(+), 90 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> index ed081b5bad7c..ba753a279b00 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> @@ -41,6 +41,12 @@ STATIC CONST CHAR16 mTftpProgressFrame[] = L"[
>  // (TFTP_PROGRESS_MESSAGE_SIZE-1) '\b'
>  STATIC CONST CHAR16 mTftpProgressDelete[] =
> L"\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\
> b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b";
> 
> +// Local File Handle
> +SHELL_FILE_HANDLE mFileHandle;
> +
> +// Path of the local file, Unicode encoded
> +CONST CHAR16 *mLocalFilePath;
> +
>  /**
>Check and convert the UINT16 option values of the 'tftp' command
> 
> @@ -166,9 +172,6 @@ GetFileSize (
>@param[in]   FileSize   Size of the file in number of bytes
>@param[in]   BlockSize  Value of the TFTP blksize option
>@param[in]   WindowSize Value of the TFTP window size 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.
> 
>@retval  EFI_SUCCESS   The file was downloaded.
>@retval  EFI_OUT_OF_RESOURCES  A memory allocation failed.
> @@ -184,8 +187,7 @@ DownloadFile (
>IN   CONST CHAR8  *AsciiFilePath,
>IN   UINTNFileSize,
>IN   UINT16   BlockSize,
> -  IN   UINT16   WindowSize,
> -  OUT  VOID **Data
> +  IN   UINT16   WindowSize
>);
> 
>  /**
> @@ -287,7 +289,6 @@ RunTftp (
>CHAR8   *AsciiRemoteFilePath;
>UINTN   FilePathSize;
>CONST CHAR16*Walker;
> -  CONST CHAR16*LocalFilePath;
>EFI_MTFTP4_CONFIG_DATA  Mtftp4ConfigData;
>EFI_HANDLE  *Handles;
>UINTN   HandleCount;
> @@ -297,9 +298,6 @@ RunTftp (
>EFI_HANDLE  Mtftp4ChildHandle;
>EFI_MTFTP4_PROTOCOL *Mtftp4;
>UINTN   FileSize;
> -  UINTN   DataSize;
> -  VOID*Data;
> -  SHELL_FILE_HANDLE   FileHandle;
>UINT16  BlockSize;
>UINT16  WindowSize;
> 
> @@ -309,7 +307,6 @@ RunTftp (
>AsciiRemoteFilePath = NULL;
>Handles = NULL;
>FileSize= 0;
> -  DataSize= 0;
>BlockSize   = MTFTP_DEFAULT_BLKSIZE;
>WindowSize  = MTFTP_DEFAULT_WINDOWSIZE;
> 
> @@ -385,7 +382,7 @@ RunTftp (
>UnicodeStrToAsciiStrS (RemoteFilePath, AsciiRemoteFilePath, FilePathSize);
> 
>if (ParamCount == 4) {
> -LocalFilePath = ShellCommandLineGetRawValue (CheckPackage, 3);
> +mLocalFilePath = ShellCommandLineGetRawValue (CheckPackage, 3);
>} else {
>  Walker = RemoteFilePath + StrLen (RemoteFilePath);
>  while ((--Walker) >= RemoteFilePath) {
> @@ -394,7 +391,7 @@ RunTftp (
>  break;
>}
>  }
> -LocalFilePath = Walker + 1;
> +mLocalFilePath = Walker + 1;
>}
> 
>//
> @@ -492,7 +489,6 @@ RunTftp (
> (NicNumber < HandleCount) && (ShellStatus != SHELL_SUCCESS);
> NicNumber++) {
>  ControllerHandle = Handles[NicNumber];
> -Data = NULL;
> 
>  Status = GetNicName (ControllerHandle, NicNumber, NicName);
>  if (EFI_ERROR (Status)) {
> @@ -543,7 +539,7 @@ RunTftp (
>goto NextHandle;
>  }
> 
> -Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, WindowSize, );
> +Status = DownloadFile (Mtftp4, 

Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol uninstallation

[Gao, Liming]

Ashish:
  The MdePkg change is good to me. Reviewed-by: Liming Gao 

  Please help submit another BZ to update UefiDriver to uninstall protocol when 
failure with new APIs. 
  
  If Siyuan/Jiaxin has no other comments, I will help push this patch set. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ashish 
> Singhal
> Sent: Thursday, January 10, 2019 9:19 AM
> To: Kinney, Michael D ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin ; Gao, 
> Liming 
> Subject: Re: [edk2] [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Thanks Mike. Hope to see the patches merged soon. Please let me know if you 
> want me to file the BZ.
> 
> Hi Liming,
> 
> Please let me know if you need me to take care of anything in the patch 
> before you push it.
> 
> Hi Siyuan/Jiaxin,
> 
> I think you reviewed the changes in PATCH v2 which is same as in PATCH v4. 
> Please let me know if you have any issues with this going in.
> 
> Thanks
> Ashish
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, January 9, 2019 5:56 PM
> To: Ashish Singhal ; edk2-devel@lists.01.org; 
> Kinney, Michael D 
> Cc: Gao, Liming ; Fu, Siyuan ; Wu, 
> Jiaxin 
> Subject: RE: [PATCH v4 0/2] Provide UEFILib functions for protocol 
> uninstallation
> 
> Hi Ashish,
> 
> This V4 version of the patch produces the expected size results for platform 
> and driver builds.
> 
> There are some very minor issues with some extra carriage returns, but those 
> can be handled by Liming when the patch series is
> committed.
> 
> I may be good to have an additional BZ to use these new APIs from all UEFI 
> Driver Model drivers that have failure paths in their entry
> point or support the unload feature.
> Those updates can be done later.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> > Sent: Wednesday, January 9, 2019 12:59 PM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D ; Gao, Liming
> > ; Fu, Siyuan ; Wu, Jiaxin
> > ; Ashish Singhal 
> > Subject: [PATCH v4 0/2] Provide UEFILib functions for protocol
> > uninstallation
> >
> > An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after
> > initialization failure was not done right. Bug 1428 was filed in this
> > regard.
> > As per discussions with Mike, it was also discussed that having
> > UEFILib provide protocol uninstallation abstraction would help to
> > avoid these issues in the future. Bug 1429 was found to track this.
> > These 2 patches
> > take care of this.
> >
> >
> > Ashish Singhal (2):
> >   MdePkg/UefiLib: Abstract driver model protocol uninstallation
> >   NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
> >
> >  MdePkg/Include/Library/UefiLib.h | 103 
> >  MdePkg/Library/UefiLib/UefiDriverModel.c | 972
> > ++-
> >  NetworkPkg/IScsiDxe/IScsiDriver.c|  31 +-
> >  3 files changed, 1085 insertions(+), 21 deletions(-)
> >
> > --
> > 2.7.4
> 
> ---
> This email message is for the sole use of the intended recipient(s) and may 
> contain
> confidential information.  Any unauthorized review, use, disclosure or 
> distribution
> is prohibited.  If you are not the intended recipient, please contact the 
> sender by
> reply email and destroy all copies of the original message.
> ---
> ___
> 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 v1 3/5] BaseTools/DscBuildData: move function

[Jaben Carsey]

Move IsFieldValuieAnArray from Common.Misc to this file.
There were no other consumers of the function.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/Misc.py| 18 +--
 BaseTools/Source/Python/Workspace/DscBuildData.py | 50 +---
 2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 25d8f9807fa3..7e8077558da5 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -1,7 +1,7 @@
 ## @file
 # Common routines used by all tools
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2019, 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
@@ -1149,22 +1149,6 @@ class tdict:
 keys |= self.data[Key].GetKeys(KeyIndex - 1)
 return keys
 
-def IsFieldValueAnArray (Value):
-Value = Value.strip()
-if Value.startswith(TAB_GUID) and Value.endswith(')'):
-return True
-if Value.startswith('L"') and Value.endswith('"')  and 
len(list(Value[2:-1])) > 1:
-return True
-if Value[0] == '"' and Value[-1] == '"' and len(list(Value[1:-1])) > 1:
-return True
-if Value[0] == '{' and Value[-1] == '}':
-return True
-if Value.startswith("L'") and Value.endswith("'") and 
len(list(Value[2:-1])) > 1:
-return True
-if Value[0] == "'" and Value[-1] == "'" and len(list(Value[1:-1])) > 1:
-return True
-return False
-
 def AnalyzePcdExpression(Setting):
 RanStr = ''.join(sample(string.ascii_letters + string.digits, 8))
 Setting = Setting.replace('', RanStr).strip()
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 3b0b23ca8fcb..f4a8212f412d 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1,7 +1,7 @@
 ## @file
 # This file is used to create a database used by build tool
 #
-# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2008 - 2019, 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
@@ -44,6 +44,22 @@ from Workspace.BuildClassObject import 
PlatformBuildClassObject, StructurePcd, P
 from collections import OrderedDict, defaultdict
 from .BuildClassObject import ArrayIndex
 
+def _IsFieldValueAnArray (Value):
+Value = Value.strip()
+if Value.startswith(TAB_GUID) and Value.endswith(')'):
+return True
+if Value.startswith('L"') and Value.endswith('"')  and 
len(list(Value[2:-1])) > 1:
+return True
+if Value[0] == '"' and Value[-1] == '"' and len(list(Value[1:-1])) > 1:
+return True
+if Value[0] == '{' and Value[-1] == '}':
+return True
+if Value.startswith("L'") and Value.endswith("'") and 
len(list(Value[2:-1])) > 1:
+return True
+if Value[0] == "'" and Value[-1] == "'" and len(list(Value[1:-1])) > 1:
+return True
+return False
+
 PcdValueInitName = 'PcdValueInit'
 
 PcdMainCHeader = '''
@@ -1105,7 +1121,7 @@ class DscBuildData(PlatformBuildClassObject):
 IsArray = False
 TokenCName += '.' + FieldName
 if PcdValue.startswith('H'):
-if FieldName and IsFieldValueAnArray(PcdValue[1:]):
+if FieldName and _IsFieldValueAnArray(PcdValue[1:]):
 PcdDatumType = TAB_VOID
 IsArray = True
 if FieldName and not IsArray:
@@ -1116,7 +1132,7 @@ class DscBuildData(PlatformBuildClassObject):
 EdkLogger.error('Parser', FORMAT_INVALID, 'PCD [%s.%s] Value 
"%s",  %s' %
 (TokenSpaceGuidCName, TokenCName, PcdValue, 
Value))
 elif PcdValue.startswith("L'") or PcdValue.startswith("'"):
-if FieldName and IsFieldValueAnArray(PcdValue):
+if FieldName and _IsFieldValueAnArray(PcdValue):
 PcdDatumType = TAB_VOID
 IsArray = True
 if FieldName and not IsArray:
@@ -1128,7 +1144,7 @@ class DscBuildData(PlatformBuildClassObject):
 (TokenSpaceGuidCName, TokenCName, PcdValue, 
Value))
 elif PcdValue.startswith('L'):
 PcdValue = 'L"' + PcdValue[1:] + '"'
-if FieldName and IsFieldValueAnArray(PcdValue):
+if FieldName and _IsFieldValueAnArray(PcdValue):
 PcdDatumType = TAB_VOID
 

[edk2] [Patch v1 5/5] BaseTools/GenFds/Capsule: move function logic

[Jaben Carsey]

Move PackRegistryFormatGuid logic from Common.Misc to this file.
There were no other consumers of the function.
As it is one line, just replace the logic without the separate function.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/Misc.py| 6 --
 BaseTools/Source/Python/GenFds/Capsule.py | 4 ++--
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 5968a3de4e1f..a1bfc502477c 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -1906,12 +1906,6 @@ class SkuClass():
 else:
 return 'DEFAULT'
 
-#
-# Pack a registry format GUID
-#
-def PackRegistryFormatGuid(Guid):
-return PackGUID(Guid.split('-'))
-
 ##  Get the integer value from string like "14U" or integer like 2
 #
 #   @param  Input   The object that may be either a integer value or a 
string
diff --git a/BaseTools/Source/Python/GenFds/Capsule.py 
b/BaseTools/Source/Python/GenFds/Capsule.py
index df29c40dbd20..bc1d124bdddc 100644
--- a/BaseTools/Source/Python/GenFds/Capsule.py
+++ b/BaseTools/Source/Python/GenFds/Capsule.py
@@ -20,7 +20,7 @@ from .GenFdsGlobalVariable import GenFdsGlobalVariable, 
FindExtendTool
 from CommonDataClass.FdfClass import CapsuleClassObject
 import Common.LongFilePathOs as os
 from io import BytesIO
-from Common.Misc import SaveFileOnChange, PackRegistryFormatGuid
+from Common.Misc import SaveFileOnChange
 import uuid
 from struct import pack
 from Common import EdkLogger
@@ -66,7 +66,7 @@ class Capsule (CapsuleClassObject):
 #
 # Use FMP capsule GUID: 6DCBD5ED-E82D-4C44-BDA1-7194199AD92A
 #
-
Header.write(PackRegistryFormatGuid('6DCBD5ED-E82D-4C44-BDA1-7194199AD92A'))
+
Header.write(PackGUID('6DCBD5ED-E82D-4C44-BDA1-7194199AD92A'.split('-')))
 HdrSize = 0
 if 'CAPSULE_HEADER_SIZE' in self.TokensDict:
 Header.write(pack('=I', 
int(self.TokensDict['CAPSULE_HEADER_SIZE'], 16)))
-- 
2.16.2.windows.1

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

[edk2] [PATCH] NetworkPkg: Protocol Uninstallation Cleanup

[Ashish Singhal]

Use UEFILib provided protocol uninstallation abstraction
instead of direct API for a proper cleanup.

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

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 NetworkPkg/DnsDxe/DnsDriver.c | 30 ++
 NetworkPkg/HttpBootDxe/HttpBootDxe.c  | 15 +--
 NetworkPkg/HttpDxe/HttpDriver.c   | 15 +--
 NetworkPkg/IpSecDxe/IpSecDriver.c | 15 +--
 NetworkPkg/TcpDxe/TcpDriver.c | 15 +--
 NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 15 +--
 6 files changed, 35 insertions(+), 70 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsDriver.c b/NetworkPkg/DnsDxe/DnsDriver.c
index 1f9b924..b74f5ba 100644
--- a/NetworkPkg/DnsDxe/DnsDriver.c
+++ b/NetworkPkg/DnsDxe/DnsDriver.c
@@ -510,28 +510,18 @@ DnsDriverEntryPoint (
 FreePool (mDriverData);
 
   Error2:
- gBS->UninstallMultipleProtocolInterfaces (
-   gDns6DriverBinding.DriverBindingHandle,
-   ,
-   ,
-   ,
-   ,
-   ,
-   ,
-   NULL
-   );
+ EfiLibUninstallDriverBindingComponentName2 (
+   ,
+   ,
+   
+   );
 
   Error1:
-gBS->UninstallMultipleProtocolInterfaces (
-   ImageHandle,
-   ,
-   ,
-   ,
-   ,
-   ,
-   ,
-   NULL
-   );
+EfiLibUninstallDriverBindingComponentName2 (
+  ,
+  ,
+  
+  );
 
   return Status;
 }
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c 
b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
index 7ec06f960..0b16f95 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
@@ -1327,16 +1327,11 @@ HttpBootDxeDriverEntryPoint (
  
  );
   if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces(
-   ImageHandle,
-   ,
-   ,
-   ,
-   ,
-   ,
-   ,
-   NULL
-   );
+EfiLibUninstallDriverBindingComponentName2(
+  ,
+  ,
+  
+  );
   }
   return Status;
 }
diff --git a/NetworkPkg/HttpDxe/HttpDriver.c b/NetworkPkg/HttpDxe/HttpDriver.c
index 8df984d..979d76d 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.c
+++ b/NetworkPkg/HttpDxe/HttpDriver.c
@@ -230,16 +230,11 @@ HttpDxeDriverEntryPoint (
  
  );
   if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces (
-   ImageHandle,
-   ,
-   ,
-   ,
-   ,
-   ,
-   ,
-   NULL
-   );
+EfiLibUninstallDriverBindingComponentName2 (
+  ,
+  ,
+  
+  );
   }
   return Status;
 }
diff --git a/NetworkPkg/IpSecDxe/IpSecDriver.c 
b/NetworkPkg/IpSecDxe/IpSecDriver.c
index f66f89a..3082d99 100644
--- a/NetworkPkg/IpSecDxe/IpSecDriver.c
+++ b/NetworkPkg/IpSecDxe/IpSecDriver.c
@@ -631,16 +631,11 @@ IpSecDriverEntryPoint (
   return Status;
 
 ON_UNINSTALL_IPSEC4_DB:
-  gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+  EfiLibUninstallDriverBindingComponentName2 (
+,
+,
+
+);
 
 ON_UNINSTALL_IPSEC:
   gBS->UninstallProtocolInterface (
diff --git a/NetworkPkg/TcpDxe/TcpDriver.c b/NetworkPkg/TcpDxe/TcpDriver.c
index 2d4b16c..00d172b 100644
--- a/NetworkPkg/TcpDxe/TcpDriver.c
+++ b/NetworkPkg/TcpDxe/TcpDriver.c
@@ -202,16 +202,11 @@ TcpDriverEntryPoint (
  
  );
   if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces (
-   ImageHandle,
-   ,
-   ,
-   ,
-   ,
-   ,
-   ,
-   NULL
-   );
+EfiLibUninstallDriverBindingComponentName2 (
+  ,
+  ,
+  
+  );
 return Status;
   }
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
index 0ab640b..f25c27a 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
@@ -1269,16 +1269,11 @@ PxeBcDriverEntryPoint (
  
  );
   if (EFI_ERROR (Status)) {
-gBS->UninstallMultipleProtocolInterfaces (
-   ImageHandle,
-   ,
-   ,
-   ,
-   ,
-   ,
-   ,
-   NULL
-   );
+EfiLibUninstallDriverBindingComponentName2 (
+  ,
+  ,
+  
+  );
   }
 
   return Status;
-- 
2.7.4

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

[edk2] [Patch v1 4/5] BaseTools/AutoGen: move functions

[Jaben Carsey]

Move SplitOption and ConvertStringToByteArray from Common.Misc to this file.
There were no other consumers of the functions.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 77 ++--
 BaseTools/Source/Python/Common/Misc.py | 68 -
 2 files changed, 72 insertions(+), 73 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index acd34692b6c2..f68166403edf 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -30,7 +30,7 @@ from io import BytesIO
 
 from .StrGather import *
 from .BuildEngine import BuildRule
-
+import shutil
 from Common.LongFilePathSupport import CopyLongFilePath
 from Common.BuildToolError import *
 from Common.DataType import *
@@ -170,6 +170,73 @@ ${tail_comments}
 ## @AsBuilt${BEGIN}
 ##   ${flags_item}${END}
 """)
+## Split command line option string to list
+#
+# subprocess.Popen needs the args to be a sequence. Otherwise there's problem
+# in non-windows platform to launch command
+#
+def _SplitOption(OptionString):
+OptionList = []
+LastChar = " "
+OptionStart = 0
+QuotationMark = ""
+for Index in range(0, len(OptionString)):
+CurrentChar = OptionString[Index]
+if CurrentChar in ['"', "'"]:
+if QuotationMark == CurrentChar:
+QuotationMark = ""
+elif QuotationMark == "":
+QuotationMark = CurrentChar
+continue
+elif QuotationMark:
+continue
+
+if CurrentChar in ["/", "-"] and LastChar in [" ", "\t", "\r", "\n"]:
+if Index > OptionStart:
+OptionList.append(OptionString[OptionStart:Index - 1])
+OptionStart = Index
+LastChar = CurrentChar
+OptionList.append(OptionString[OptionStart:])
+return OptionList
+
+#
+# Convert string to C format array
+#
+def _ConvertStringToByteArray(Value):
+Value = Value.strip()
+if not Value:
+return None
+if Value[0] == '{':
+if not Value.endswith('}'):
+return None
+Value = Value.replace(' ', '').replace('{', '').replace('}', '')
+ValFields = Value.split(',')
+try:
+for Index in range(len(ValFields)):
+ValFields[Index] = str(int(ValFields[Index], 0))
+except ValueError:
+return None
+Value = '{' + ','.join(ValFields) + '}'
+return Value
+
+Unicode = False
+if Value.startswith('L"'):
+if not Value.endswith('"'):
+return None
+Value = Value[1:]
+Unicode = True
+elif not Value.startswith('"') or not Value.endswith('"'):
+return None
+
+Value = eval(Value) # translate escape character
+NewValue = '{'
+for Index in range(0, len(Value)):
+if Unicode:
+NewValue = NewValue + str(ord(Value[Index]) % 0x1) + ','
+else:
+NewValue = NewValue + str(ord(Value[Index]) % 0x100) + ','
+Value = NewValue + '0}'
+return Value
 
 ## Base class for AutoGen
 #
@@ -1719,11 +1786,11 @@ class PlatformAutoGen(AutoGen):
 def BuildCommand(self):
 RetVal = []
 if "MAKE" in self.ToolDefinition and "PATH" in 
self.ToolDefinition["MAKE"]:
-RetVal += SplitOption(self.ToolDefinition["MAKE"]["PATH"])
+RetVal += _SplitOption(self.ToolDefinition["MAKE"]["PATH"])
 if "FLAGS" in self.ToolDefinition["MAKE"]:
 NewOption = self.ToolDefinition["MAKE"]["FLAGS"].strip()
 if NewOption != '':
-RetVal += SplitOption(NewOption)
+RetVal += _SplitOption(NewOption)
 if "MAKE" in self.EdkIIBuildOption:
 if "FLAGS" in self.EdkIIBuildOption["MAKE"]:
 Flags = self.EdkIIBuildOption["MAKE"]["FLAGS"]
@@ -3425,7 +3492,7 @@ class ModuleAutoGen(AutoGen):
 Guid = gEfiVarStoreGuidPattern.search(Content, Pos)
 if not Guid:
 break
-NameArray = ConvertStringToByteArray('L"' + Name.group(1) + 
'"')
+NameArray = _ConvertStringToByteArray('L"' + Name.group(1) + 
'"')
 NameGuids.add((NameArray, 
GuidStructureStringToGuidString(Guid.group(1
 Pos = Content.find('efivarstore', Name.end())
 if not NameGuids:
@@ -3438,7 +3505,7 @@ class ModuleAutoGen(AutoGen):
 Value = GuidValue(SkuInfo.VariableGuid, 
self.PlatformInfo.PackageList, self.MetaFile.Path)
 if not Value:
 continue
-Name = ConvertStringToByteArray(SkuInfo.VariableName)
+Name = _ConvertStringToByteArray(SkuInfo.VariableName)
 Guid = GuidStructureStringToGuidString(Value)
   

[edk2] [Patch v1 1/5] BaseTools/build/build: refactor and move functions

[Jaben Carsey]

Move DataDump and DataRestore from Common.Misc to this file.
There were no other consumers of these 2 functions.

Import threading since that module is used in build.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/Misc.py | 37 
 BaseTools/Source/Python/build/build.py | 46 ++--
 2 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 76a73d1c33db..43e016febcbb 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -487,43 +487,6 @@ def SaveFileOnChange(File, Content, IsBinaryFile=True):
 
 return True
 
-## Make a Python object persistent on file system
-#
-#   @param  DataThe object to be stored in file
-#   @param  FileThe path of file to store the object
-#
-def DataDump(Data, File):
-Fd = None
-try:
-Fd = open(File, 'wb')
-pickle.dump(Data, Fd, pickle.HIGHEST_PROTOCOL)
-except:
-EdkLogger.error("", FILE_OPEN_FAILURE, ExtraData=File, 
RaiseError=False)
-finally:
-if Fd is not None:
-Fd.close()
-
-## Restore a Python object from a file
-#
-#   @param  FileThe path of file stored the object
-#
-#   @retval object  A python object
-#   @retval NoneIf failure in file operation
-#
-def DataRestore(File):
-Data = None
-Fd = None
-try:
-Fd = open(File, 'rb')
-Data = pickle.load(Fd)
-except Exception as e:
-EdkLogger.verbose("Failed to load [%s]\n\t%s" % (File, str(e)))
-Data = None
-finally:
-if Fd is not None:
-Fd.close()
-return Data
-
 ## Retrieve and cache the real path name in file system
 #
 #   @param  RootThe root directory of path relative to
diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index b992a4c1b303..85b36cacd00c 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -32,6 +32,7 @@ import multiprocessing
 
 from struct import *
 from threading import *
+import threading
 from optparse import OptionParser
 from subprocess import *
 from Common import Misc as Utils
@@ -71,6 +72,43 @@ gToolsDefinition = "tools_def.txt"
 TemporaryTablePattern = re.compile(r'^_\d+_\d+_[a-fA-F0-9]+$')
 TmpTableDict = {}
 
+## Make a Python object persistent on file system
+#
+#   @param  DataThe object to be stored in file
+#   @param  FileThe path of file to store the object
+#
+def _DataDump(Data, File):
+Fd = None
+try:
+Fd = open(File, 'wb')
+pickle.dump(Data, Fd, pickle.HIGHEST_PROTOCOL)
+except:
+EdkLogger.error("", FILE_OPEN_FAILURE, ExtraData=File, 
RaiseError=False)
+finally:
+if Fd is not None:
+Fd.close()
+
+## Restore a Python object from a file
+#
+#   @param  FileThe path of file stored the object
+#
+#   @retval object  A python object
+#   @retval NoneIf failure in file operation
+#
+def _DataRestore(File):
+Data = None
+Fd = None
+try:
+Fd = open(File, 'rb')
+Data = pickle.load(Fd)
+except Exception as e:
+EdkLogger.verbose("Failed to load [%s]\n\t%s" % (File, str(e)))
+Data = None
+finally:
+if Fd is not None:
+Fd.close()
+return Data
+
 ## Check environment PATH variable to make sure the specified tool is found
 #
 #   If the tool is found in the PATH, then True is returned
@@ -2242,19 +2280,19 @@ class Build():
 def DumpBuildData(self):
 CacheDirectory = os.path.dirname(GlobalData.gDatabasePath)
 Utils.CreateDirectory(CacheDirectory)
-Utils.DataDump(Utils.gFileTimeStampCache, os.path.join(CacheDirectory, 
"gFileTimeStampCache"))
-Utils.DataDump(Utils.gDependencyDatabase, os.path.join(CacheDirectory, 
"gDependencyDatabase"))
+Utils._DataDump(Utils.gFileTimeStampCache, 
os.path.join(CacheDirectory, "gFileTimeStampCache"))
+Utils._DataDump(Utils.gDependencyDatabase, 
os.path.join(CacheDirectory, "gDependencyDatabase"))
 
 def RestoreBuildData(self):
 FilePath = os.path.join(os.path.dirname(GlobalData.gDatabasePath), 
"gFileTimeStampCache")
 if Utils.gFileTimeStampCache == {} and os.path.isfile(FilePath):
-Utils.gFileTimeStampCache = Utils.DataRestore(FilePath)
+Utils.gFileTimeStampCache = Utils._DataRestore(FilePath)
 if Utils.gFileTimeStampCache is None:
 Utils.gFileTimeStampCache = {}
 
 FilePath = os.path.join(os.path.dirname(GlobalData.gDatabasePath), 
"gDependencyDatabase")
 if Utils.gDependencyDatabase == {} and os.path.isfile(FilePath):
-Utils.gDependencyDatabase = Utils.DataRestore(FilePath)
+Utils.gDependencyDatabase 

[edk2] [Patch v1 0/5] cleanup shared functions

[Jaben Carsey]

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

Many shared functions are not used by more than one consumer. 
Move them local to consumer until the use arises for sharing.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 

Jaben Carsey (5):
  BaseTools/build/build: refactor and move functions
  BaseTools/Workspace/InfBuildData: move functions
  BaseTools/DscBuildData: move function
  BaseTools/AutoGen: move functions
  BaseTools/GenFds/Capsule: move function logic

 BaseTools/Source/Python/AutoGen/AutoGen.py|  77 -
 BaseTools/Source/Python/Common/Misc.py| 167 +---
 BaseTools/Source/Python/GenFds/Capsule.py |   4 +-
 BaseTools/Source/Python/Workspace/DscBuildData.py |  50 --
 BaseTools/Source/Python/Workspace/InfBuildData.py |  46 +-
 BaseTools/Source/Python/build/build.py|  46 +-
 6 files changed, 192 insertions(+), 198 deletions(-)

-- 
2.16.2.windows.1

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

[edk2] [Patch v1 2/5] BaseTools/Workspace/InfBuildData: move functions

[Jaben Carsey]

Move ProtocolValue and PpiValue from Common.Misc to this file.
There were no other consumers of these 2 functions.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/Misc.py| 38 
 BaseTools/Source/Python/Workspace/InfBuildData.py | 46 ++--
 2 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 43e016febcbb..25d8f9807fa3 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -611,44 +611,6 @@ def GuidValue(CName, PackageList, Inffile = None):
 return P.Guids[CName]
 return None
 
-## Get Protocol value from given packages
-#
-#   @param  CName   The CName of the GUID
-#   @param  PackageList List of packages looking-up in
-#   @param  Inffile The driver file
-#
-#   @retval GuidValue   if the CName is found in any given package
-#   @retval Noneif the CName is not found in all given packages
-#
-def ProtocolValue(CName, PackageList, Inffile = None):
-for P in PackageList:
-ProtocolKeys = P.Protocols.keys()
-if Inffile and P._PrivateProtocols:
-if not Inffile.startswith(P.MetaFile.Dir):
-ProtocolKeys = [x for x in P.Protocols if x not in 
P._PrivateProtocols]
-if CName in ProtocolKeys:
-return P.Protocols[CName]
-return None
-
-## Get PPI value from given packages
-#
-#   @param  CName   The CName of the GUID
-#   @param  PackageList List of packages looking-up in
-#   @param  Inffile The driver file
-#
-#   @retval GuidValue   if the CName is found in any given package
-#   @retval Noneif the CName is not found in all given packages
-#
-def PpiValue(CName, PackageList, Inffile = None):
-for P in PackageList:
-PpiKeys = P.Ppis.keys()
-if Inffile and P._PrivatePpis:
-if not Inffile.startswith(P.MetaFile.Dir):
-PpiKeys = [x for x in P.Ppis if x not in P._PrivatePpis]
-if CName in PpiKeys:
-return P.Ppis[CName]
-return None
-
 ## A string template class
 #
 #  This class implements a template for string replacement. A string template
diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py 
b/BaseTools/Source/Python/Workspace/InfBuildData.py
index 99bbecfd1f87..351f596d76b4 100644
--- a/BaseTools/Source/Python/Workspace/InfBuildData.py
+++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
@@ -21,6 +21,44 @@ from .MetaFileParser import *
 from collections import OrderedDict
 from Workspace.BuildClassObject import ModuleBuildClassObject, 
LibraryClassObject, PcdClassObject
 
+## Get Protocol value from given packages
+#
+#   @param  CName   The CName of the GUID
+#   @param  PackageList List of packages looking-up in
+#   @param  Inffile The driver file
+#
+#   @retval GuidValue   if the CName is found in any given package
+#   @retval Noneif the CName is not found in all given packages
+#
+def _ProtocolValue(CName, PackageList, Inffile = None):
+for P in PackageList:
+ProtocolKeys = P.Protocols.keys()
+if Inffile and P._PrivateProtocols:
+if not Inffile.startswith(P.MetaFile.Dir):
+ProtocolKeys = [x for x in P.Protocols if x not in 
P._PrivateProtocols]
+if CName in ProtocolKeys:
+return P.Protocols[CName]
+return None
+
+## Get PPI value from given packages
+#
+#   @param  CName   The CName of the GUID
+#   @param  PackageList List of packages looking-up in
+#   @param  Inffile The driver file
+#
+#   @retval GuidValue   if the CName is found in any given package
+#   @retval Noneif the CName is not found in all given packages
+#
+def _PpiValue(CName, PackageList, Inffile = None):
+for P in PackageList:
+PpiKeys = P.Ppis.keys()
+if Inffile and P._PrivatePpis:
+if not Inffile.startswith(P.MetaFile.Dir):
+PpiKeys = [x for x in P.Ppis if x not in P._PrivatePpis]
+if CName in PpiKeys:
+return P.Ppis[CName]
+return None
+
 ## Module build information from INF file
 #
 #  This class is used to retrieve information stored in database and convert 
them
@@ -645,7 +683,7 @@ class InfBuildData(ModuleBuildClassObject):
 RecordList = self._RawData[MODEL_EFI_PROTOCOL, self._Arch, 
self._Platform]
 for Record in RecordList:
 CName = Record[0]
-Value = ProtocolValue(CName, self.Packages, self.MetaFile.Path)
+Value = _ProtocolValue(CName, self.Packages, self.MetaFile.Path)
 if Value is None:
 PackageList = "\n\t".join(str(P) for P in self.Packages)
 

[edk2] about 'sr.ht' [was: Research Request]

[Laszlo Ersek]

On 11/14/18 19:34, stephano wrote:
> We are currently researching several different options to help make
> contributing to TianoCore easier for the community. A big part of this
> effort will be enabling pull requests and allowing for a more
> customizable code review process.
> 
> I am looking for members of the community willing to answer a few
> questions about these solutions to allow us to evaluate our options
> quickly. The options are:
> 
> System/Tool    Investigator
> Phabricator    Rebecca Cran (thank you again :) )
> Github    ???
> Gerrit    ???
> Gitlab    ???
> 
> I have a list of questions that I can send out to each investigator.
> Assuming you are familiar with the software/system, these questions
> should be answerable with a couple hours of research, writing, and
> screenshots / examples.

I'm not making a real proposal for "sr.ht" at this time, just raising it
as one collaboration software ("forge") that seems to get its goals
exactly right (in my opinion anyway):

  https://lwn.net/SubscriberLink/775963/e9849144cef5c99d/
  https://drewdevault.com/2018/11/15/sr.ht-general-availability.html
  https://lwn.net/Articles/776296/

It is admittedly alpha at this point, hence likely unsuitable for
production use. If it were a mature project, it would be extremely
attractive to me. I hope I can keep an eye on it over time.

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

Re: [edk2] Conditional Compilation support in INF file

[KarunakarPoosapalli]

Hi All,

Thank you very much for your valuable thoughts.
Below are my concerns/thoughts, Could you please help on

1. Is there any module or INF already using FeatureFlagExpression feature 
support in current edk2 source?
2. If not, could you please help in providing more detailed spec/Doc to verify 
this support.
3. Below are the few of use cases we're looking for, Did really 
FeatureFlagExpression support all of this?
a.  If we've this support we can add different protocols in 
DEPEX section, like we can put condition check and add rotococol1 for Notebook 
and Protocol2 for Desktop.
b.   Help in simplifying build files. We could dispense 
with the prefix to inf files and more developer convenient. 
c.  Sometimes we would like to add Conditional  .C and .H 
for file inclusion. Helps in our Debug / release builds as well..
Example; we do have release and debug build, 
although Source Section has - IA32,X64, Common, IPF, EBC exclusively. 
  But for the same Source section, we 
can't have choices based on our build inputs...

#if Condition1
  [Sources]
Main.c
#elif Condition2
 [Sources]
DebugMain.c

4. I don't think INF file support MACRO support, How complex in modifying the 
BaseTools to support Condition checks and MACRO support?

Thanks & Regards,
Karunakar

-Original Message-
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com] 
Sent: Thursday, January 10, 2019 10:23 PM
To: Gao, Liming; Laszlo Ersek; Poosapalli, Karunakar; edk2-devel@lists.01.org; 
Kinney, Michael D
Cc: Vidyadhara, Sumanth; Gao, Liming; Raju, SriramKumar
Subject: RE: [edk2] Conditional Compilation support in INF file


[EXTERNAL EMAIL] 

There is one additional constraint for adding conditionals to INF files.

The UEFI Distribution Packaging Specification defines a format to share package 
and module content in a standard format and uses an XML schema for the 
metadata.  We need to be able to convert between INF <--> XML and DEC <--> XML.

http://www.uefi.org/sites/default/files/resources/Dist_Package_Spec_1_1.pdf

If we define extensions to INF or DEC files, we need to make sure these 
transforms are still supported.  If an extension prevents these transforms, 
then we either need to change the extension to be compatible or work on an 
update to the UDP spec to support the extension in the XML.

Best regards,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Gao, Liming
> Sent: Thursday, January 10, 2019 7:48 AM
> To: Laszlo Ersek ;
> karunakarpoosapa...@dell.com; edk2-devel@lists.01.org
> Cc: sumanth.vidyadh...@dell.com; Gao, Liming ; 
> sriramkumar.r...@dell.com
> Subject: Re: [edk2] Conditional Compilation support in INF file
> 
> I have same question. What flexibility is expected in INF? I see one 
> request in [Depex] section. So, PCD support in [Depex] is added.
> 
> Edk2 INF is used to describe the source code behavior.
> If the source uses Ppi/Protocol/Guid/Pcd, these information are always 
> required to be described in INF file. The compiler can optimize the 
> code and remove the unused Ppi/Protocol/Guid/Pcd. It doesn't need 
> developer specify the conditional statement.
> 
> Thanks
> Liming
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Thursday, January 10, 2019 8:54 PM
> > To: karunakarpoosapa...@dell.com; Gao, Liming
> ; edk2-devel@lists.01.org
> > Cc: sumanth.vidyadh...@dell.com;
> sriramkumar.r...@dell.com
> > Subject: Re: [edk2] Conditional Compilation support in
> INF file
> >
> > On 01/10/19 07:03, karunakarpoosapa...@dell.com wrote:
> > > Hi All,
> > >
> > > I agree with providing the support like
> "FixedAtBuild PCD in INF". And we need to modify or provide support in 
> BaseTools to support
> > this feature.
> > >
> > > There are more use cases or flexibility to developer
> if we support Conditional compilation support in INF.
> > > As we're providing support in BaseTools for
> FixedAtBuild PCD support in INF, Is there any challenges or drawbacks 
> in  providing
> > conditional compilation support in INF?
> >
> > This is not for me to say authoritatively, but I'm
> unaware of any
> > specific use case that cannot be solved without this
> feature addition,
> > and any further complexity to BaseTools should be
> strongly justified.
> > "More convenient" is too vague for me, and the
> BaseTools code is already
> > hard to read and debug.
> >
> > That's just my opinion, again.
> >
> > Thanks
> > Laszlo
> 

[edk2] [Patch 1/2] UefiCpuPkg/S3Resume2Pei: Use correct field name.

[Eric Dong]

((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0))

In above code, Facs->OspmFlags should be used instead.
EFI_ACPI_4_0_OSPM_64BIT_WAKE__F is a bit in OSPM Enabled Firmware
Control Structure Flags field, not in Firmware Control Structure
Feature Flags.

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

Cc: Aleksiy 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
signed-off-by: Eric Dong 
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c 
b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index 05234a6312..c85dfc0290 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -4,7 +4,7 @@
   This module will execute the boot script saved during last boot and after 
that,
   control is passed to OS waking up handler.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
   Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
   This program and the accompanying materials
@@ -316,7 +316,7 @@ IsLongModeWakingVector (
   if (Facs->XFirmwareWakingVector != 0) {
 if ((Facs->Version == 
EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) &&
 ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) != 0) &&
-((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
+((Facs->OspmFlags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
   // Both BIOS and OS wants 64bit vector
   if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
 return TRUE;
@@ -499,7 +499,7 @@ S3ResumeBootOs (
   ));
 if ((Facs->Version == 
EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) &&
 ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) != 0) &&
-((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
+((Facs->OspmFlags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
   //
   // X64 long mode waking vector
   //
-- 
2.15.0.windows.1

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

[edk2] [Patch 2/2] MdeModulePkg/BootScriptExecuteorDxe: Use correct field name.

[Eric Dong]

((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0))

In above code, Facs->OspmFlags should be used instead.
EFI_ACPI_4_0_OSPM_64BIT_WAKE__F is a bit in OSPM Enabled Firmware
Control Structure Flags field, not in Firmware Control Structure
Feature Flags.

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

Cc: Aleksiy 
Cc: Jian Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
signed-off-by: Eric Dong 
Signed-off-by: Eric Dong 
---
 MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c   | 4 ++--
 MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c 
b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
index e76abb7b7b..13af957a4d 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
+++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
@@ -4,7 +4,7 @@
   This driver is dispatched by Dxe core and the driver will reload itself to 
ACPI reserved memory
   in the entry point. The functionality is to interpret and restore the S3 
boot script
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 This program and the accompanying materials
@@ -156,7 +156,7 @@ S3BootScriptExecutorEntryFunction (
 TempStackTop = (UINTN) + sizeof(TempStack);
 if ((Facs->Version == 
EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) &&
 ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) != 0) &&
-((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
+((Facs->OspmFlags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
   //
   // X64 long mode waking vector
   //
diff --git 
a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c 
b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
index 1c6bb47b60..7b9627d579 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
+++ b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
@@ -3,7 +3,7 @@
 
   Set a IDT entry for interrupt vector 3 for debug purpose for x64 platform
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 
@@ -117,7 +117,7 @@ IsLongModeWakingVector (
   if (Facs->XFirmwareWakingVector != 0) {
 if ((Facs->Version == 
EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) &&
 ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) != 0) &&
-((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
+((Facs->OspmFlags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0)) {
   // Both BIOS and OS wants 64bit vector
   if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
 return TRUE;
-- 
2.15.0.windows.1

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

[edk2] [Patch 0/2] Use correct field name.

[Eric Dong]

((Facs->Flags & EFI_ACPI_4_0_OSPM_64BIT_WAKE__F) != 0))

In above code, Facs->OspmFlags should be used instead.
EFI_ACPI_4_0_OSPM_64BIT_WAKE__F is a bit in OSPM Enabled Firmware
Control Structure Flags field, not in Firmware Control Structure
Feature Flags.

Update all related code to use correct field name.

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

Cc: Aleksiy 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Jian Wang 
Eric Dong (2):
  UefiCpuPkg/S3Resume2Pei: Use correct field name.
  MdeModulePkg/BootScriptExecuteorDxe: Use correct field name.

 MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c   | 4 ++--
 MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c | 4 ++--
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c   | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

-- 
2.15.0.windows.1

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

Re: [edk2] A question about shell-application's argument make system blocked;

[Carsey, Jaben]

Agreed.  That seems like a bug that needs a Bugzilla filed.  

My question is if the comment character is valid on a command line typed in 
versus in a script file.

> -Original Message-
> From: jim.dai...@dell.com [mailto:jim.dai...@dell.com]
> Sent: Thursday, January 10, 2019 3:34 PM
> To: Carsey, Jaben 
> Cc: sssky...@163.com; edk2-devel@lists.01.org
> Subject: RE: [edk2] A question about shell-application's argument make
> system blocked;
> Importance: High
> 
> Jaben,
> 
> The shell does not parse properly (my opinion) in some instances.
> That is one of the reasons I wrote a separate parser for the shell I
> maintain here at Dell.
> 
> One of the areas I feel the parsing is wrong is when an unescaped "#"
> is inside a quoted string:
> 
>   FS0:\> echo "This should # work."
>   Command Error Status: Invalid Parameter
>   FS0:\> echo "This should ^# work."
>   This should # work.
>   FS0:\>
> 
> The first command is parsed as if this were the command line:
> 
>   FS0:\> echo "This should
>   Command Error Status: Invalid Parameter
>   FS0:\>
> 
> I think many people would expect certain characters inside a quoted
> string, like "#" for example, to NOT need escaping.  The only ones
> that should need escaping (again IMHO) are: ", ^, and %.
> 
> Regards,
> Jim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Carsey, Jaben
> Sent: Thursday, January 10, 2019 9:16 AM
> To: krishnaLee; edk2-devel@lists.01.org
> Subject: Re: [edk2] A question about shell-application's argument make
> system blocked;
> 
> 
> Is this in a script file?  I don't remember how "comments" work on raw
> command lines where the user types them.
> 
> -Jaben
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > krishnaLee
> > Sent: Wednesday, January 09, 2019 10:13 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] A question about shell-application's argument make system
> > blocked;
> > Importance: High
> >
> > Hi everybody,
> > I meet a question,a special arg can make system blocked,follow is my steps.
> > 1,go to uefi shell v2.2(uefi v2.70),run this application in QEMU-ovmf:
> > testapp.efi
> > 2,the output is "index:0,string:FS0:\testapp.efi"
> >
> >
> > 3,testapp.efi #abc.
> > 4,the output is same as step 2.  ///< I had read the uefi shell 
> > specification
> > 2.2,the '#' is a comment remark,so I think it is ok.
> >
> >
> > 5 testapp.efi "#abc"
> > 6,the system blocked(dead).  ///< I think it is a bug.
> >
> >
> > //follow is the testapp.efi source code:
> > EFI_STATUS
> > EFIAPI
> > UefiMain (
> > IN EFI_HANDLE ImageHandle,
> > IN EFI_SYSTEM_TABLE *SystemTable
> > )
> > {
> > EFI_STATUS status;
> > EFI_SHELL_PARAMETERS_PROTOCOL* param;
> > status=SystemTable->BootServices-
> >
> >HandleProtocol(ImageHandle,,);
> > if(status!=EFI_SUCCESS)
> > {
> > return0;
> > }
> >
> >
> > for(UINTN i=0;i< param->Argc;i++)
> > {
> > Print(L"index:%d,string:%s\n",i,param->Argv[i]);
> > }
> >
> >
> > return EFI_SUCCESS;
> > }
> >
> >
> > //test environment:
> > //QEMU v2.10.95 + edk2-2018-ovmf-x64.
> > //shell command line:
> > //"D:\qemu\qemu-system-x86_64.exe" -machine pc-q35-2.9 -pflash
> > "D:\qemu\bios\OVMF_x64_debug.fd" -serial stdio -hda fat:rw:G:\temp -
> net
> > none
> > //end
> >
> >
> >
> >
> >
> >
> > thanks,
> > krishna.
> >
> >
> >
> > ___
> > 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] [Patch] BaseTools: Report Error if use SET in Dsc

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Feng, Bob C
>Sent: Wednesday, January 02, 2019 4:44 PM
>To: edk2-devel@lists.01.org
>Cc: Feng, Bob C ; Gao, Liming 
>Subject: [Patch] BaseTools: Report Error if use SET in Dsc
>
>Build tool do not support SET syntax in DSC.
>
>If the SET statement is used in DSC, build tool just ignore it.
>
>That behavior confused some users that
>they think SET statement works in DSC like in FDF.
>
>To avoid such confusion, build tool report ERROR
>
>if there is "SET" statement in Dsc file.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Bob Feng 
>Cc: Liming Gao 
>---
> BaseTools/Source/Python/Workspace/MetaFileParser.py | 5 +
> 1 file changed, 5 insertions(+)
>
>diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py
>b/BaseTools/Source/Python/Workspace/MetaFileParser.py
>index 032220813b..19d8452a35 100644
>--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
>+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
>@@ -975,10 +975,15 @@ class DscParser(MetaFileParser):
> else:
> SectionType = self._SectionType
> self._ItemType = SectionType
>
> self._ValueList = ['', '', '']
>+# "SET pcd = pcd_expression" syntax is not supported in Dsc file.
>+if self._CurrentLine.upper().strip().startswith("SET "):
>+EdkLogger.error('Parser', FORMAT_INVALID, '''"SET pcd =
>pcd_expression" syntax is not support in Dsc file''',
>+ExtraData=self._CurrentLine,
>+File=self.MetaFile, Line=self._LineIndex + 1)
> self._SectionParser[SectionType](self)
> if self._ValueList is None:
> continue
> #
> # Model, Value1, Value2, Value3, Arch, ModuleType, 
> BelongsToItem=-1,
>BelongsToFile=-1,
>--
>2.19.1.windows.1

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

Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

[Zeng, Star]

On 2019/1/11 0:23, Ard Biesheuvel wrote:

On Thu, 10 Jan 2019 at 14:03, Laszlo Ersek  wrote:


On 01/10/19 08:59, Zeng, Star wrote:

On 2019/1/10 15:33, Ard Biesheuvel wrote:

On Thu, 10 Jan 2019 at 08:30, Zeng, Star  wrote:


Hi Ard,

Another minor feedback.

On 2019/1/10 14:47, Zeng, Star wrote:

Hi Ard,

Some minor feedback added inline.

On 2019/1/4 2:28, Ard Biesheuvel wrote:

Implement a new version of the fault tolerant write driver that can
be used in the context of a standalone MM implementation.

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

MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

| 70 +++

MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

| 90 
2 files changed, 160 insertions(+)


Please add it into MdeModulePkg.dsc for package build verification.



Hello Star,

Thanks for all the feedback. I will respond in more detail later.

However, to the point raised here: it is not possible to add these
drivers to MdeModulePkg.dsc unless we add a dummy implementation of
StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
do that?


Oh, good information.
To have full code building coverage for the package, personally I think
we can move StandaloneMmDriverEntryPoint library class and instance into
MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should
be generic enough.

I do not want to block this patch set because of this. So let's discuss
this in parallel as separated topic.

Mike, Liming, Laszlo, Jian and Hao,\
What's your opinion?


It should be possible to build all library instances in a central
Package (well, all Packages really), using the Package's DSC file. To my
understanding, libraries built like this are not expected to be used in
actual (shipped) drivers / applications, nor is their indiscriminate
distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull
library instance in binary form seems quite useless.

With that in mind, I think a Null instance for the entry point in
question makes sense, under MdeModulePkg.



I will look into this a bit deeper next week. I think it makes sense
for the core PI architected pieces to all live in MdePkg rather than
StandaloneMmPkg. For instance, MmServicesTableLib for standalone MM
should live there, MmEntryPoint should live there (and have
traditional and standalone MM implementation) and perhaps some other
core pieces as well.

This may be a slippery slope, so I will dedicate some time to look
into this carefully, at least with the goal to make the
FaultTolerantWrite and Variable driver buildable from within
MdeModulePkg.


Make sense to me. You'd better to submit a bugzilla to track this after 
this patchset is pushed.


Thanks,
Star






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

[edk2] [Patch] BaseTools: Enable component override functionality

[BobCF]

https://bugzilla.tianocore.org/show_bug.cgi?id=1449
This patch enable build tools to recognize that
when two given files have the same GUID, file path and ARCH in Dsc,
The later one's definition will be used.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
Cc: Carsey Jaben 
---
 .../Source/Python/Workspace/DscBuildData.py   | 24 ---
 .../Source/Python/Workspace/MetaFileParser.py |  5 
 .../Source/Python/Workspace/MetaFileTable.py  |  7 --
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 7e82e8e934..f9805f58f5 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -704,36 +704,44 @@ class DscBuildData(PlatformBuildClassObject):
 if TAB_DEFAULT_STORES_DEFAULT not in self.DefaultStores:
 self.DefaultStores[TAB_DEFAULT_STORES_DEFAULT] = (0, 
TAB_DEFAULT_STORES_DEFAULT)
 GlobalData.gDefaultStores = sorted(self.DefaultStores.keys())
 return self.DefaultStores
 
+def OverrideDuplicateModule(self):
+RecordList = self._RawData[MODEL_META_DATA_COMPONENT, self._Arch]
+Macros = self._Macros
+Macros["EDK_SOURCE"] = GlobalData.gEcpSource
+Components = {}
+for Record in RecordList:
+ModuleId = Record[6]
+file_guid = self._RawData[MODEL_META_DATA_HEADER, self._Arch, 
None, ModuleId]
+file_guid_str = file_guid[0][2] if file_guid else "NULL"
+ModuleFile = PathClass(NormPath(Record[0], Macros), 
GlobalData.gWorkspace, Arch=self._Arch)
+if self._Arch != TAB_ARCH_COMMON and 
(file_guid_str,str(ModuleFile)) in Components:
+
self._RawData.DisableOverrideComponent(Components[(file_guid_str,str(ModuleFile))])
+Components[(file_guid_str,str(ModuleFile))] = ModuleId
+self._RawData._PostProcessed = False
 ## Retrieve [Components] section information
 @property
 def Modules(self):
 if self._Modules is not None:
 return self._Modules
-
+self.OverrideDuplicateModule()
 self._Modules = OrderedDict()
 RecordList = self._RawData[MODEL_META_DATA_COMPONENT, self._Arch]
 Macros = self._Macros
 Macros["EDK_SOURCE"] = GlobalData.gEcpSource
 for Record in RecordList:
-DuplicatedFile = False
-
 ModuleFile = PathClass(NormPath(Record[0], Macros), 
GlobalData.gWorkspace, Arch=self._Arch)
 ModuleId = Record[6]
 LineNo = Record[7]
 
 # check the file validation
 ErrorCode, ErrorInfo = ModuleFile.Validate('.inf')
 if ErrorCode != 0:
 EdkLogger.error('build', ErrorCode, File=self.MetaFile, 
Line=LineNo,
 ExtraData=ErrorInfo)
-# Check duplication
-# If arch is COMMON, no duplicate module is checked since all 
modules in all component sections are selected
-if self._Arch != TAB_ARCH_COMMON and ModuleFile in self._Modules:
-DuplicatedFile = True
 
 Module = ModuleBuildClassObject()
 Module.MetaFile = ModuleFile
 
 # get module private library instance
@@ -792,12 +800,10 @@ class DscBuildData(PlatformBuildClassObject):
 else:
 OptionString = Module.BuildOptions[ToolChainFamily, 
ToolChain]
 Module.BuildOptions[ToolChainFamily, ToolChain] = 
OptionString + " " + Option
 
 RecordList = self._RawData[MODEL_META_DATA_HEADER, self._Arch, 
None, ModuleId]
-if DuplicatedFile and not RecordList:
-EdkLogger.error('build', FILE_DUPLICATED, File=self.MetaFile, 
ExtraData=str(ModuleFile), Line=LineNo)
 if RecordList:
 if len(RecordList) != 1:
 EdkLogger.error('build', OPTION_UNKNOWN, 'Only FILE_GUID 
can be listed in  section.',
 File=self.MetaFile, 
ExtraData=str(ModuleFile), Line=LineNo)
 ModuleFile = ProcessDuplicatedInf(ModuleFile, 
RecordList[0][2], GlobalData.gWorkspace)
diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
b/BaseTools/Source/Python/Workspace/MetaFileParser.py
index 032220813b..a52e9229df 100644
--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
@@ -1711,10 +1711,15 @@ class DscParser(MetaFileParser):
 
 def __ProcessBuildOption(self):
 self._ValueList = [ReplaceMacro(Value, self._Macros, RaiseError=False)
for Value in self._ValueList]
 
+def DisableOverrideComponent(self,module_id):
+for ori_id in self._IdMapping:
+if self._IdMapping[ori_id] == module_id:
+

Re: [edk2] A question about shell-application's argument make system blocked;

[Jim.Dailey]

Jaben,

The shell does not parse properly (my opinion) in some instances.
That is one of the reasons I wrote a separate parser for the shell I
maintain here at Dell.

One of the areas I feel the parsing is wrong is when an unescaped "#"
is inside a quoted string:

  FS0:\> echo "This should # work."
  Command Error Status: Invalid Parameter
  FS0:\> echo "This should ^# work."
  This should # work.
  FS0:\>

The first command is parsed as if this were the command line:

  FS0:\> echo "This should 
  Command Error Status: Invalid Parameter
  FS0:\>

I think many people would expect certain characters inside a quoted
string, like "#" for example, to NOT need escaping.  The only ones
that should need escaping (again IMHO) are: ", ^, and %.

Regards,
Jim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Carsey, 
Jaben
Sent: Thursday, January 10, 2019 9:16 AM
To: krishnaLee; edk2-devel@lists.01.org
Subject: Re: [edk2] A question about shell-application's argument make system 
blocked; 


Is this in a script file?  I don't remember how "comments" work on raw command 
lines where the user types them.

-Jaben


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> krishnaLee
> Sent: Wednesday, January 09, 2019 10:13 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] A question about shell-application's argument make system
> blocked;
> Importance: High
> 
> Hi everybody,
> I meet a question,a special arg can make system blocked,follow is my steps.
> 1,go to uefi shell v2.2(uefi v2.70),run this application in QEMU-ovmf:
> testapp.efi
> 2,the output is "index:0,string:FS0:\testapp.efi"
> 
> 
> 3,testapp.efi #abc.
> 4,the output is same as step 2.  ///< I had read the uefi shell specification
> 2.2,the '#' is a comment remark,so I think it is ok.
> 
> 
> 5 testapp.efi "#abc"
> 6,the system blocked(dead).  ///< I think it is a bug.
> 
> 
> //follow is the testapp.efi source code:
> EFI_STATUS
> EFIAPI
> UefiMain (
> IN EFI_HANDLE ImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> EFI_STATUS status;
> EFI_SHELL_PARAMETERS_PROTOCOL* param;
> status=SystemTable->BootServices-
> >HandleProtocol(ImageHandle,,);
> if(status!=EFI_SUCCESS)
> {
> return0;
> }
> 
> 
> for(UINTN i=0;i< param->Argc;i++)
> {
> Print(L"index:%d,string:%s\n",i,param->Argv[i]);
> }
> 
> 
> return EFI_SUCCESS;
> }
> 
> 
> //test environment:
> //QEMU v2.10.95 + edk2-2018-ovmf-x64.
> //shell command line:
> //"D:\qemu\qemu-system-x86_64.exe" -machine pc-q35-2.9 -pflash
> "D:\qemu\bios\OVMF_x64_debug.fd" -serial stdio -hda fat:rw:G:\temp -net
> none
> //end
> 
> 
> 
> 
> 
> 
> thanks,
> krishna.
> 
> 
> 
> ___
> 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] [PATCH] NetworkPkg: Protocol Uninstallation Cleanup

[Wu, Jiaxin]

Looks good to me.

Reviewed-by: Wu Jiaxin 

Thanks,
Jiaxin

> -Original Message-
> From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
> Sent: Friday, January 11, 2019 3:27 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wu, Jiaxin ;
> Ashish Singhal 
> Subject: [PATCH] NetworkPkg: Protocol Uninstallation Cleanup
> 
> Use UEFILib provided protocol uninstallation abstraction
> instead of direct API for a proper cleanup.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1444
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  NetworkPkg/DnsDxe/DnsDriver.c | 30 ++
>  NetworkPkg/HttpBootDxe/HttpBootDxe.c  | 15 +--
>  NetworkPkg/HttpDxe/HttpDriver.c   | 15 +--
>  NetworkPkg/IpSecDxe/IpSecDriver.c | 15 +--
>  NetworkPkg/TcpDxe/TcpDriver.c | 15 +--
>  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 15 +--
>  6 files changed, 35 insertions(+), 70 deletions(-)
> 
> diff --git a/NetworkPkg/DnsDxe/DnsDriver.c
> b/NetworkPkg/DnsDxe/DnsDriver.c
> index 1f9b924..b74f5ba 100644
> --- a/NetworkPkg/DnsDxe/DnsDriver.c
> +++ b/NetworkPkg/DnsDxe/DnsDriver.c
> @@ -510,28 +510,18 @@ DnsDriverEntryPoint (
>  FreePool (mDriverData);
> 
>Error2:
> - gBS->UninstallMultipleProtocolInterfaces (
> -   gDns6DriverBinding.DriverBindingHandle,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   NULL
> -   );
> + EfiLibUninstallDriverBindingComponentName2 (
> +   ,
> +   ,
> +   
> +   );
> 
>Error1:
> -gBS->UninstallMultipleProtocolInterfaces (
> -   ImageHandle,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   NULL
> -   );
> +EfiLibUninstallDriverBindingComponentName2 (
> +  ,
> +  ,
> +  
> +  );
> 
>return Status;
>  }
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> index 7ec06f960..0b16f95 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
> @@ -1327,16 +1327,11 @@ HttpBootDxeDriverEntryPoint (
>   
>   );
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces(
> -   ImageHandle,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   NULL
> -   );
> +EfiLibUninstallDriverBindingComponentName2(
> +  ,
> +  ,
> +  
> +  );
>}
>return Status;
>  }
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.c
> b/NetworkPkg/HttpDxe/HttpDriver.c
> index 8df984d..979d76d 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.c
> +++ b/NetworkPkg/HttpDxe/HttpDriver.c
> @@ -230,16 +230,11 @@ HttpDxeDriverEntryPoint (
>   
>   );
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces (
> -   ImageHandle,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   NULL
> -   );
> +EfiLibUninstallDriverBindingComponentName2 (
> +  ,
> +  ,
> +  
> +  );
>}
>return Status;
>  }
> diff --git a/NetworkPkg/IpSecDxe/IpSecDriver.c
> b/NetworkPkg/IpSecDxe/IpSecDriver.c
> index f66f89a..3082d99 100644
> --- a/NetworkPkg/IpSecDxe/IpSecDriver.c
> +++ b/NetworkPkg/IpSecDxe/IpSecDriver.c
> @@ -631,16 +631,11 @@ IpSecDriverEntryPoint (
>return Status;
> 
>  ON_UNINSTALL_IPSEC4_DB:
> -  gBS->UninstallMultipleProtocolInterfaces (
> - ImageHandle,
> - ,
> - ,
> - ,
> - ,
> - ,
> - ,
> - NULL
> - );
> +  EfiLibUninstallDriverBindingComponentName2 (
> +,
> +,
> +
> +);
> 
>  ON_UNINSTALL_IPSEC:
>gBS->UninstallProtocolInterface (
> diff --git a/NetworkPkg/TcpDxe/TcpDriver.c
> b/NetworkPkg/TcpDxe/TcpDriver.c
> index 2d4b16c..00d172b 100644
> --- a/NetworkPkg/TcpDxe/TcpDriver.c
> +++ b/NetworkPkg/TcpDxe/TcpDriver.c
> @@ -202,16 +202,11 @@ TcpDriverEntryPoint (
>   
>   );
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces (
> -   ImageHandle,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   ,
> -   NULL
> -   );
> +EfiLibUninstallDriverBindingComponentName2 (
> +  ,
> +  ,
> +  
> +  );
>  return Status;
>}
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> index 0ab640b..f25c27a 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> @@ -1269,16 +1269,11 @@ PxeBcDriverEntryPoint (
>   
>   );
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces (

Re: [edk2] A question about shell-application's argument make system blocked;

[krishnaLee]

Jaben,
>My question is if the comment character is valid on a command line typed in 
>versus in a script file.
Maybe when parsing a script file,it should record a  in-script-file-status,
and this command [  testapp.efi  #abc ]  in command line(not in a script file) 
maybe looked as two argument (not one argument in current implementation).


thanks,
krishna.



At 2019-01-11 08:02:35, "Carsey, Jaben"  wrote:
>Agreed.  That seems like a bug that needs a Bugzilla filed.  
>
>My question is if the comment character is valid on a command line typed in 
>versus in a script file.
>
>> -Original Message-
>> From: jim.dai...@dell.com [mailto:jim.dai...@dell.com]
>> Sent: Thursday, January 10, 2019 3:34 PM
>> To: Carsey, Jaben 
>> Cc: sssky...@163.com; edk2-devel@lists.01.org
>> Subject: RE: [edk2] A question about shell-application's argument make
>> system blocked;
>> Importance: High
>> 
>> Jaben,
>> 
>> The shell does not parse properly (my opinion) in some instances.
>> That is one of the reasons I wrote a separate parser for the shell I
>> maintain here at Dell.
>> 
>> One of the areas I feel the parsing is wrong is when an unescaped "#"
>> is inside a quoted string:
>> 
>>   FS0:\> echo "This should # work."
>>   Command Error Status: Invalid Parameter
>>   FS0:\> echo "This should ^# work."
>>   This should # work.
>>   FS0:\>
>> 
>> The first command is parsed as if this were the command line:
>> 
>>   FS0:\> echo "This should
>>   Command Error Status: Invalid Parameter
>>   FS0:\>
>> 
>> I think many people would expect certain characters inside a quoted
>> string, like "#" for example, to NOT need escaping.  The only ones
>> that should need escaping (again IMHO) are: ", ^, and %.
>> 
>> Regards,
>> Jim
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Carsey, Jaben
>> Sent: Thursday, January 10, 2019 9:16 AM
>> To: krishnaLee; edk2-devel@lists.01.org
>> Subject: Re: [edk2] A question about shell-application's argument make
>> system blocked;
>> 
>> 
>> Is this in a script file?  I don't remember how "comments" work on raw
>> command lines where the user types them.
>> 
>> -Jaben
>> 
>> 
>> > -Original Message-
>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> > krishnaLee
>> > Sent: Wednesday, January 09, 2019 10:13 PM
>> > To: edk2-devel@lists.01.org
>> > Subject: [edk2] A question about shell-application's argument make system
>> > blocked;
>> > Importance: High
>> >
>> > Hi everybody,
>> > I meet a question,a special arg can make system blocked,follow is my steps.
>> > 1,go to uefi shell v2.2(uefi v2.70),run this application in QEMU-ovmf:
>> > testapp.efi
>> > 2,the output is "index:0,string:FS0:\testapp.efi"
>> >
>> >
>> > 3,testapp.efi #abc.
>> > 4,the output is same as step 2.  ///< I had read the uefi shell 
>> > specification
>> > 2.2,the '#' is a comment remark,so I think it is ok.
>> >
>> >
>> > 5 testapp.efi "#abc"
>> > 6,the system blocked(dead).  ///< I think it is a bug.
>> >
>> >
>> > //follow is the testapp.efi source code:
>> > EFI_STATUS
>> > EFIAPI
>> > UefiMain (
>> > IN EFI_HANDLE ImageHandle,
>> > IN EFI_SYSTEM_TABLE *SystemTable
>> > )
>> > {
>> > EFI_STATUS status;
>> > EFI_SHELL_PARAMETERS_PROTOCOL* param;
>> > status=SystemTable->BootServices-
>> >
>> >HandleProtocol(ImageHandle,,);
>> > if(status!=EFI_SUCCESS)
>> > {
>> > return0;
>> > }
>> >
>> >
>> > for(UINTN i=0;i< param->Argc;i++)
>> > {
>> > Print(L"index:%d,string:%s\n",i,param->Argv[i]);
>> > }
>> >
>> >
>> > return EFI_SUCCESS;
>> > }
>> >
>> >
>> > //test environment:
>> > //QEMU v2.10.95 + edk2-2018-ovmf-x64.
>> > //shell command line:
>> > //"D:\qemu\qemu-system-x86_64.exe" -machine pc-q35-2.9 -pflash
>> > "D:\qemu\bios\OVMF_x64_debug.fd" -serial stdio -hda fat:rw:G:\temp -
>> net
>> > none
>> > //end
>> >
>> >
>> >
>> >
>> >
>> >
>> > thanks,
>> > krishna.
>> >
>> >
>> >
>> > ___
>> > 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] [PATCH v2] ShellPkg/TftpDynamicCommand: Change file writing method in tftp

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Li, Songpeng
> Sent: Thursday, January 10, 2019 10:54 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ni, Ray ;
> Wu, Jiaxin 
> Subject: [PATCH v2] ShellPkg/TftpDynamicCommand: Change file writing
> method in tftp
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1433
> 
> v2: Remove an unused variable.
> 
> Current logic of shell tftp download was writing file after tftp
> download finished, when the file is large, it looks like the shell
> tftp command hanged after download was finished. To improve
> end-user experience, the solution is using split file writing
> instead.
> 
> This patch update the code to open and close file inside
> DownloadFile(), and save each packet to file within callback
> function CheckPacket().
> 
> Since AllocatePage() is no-longer needed, This patch can also
> remove the memory limitation. The download file can be larger
> than system free memory now.
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Cc: Wu Jiaxin 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Songpeng Li 
> ---
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c | 154 -
> ---
>  1 file changed, 64 insertions(+), 90 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> index ed081b5bad7c..ba753a279b00 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> @@ -41,6 +41,12 @@ STATIC CONST CHAR16 mTftpProgressFrame[] = L"[
>  // (TFTP_PROGRESS_MESSAGE_SIZE-1) '\b'
>  STATIC CONST CHAR16 mTftpProgressDelete[] =
> L"\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b
> \b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b";
> 
> +// Local File Handle
> +SHELL_FILE_HANDLE mFileHandle;
> +
> +// Path of the local file, Unicode encoded
> +CONST CHAR16 *mLocalFilePath;
> +
>  /**
>Check and convert the UINT16 option values of the 'tftp' command
> 
> @@ -166,9 +172,6 @@ GetFileSize (
>@param[in]   FileSize   Size of the file in number of bytes
>@param[in]   BlockSize  Value of the TFTP blksize option
>@param[in]   WindowSize Value of the TFTP window size 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.
> 
>@retval  EFI_SUCCESS   The file was downloaded.
>@retval  EFI_OUT_OF_RESOURCES  A memory allocation failed.
> @@ -184,8 +187,7 @@ DownloadFile (
>IN   CONST CHAR8  *AsciiFilePath,
>IN   UINTNFileSize,
>IN   UINT16   BlockSize,
> -  IN   UINT16   WindowSize,
> -  OUT  VOID **Data
> +  IN   UINT16   WindowSize
>);
> 
>  /**
> @@ -287,7 +289,6 @@ RunTftp (
>CHAR8   *AsciiRemoteFilePath;
>UINTN   FilePathSize;
>CONST CHAR16*Walker;
> -  CONST CHAR16*LocalFilePath;
>EFI_MTFTP4_CONFIG_DATA  Mtftp4ConfigData;
>EFI_HANDLE  *Handles;
>UINTN   HandleCount;
> @@ -297,9 +298,6 @@ RunTftp (
>EFI_HANDLE  Mtftp4ChildHandle;
>EFI_MTFTP4_PROTOCOL *Mtftp4;
>UINTN   FileSize;
> -  UINTN   DataSize;
> -  VOID*Data;
> -  SHELL_FILE_HANDLE   FileHandle;
>UINT16  BlockSize;
>UINT16  WindowSize;
> 
> @@ -309,7 +307,6 @@ RunTftp (
>AsciiRemoteFilePath = NULL;
>Handles = NULL;
>FileSize= 0;
> -  DataSize= 0;
>BlockSize   = MTFTP_DEFAULT_BLKSIZE;
>WindowSize  = MTFTP_DEFAULT_WINDOWSIZE;
> 
> @@ -385,7 +382,7 @@ RunTftp (
>UnicodeStrToAsciiStrS (RemoteFilePath, AsciiRemoteFilePath, FilePathSize);
> 
>if (ParamCount == 4) {
> -LocalFilePath = ShellCommandLineGetRawValue (CheckPackage, 3);
> +mLocalFilePath = ShellCommandLineGetRawValue (CheckPackage, 3);
>} else {
>  Walker = RemoteFilePath + StrLen (RemoteFilePath);
>  while ((--Walker) >= RemoteFilePath) {
> @@ -394,7 +391,7 @@ RunTftp (
>  break;
>}
>  }
> -LocalFilePath = Walker + 1;
> +mLocalFilePath = Walker + 1;
>}
> 
>//
> @@ -492,7 +489,6 @@ RunTftp (
> (NicNumber < HandleCount) && (ShellStatus != SHELL_SUCCESS);
> NicNumber++) {
>  ControllerHandle = Handles[NicNumber];
> -Data = NULL;
> 
>  Status = GetNicName (ControllerHandle, NicNumber, NicName);
>  if (EFI_ERROR (Status)) {
> @@ -543,7 +539,7 @@ RunTftp (
>goto NextHandle;
>  }
> 
> -Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, WindowSize, );
> +Status = DownloadFile (Mtftp4, RemoteFilePath,