Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

[Gao, Liming]

Andrew:
  I get your point. I agree edk2 firmware to support old EFI image. And, I 
review the change in BasePeCoffLib. But, I think it is unnecessary. Below is 
the current logic. On the first loop, DebugEntry.Type is 
EFI_IMAGE_DEBUG_TYPE_CODEVIEW, then will return. It doesn't enter into second 
loop. For mtoc debug entry, its DebugEntry.Type is also 
EFI_IMAGE_DEBUG_TYPE_CODEVIEW, its DEBUG_CODEVIEW_ ENTRY Signature is 
CODEVIEW_SIGNATURE_MTOC. So, it will be handled in this logic. 

for (Index = 0; Index < DebugDirectoryEntry->Size; Index += sizeof 
(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY)) {
  //
  // Read next debug directory entry
  //
  Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
  ...

  //
  // From PeCoff spec, when DebugEntry.RVA == 0 means this debug info 
will not load into memory.
  // Here we will always load EFI_IMAGE_DEBUG_TYPE_CODEVIEW type debug 
info. so need adjust the
  // ImageContext->ImageSize when DebugEntry.RVA == 0.
  //
  if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
ImageContext->DebugDirectoryEntryRva = (UINT32) 
(DebugDirectoryEntryRva + Index);
if (DebugEntry.RVA == 0 && DebugEntry.FileOffset != 0) {
  ImageContext->ImageSize += DebugEntry.SizeOfData;
}

return RETURN_SUCCESS;
  }
}

Thanks
Liming
>-Original Message-
>From: af...@apple.com [mailto:af...@apple.com]
>Sent: Friday, August 11, 2017 1:08 PM
>To: Gao, Liming 
>Cc: Kinney, Michael D ; edk2-
>de...@lists.01.org
>Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
>AppPkg with XCODE5
>
>
>> On Aug 10, 2017, at 9:48 PM, Gao, Liming  wrote:
>>
>> Andrew:
>>  Edk2 Build system calls GenFw to generate EFI image in build phase. Even if
>this image is not built into BIOS image, its EFI image will be generated by
>GenFw. So, only if this EFI image is built from EDK2 project, it can be updated
>by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi
>image.
>>
>
>Gao,
>
>We can fix the future for edk2, but we can't change the past, or change other
>build systems. It is not safe to assume that every EFI executable that the DXE
>Core will see is as new as the firmware, or was even built via the edk2. Option
>ROMs and OS loaders are good examples of this issue.
>
>Thanks,
>
>Andrew Fish
>
>> Thanks
>> Liming
>> From: af...@apple.com [mailto:af...@apple.com]
>> Sent: Friday, August 11, 2017 12:59 AM
>> To: Gao, Liming 
>> Cc: Zhu, Yonghong ; Kinney, Michael D
>; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build
>AppPkg with XCODE5
>>
>>
>> On Aug 10, 2017, at 3:38 AM, Gao, Liming
>> wrote:
>>
>> Andrew:
>> If this is a mtoc bug, I suggest to update GenFw to always correct it in the
>generated EFI image. If so, the EFI image is always correct. There is no change
>requirement in PeCoff library in MdePkg.
>>
>>
>> Liming,
>>
>> EFI supports loading PE/COFF images that are not built at the same time as
>the platform firmware (UEFI Shell, OS loader), and that is why I added the fix
>to the PE/COFF library code.
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>
>> Thanks
>> Liming
>> From: af...@apple.com [mailto:af...@apple.com]
>> Sent: Tuesday, August 8, 2017 12:26 AM
>> To: Zhu, Yonghong
>>
>> Cc: edk2-devel@lists.01.org; Gao, Liming
>>; Kinney, Michael D
>>
>> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build
>AppPkg with XCODE5
>>
>> Should that be:
>> Contributed-under: TianoCore Contribution Agreement 1.1
>>
>> I also noticed the PeCoff lib is going to loop and reload the .debug suction
>due to this mtoc bug, so it would be good to harden that code too.
>>
>> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> index 8d1daba..1e4c67e 100644
>> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
>>}
>>
>>return RETURN_SUCCESS;
>> +  } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
>> +return RETURN_SUCCESS;
>>  }
>>}
>>  }
>> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
>>if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
>>  ImageContext->DebugDirectoryEntryRva = (UINT32)
>(DebugDirectoryEntryRva + Index);
>>  return 

Re: [edk2] [RFC Patch 3/3] BaseTools/Scripts: Add sample makefile for use with RunMakefile.py

[Zhu, Yonghong]

Hi Mike,

The patch 3 has some "Tab character used". Please fix it when you commit.
Others are good to me.

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Kinney, Michael D 
Sent: Friday, August 04, 2017 1:30 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong ; 
Kinney, Michael D 
Subject: [RFC Patch 3/3] BaseTools/Scripts: Add sample makefile for use with 
RunMakefile.py

Add sample makefile that can be used to test RunMakefile.py script and can also 
be used as a template to start a new PREBUILD/POSTBUILD makefile.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 BaseTools/Scripts/RunMakefileSample.mak | 43 +
 1 file changed, 43 insertions(+)
 create mode 100644 BaseTools/Scripts/RunMakefileSample.mak

diff --git a/BaseTools/Scripts/RunMakefileSample.mak 
b/BaseTools/Scripts/RunMakefileSample.mak
new file mode 100644
index 00..b0947b7644
--- /dev/null
+++ b/BaseTools/Scripts/RunMakefileSample.mak
@@ -0,0 +1,43 @@
+## @file
+# Sample makefile for PREBUILD or POSTBUILD action.
+#
+# Copyright (c) 2017, 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
+# 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.
+#
+
+all: show
+   @echo $@
+genc: show
+   @echo $@
+genmake: show
+   @echo $@
+modules: show
+   @echo $@
+libraries: show
+   @echo $@
+fds: show
+   @echo $@
+clean: show
+   @echo $@
+cleanall: show
+   @echo $@
+cleanlib: show
+   @echo $@
+run: show
+   @echo $@
+
+show:
+   @echo WORKSPACE $(WORKSPACE)
+   @echo PACKAGES_PATH $(PACKAGES_PATH)
+   @echo ACTIVE_PLATFORM.. $(ACTIVE_PLATFORM)
+   @echo TARGET_ARCH.. $(TARGET_ARCH)
+   @echo TOOL_CHAIN_TAG... $(TOOL_CHAIN_TAG)
+   @echo CONF_DIRECTORY... $(CONF_DIRECTORY)
+   @echo TARGET... $(TARGET)
+   @echo EXTRA_FLAGS.. $(EXTRA_FLAGS)
--
2.13.1.windows.2

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

Re: [edk2] [PATCH 1/1] BaseTools/tools_def.template: revert to large code model for X64/GCC5/LTO

[Shi, Steven]

Hi Laszlo,

I'm trying to reproduce your boot failure with OVMF in my Ubuntu system, but 
not succeed.  My GCC was built from GCC main trunk code in 20170601, and my ld 
linker is version 2.28. Could you try the ld 2.28 with your gcc-7.1 and check 
whether it works in your side?  Or, do you know where can I download the 
gcc-7.1 pre-built binaries?





jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ gcc --version

gcc (GCC) 8.0.0 20170601 (experimental)



jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ ld --version

GNU ld (GNU Binutils) 2.28



Below are my trying steps, and the Qemu can boot well into shell:

jshi19@jshi19-desktop:~/wksp_efi$ git clone https://github.com/lersek/edk2.git

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ git checkout gcc7_lto

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ git checkout 
7ef0dae092afcfb6fab7e8372c78097672168c4a

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ rm Build -r

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ rm Conf/tools_def.txt

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ rm Conf/target.txt

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ rm Conf/build_rule.txt

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ source edksetup.sh

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ make -C BaseTools/Source/C/ clean

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ make -C BaseTools/Source/C/
jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ build -p OvmfPkg/OvmfPkgX64.dsc 
-t GCC5 -DSECURE_BOOT_ENABLE=TRUE -a X64 -b DEBUG  -DDEBUG_ON_SERIAL_PORT -n 5

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2$ build -p 
OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -DSECURE_BOOT_ENABLE=TRUE -a IA32 -a X64 -b 
DEBUG  -DDEBUG_ON_SERIAL_PORT -n 5

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2/Build/OvmfX64/DEBUG_GCC5/FV$ 
qemu-system-x86_64  -bios OVMF.fd -serial file:serial.log -m 512 -hda fat:. 
-monitor stdio --enable-kvm

jshi19@jshi19-desktop:~/wksp_efi/Laszlo/edk2/Build/Ovmf3264/DEBUG_GCC5/FV$ 
qemu-system-x86_64  -bios OVMF.fd -serial file:serial.log -m 512 -hda fat:. 
-monitor stdio --enable-kvm







Steven Shi

Intel\SSG\STO\UEFI Firmware



Tel: +86 021-61166522

iNet: 821-6522





> -Original Message-

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of

> Laszlo Ersek

> Sent: Friday, August 11, 2017 8:34 AM

> To: edk2-devel-01 

> Cc: Ard Biesheuvel ; Justen, Jordan L

> ; Gao, Liming ; Kinney,

> Michael D 

> Subject: [edk2] [PATCH 1/1] BaseTools/tools_def.template: revert to large

> code model for X64/GCC5/LTO

>

> Several users have recently reported boot failures with OVMF. The symptoms

> are: blank screen and all VCPUs pegged at 100%. Alex Williamson has found

> the commit (via bisection) that exposes the issue. In this patch I'll

> attempt to analyze the symptoms and fix the root problem.

>

> (1) "UefiCpuPkg/Library/MpInitLib" is an IA32/X64 library instance that

> provides multiprocessing services. It is built into the CpuMpPei

> module (for the PEI phase) and the CpuDxe module (for the DXE and BDS

> phases). When either of these modules starts up during boot, it

> briefly boots up all of the CPUs, perfoms some counting /

> synchronization, and then all the APs (application processors) are

> quiesced until further work arrives.

>

> The APs boot in real mode, and need assembly language init code (a

> "reset vector") that gradually takes them into 64-bit mode, before

> they can call back into normal C code. The reset vector code is

> assembled at build time, but it is copied as data under 1MB (for real

> mode execution by the APs) during boot, whenever CpuMpPei or CpuDxe

> launches.

>

> Part of the reset vector code is FPU initialization. The reset vector

> in "UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm" calls the assembly

> language function InitializeFloatingPointUnits(), which is however

> provided by an external library. Normally, when the object files were

> linked together, this function call would result in an absolute

> reference (and matching relocation, performed at module loading), and

> the instance of the reset vector that was copied under 1MB during boot

> would refer to the same, unchanged, absolute address of

> InitializeFloatingPointUnits(), residing in CpuMpPei or CpuDxe.

>

> This didn't work with X64 XCODE5 builds however. XCODE5 prefers

> RIP-relative addressing for X64 (i.e., the assembly language function

> call depended on the distance between the call site and the callee).

> When the call site was copied under 1MB but

> InitializeFloatingPointUnits() would not move, the call broke.

>

> This was tracked in TianoCore BZ#565 and fixed in commit 3b2928b46987

> ("UefiCpuPkg/MpInitLib: Fix X64 XCODE5/NASM compatibility issues",

> 2017-05-17). The 

Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

[Andrew Fish]

> On Aug 10, 2017, at 9:48 PM, Gao, Liming  wrote:
> 
> Andrew:
>  Edk2 Build system calls GenFw to generate EFI image in build phase. Even if 
> this image is not built into BIOS image, its EFI image will be generated by 
> GenFw. So, only if this EFI image is built from EDK2 project, it can be 
> updated by GenFw tool. You can see this step in build_rule.txt to convert 
> .dll to .efi image.
> 

Gao,

We can fix the future for edk2, but we can't change the past, or change other 
build systems. It is not safe to assume that every EFI executable that the DXE 
Core will see is as new as the firmware, or was even built via the edk2. Option 
ROMs and OS loaders are good examples of this issue. 

Thanks,

Andrew Fish

> Thanks
> Liming
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Friday, August 11, 2017 12:59 AM
> To: Gao, Liming 
> Cc: Zhu, Yonghong ; Kinney, Michael D 
> ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build 
> AppPkg with XCODE5
> 
> 
> On Aug 10, 2017, at 3:38 AM, Gao, Liming 
> > wrote:
> 
> Andrew:
> If this is a mtoc bug, I suggest to update GenFw to always correct it in the 
> generated EFI image. If so, the EFI image is always correct. There is no 
> change requirement in PeCoff library in MdePkg.
> 
> 
> Liming,
> 
> EFI supports loading PE/COFF images that are not built at the same time as 
> the platform firmware (UEFI Shell, OS loader), and that is why I added the 
> fix to the PE/COFF library code.
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> Thanks
> Liming
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Tuesday, August 8, 2017 12:26 AM
> To: Zhu, Yonghong >
> Cc: edk2-devel@lists.01.org; Gao, Liming 
> >; Kinney, Michael D 
> >
> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg 
> with XCODE5
> 
> Should that be:
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> I also noticed the PeCoff lib is going to loop and reload the .debug suction 
> due to this mtoc bug, so it would be good to harden that code too.
> 
> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
> b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 8d1daba..1e4c67e 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
>}
> 
>return RETURN_SUCCESS;
> +  } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> +return RETURN_SUCCESS;
>  }
>}
>  }
> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
>if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
>  ImageContext->DebugDirectoryEntryRva = (UINT32) 
> (DebugDirectoryEntryRva + Index);
>  return RETURN_SUCCESS;
> +} else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> +  return RETURN_SUCCESS;
>}
>  }
>}
> 
> 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=663
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu 
> >
>  wrote:
> 
> it is a bug in mtoc setting the size of the debug directory entry to
> the size of the .debug section, not the size of the
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
> memset() and boom.
> 
> Cc: Liming Gao 
> >
> Cc: Michael D Kinney 
> >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish 
> >
> ---
> BaseTools/Source/C/GenFw/GenFw.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c 
> b/BaseTools/Source/C/GenFw/GenFw.c
> index 246deb0..af60c92 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2813,10 +2813,11 @@ Returns:
> //
> // Get Debug, Export and Resource EntryTable RVA address.
> // Resource Directory entry need to review.
> //
> Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof 
> (EFI_IMAGE_FILE_HEADER));
> +  Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof 
> 

Re: [edk2] [PATCH v3] MdeModulePkg/EmmcDxe: Make sure no extra data is erased by EraseBlocks

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: Wu, Hao A 
Sent: Friday, August 11, 2017 12:38 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Zeng, Star ; Ni, Ruiyu 

Subject: [PATCH v3] MdeModulePkg/EmmcDxe: Make sure no extra data is erased by 
EraseBlocks

V3 changes:
Add debug messages for new return path when successfully erase the specified 
blocks. Refine logic for calculating the size for writing zeros to device.

V2 changes:

The Trim command is not supported on all eMMC devices. For those devices that 
do not support such command, add codes to handle the scenario.

Commit message:

The current implementation of the Erase Block Protocol service
EraseBlocks() uses the erase command. According to spec eMMC Electrical 
Standard 5.1, Section 6.6.9:

The erasable unit of the eMMC is the "Erase Group"; Erase group is measured in 
write blocks that are the basic writable units of the Device.
...
When the Erase is executed it will apply to all write blocks within an erase 
group.

However, code logic in function EmmcEraseBlocks() does not check whether the 
blocks to be erased form complete erase groups. Missing such checks will lead 
to erasing extra data on the device.

This commit will:
a. If the device support the Trim command, use the Trim command to perform the 
erase operations for eMMC devices.

According to the spec:
Unlike the Erase command, the Trim function applies the erase operation to 
write blocks instead of erase groups.

b. If the device does not support the Trim command, use the Erase command to 
erase the data in the erase groups. And write zeros to those blocks that cannot 
form a complete erase group.

Cc: Star Zeng 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 154 +-
 1 file changed, 151 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c 
b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index c432d26801..2e3bf5f156 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -1851,6 +1851,14 @@ EmmcEraseBlock (
   EraseBlock->SdMmcCmdBlk.CommandIndex = EMMC_ERASE;
   EraseBlock->SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
   EraseBlock->SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
+  if ((Device->ExtCsd.SecFeatureSupport & BIT4) != 0) {
+//
+// Perform a Trim operation which applies the erase operation to write 
blocks
+// instead of erase groups. (Spec JESD84-B51, eMMC Electrical Standard 5.1,
+// Section 6.6.10 and 6.10.4)
+//
+EraseBlock->SdMmcCmdBlk.CommandArgument = 1;  }
 
   EraseBlock->IsEnd = IsEnd;
   EraseBlock->Token = Token;
@@ -1903,6 +1911,43 @@ Error:
 }
 
 /**
+  Write zeros to specified blocks.
+
+  @param[in]  Partition A pointer to the EMMC_PARTITION instance.
+  @param[in]  StartLba  The starting logical block address to write 
zeros.
+  @param[in]  Size  The size in bytes to fill with zeros. This 
must be a multiple of
+the physical block size of the device.
+
+  @retval EFI_SUCCESS   The request is executed successfully.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be executed due to a 
lack of resources.
+  @retval OthersThe request could not be executed successfully.
+
+**/
+EFI_STATUS
+EmmcWriteZeros (
+  IN  EMMC_PARTITION*Partition,
+  IN  EFI_LBA   StartLba,
+  IN  UINTN Size
+  )
+{
+  EFI_STATUS   Status;
+  UINT8*Buffer;
+  UINT32   MediaId;
+
+  Buffer = AllocateZeroPool (Size);
+  if (Buffer == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  MediaId = Partition->BlockMedia.MediaId;
+
+  Status = EmmcReadWrite (Partition, MediaId, StartLba, Buffer, Size, 
+ FALSE, NULL);  FreePool (Buffer);
+
+  return Status;
+}
+
+/**
   Erase a specified number of device blocks.
 
   @param[in]   This   Indicates a pointer to the calling context.
@@ -1943,7 +1988,13 @@ EmmcEraseBlocks (
   EFI_BLOCK_IO_MEDIA*Media;
   UINTN BlockSize;
   UINTN BlockNum;
+  EFI_LBA   FirstLba;
   EFI_LBA   LastLba;
+  EFI_LBA   StartGroupLba;
+  EFI_LBA   EndGroupLba;
+  UINT32EraseGroupSize;
+  UINT32Remainder;
+  UINTN WriteZeroSize;
   UINT8 PartitionConfig;
   EMMC_PARTITION*Partition;
   EMMC_DEVICE   

Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

[Gao, Liming]

Andrew:
  Edk2 Build system calls GenFw to generate EFI image in build phase. Even if 
this image is not built into BIOS image, its EFI image will be generated by 
GenFw. So, only if this EFI image is built from EDK2 project, it can be updated 
by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi 
image.

Thanks
Liming
From: af...@apple.com [mailto:af...@apple.com]
Sent: Friday, August 11, 2017 12:59 AM
To: Gao, Liming 
Cc: Zhu, Yonghong ; Kinney, Michael D 
; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build 
AppPkg with XCODE5


On Aug 10, 2017, at 3:38 AM, Gao, Liming 
> wrote:

Andrew:
 If this is a mtoc bug, I suggest to update GenFw to always correct it in the 
generated EFI image. If so, the EFI image is always correct. There is no change 
requirement in PeCoff library in MdePkg.


Liming,

EFI supports loading PE/COFF images that are not built at the same time as the 
platform firmware (UEFI Shell, OS loader), and that is why I added the fix to 
the PE/COFF library code.

Thanks,

Andrew Fish


Thanks
Liming
From: af...@apple.com [mailto:af...@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong >
Cc: edk2-devel@lists.01.org; Gao, Liming 
>; Kinney, Michael D 
>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg 
with XCODE5

Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1

I also noticed the PeCoff lib is going to loop and reload the .debug suction 
due to this mtoc bug, so it would be good to harden that code too.

git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
}

return RETURN_SUCCESS;
+  } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+return RETURN_SUCCESS;
  }
}
  }
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
  ImageContext->DebugDirectoryEntryRva = (UINT32) 
(DebugDirectoryEntryRva + Index);
  return RETURN_SUCCESS;
+} else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+  return RETURN_SUCCESS;
}
  }
}



https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1

Thanks,

Andrew Fish


On Aug 6, 2017, at 9:00 PM, Yonghong Zhu 
>
 wrote:

it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.

Cc: Liming Gao 
>
Cc: Michael D Kinney 
>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish 
>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
 //
 // Get Debug, Export and Resource EntryTable RVA address.
 // Resource Directory entry need to review.
 //
 Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof 
(EFI_IMAGE_FILE_HEADER));
+  Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof 
(EFI_IMAGE_FILE_HEADER));
 if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr +  
FileHdr->SizeOfOptionalHeader);
   if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && 
\
   Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 
0) {
 ExportDirectoryEntryRva = 
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
   Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
   

Re: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra data is erased by EraseBlocks

[Wu, Hao A]

> -Original Message-
> From: Zeng, Star
> Sent: Friday, August 11, 2017 10:37 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Wu, Hao A; Ni, Ruiyu; Zeng, Star
> Subject: RE: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra
> data is erased by EraseBlocks
> 
> Two minor comments, others are good to me.
> 
> 1. Could the code use (LastLba + 1 - EndGroupLba) instead of (LastLba -
> EndGroupLba + 1)?
> 
> 2. Could the code have the debug message "DEBUG ((EFI_D_ERROR,
> "EmmcEraseBlocks(): Lba 0x%x BlkNo 0x%x Event %p with %r\n", Lba,
> BlockNum, Token->Event, Status))" for the new return paths the patch added?
> And the debug level should be DEBUG_INFO instead of EFI_D_ERROR?
> 

Thanks for the feedbacks. I will submit a new version of the patch.

Best Regards,
Hao Wu

> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao
> Wu
> Sent: Thursday, August 10, 2017 9:21 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ruiyu ; Zeng,
> Star 
> Subject: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra
> data is erased by EraseBlocks
> 
> V2 changes:
> 
> The Trim command is not supported on all eMMC devices. For those devices
> that do not support such command, add codes to handle the scenario.
> 
> Commit message:
> 
> The current implementation of the Erase Block Protocol service
> EraseBlocks() uses the erase command. According to spec eMMC Electrical
> Standard 5.1, Section 6.6.9:
> 
> The erasable unit of the eMMC is the "Erase Group"; Erase group is measured
> in write blocks that are the basic writable units of the Device.
> ...
> When the Erase is executed it will apply to all write blocks within an erase
> group.
> 
> However, code logic in function EmmcEraseBlocks() does not check whether
> the blocks to be erased form complete erase groups. Missing such checks will
> lead to erasing extra data on the device.
> 
> This commit will:
> a. If the device support the Trim command, use the Trim command to perform
> the erase operations for eMMC devices.
> 
> According to the spec:
> Unlike the Erase command, the Trim function applies the erase operation to
> write blocks instead of erase groups.
> 
> b. If the device does not support the Trim command, use the Erase command to
> erase the data in the erase groups. And write zeros to those blocks that 
> cannot
> form a complete erase group.
> 
> Cc: Star Zeng 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 127
> +-
>  1 file changed, 125 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> index c432d26801..916f15d429 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> @@ -1851,6 +1851,14 @@ EmmcEraseBlock (
>EraseBlock->SdMmcCmdBlk.CommandIndex = EMMC_ERASE;
>EraseBlock->SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
>EraseBlock->SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
> +  if ((Device->ExtCsd.SecFeatureSupport & BIT4) != 0) {
> +//
> +// Perform a Trim operation which applies the erase operation to write
> blocks
> +// instead of erase groups. (Spec JESD84-B51, eMMC Electrical Standard 
> 5.1,
> +// Section 6.6.10 and 6.10.4)
> +//
> +EraseBlock->SdMmcCmdBlk.CommandArgument = 1;  }
> 
>EraseBlock->IsEnd = IsEnd;
>EraseBlock->Token = Token;
> @@ -1903,6 +1911,43 @@ Error:
>  }
> 
>  /**
> +  Write zeros to specified blocks.
> +
> +  @param[in]  Partition A pointer to the EMMC_PARTITION instance.
> +  @param[in]  StartLba  The starting logical block address to write 
> zeros.
> +  @param[in]  Size  The size in bytes to fill with zeros. This 
> must be a
> multiple of
> +the physical block size of the device.
> +
> +  @retval EFI_SUCCESS   The request is executed successfully.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be executed due to
> a lack of resources.
> +  @retval OthersThe request could not be executed 
> successfully.
> +
> +**/
> +EFI_STATUS
> +EmmcWriteZeros (
> +  IN  EMMC_PARTITION*Partition,
> +  IN  EFI_LBA   StartLba,
> +  IN  UINTN Size
> +  )
> +{
> +  EFI_STATUS   Status;
> +  UINT8*Buffer;
> +  UINT32   MediaId;
> +
> +  Buffer = AllocateZeroPool (Size);
> +  if (Buffer == NULL) {
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  MediaId = Partition->BlockMedia.MediaId;
> +
> +  Status = EmmcReadWrite (Partition, MediaId, StartLba, Buffer, Size,
> + FALSE, NULL);  

[edk2] [PATCH 0/2] DxeCore: Fix double free pages on LoadImage failure path

[Star Zeng]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=667
REF: https://lists.01.org/pipermail/edk2-devel/2017-August/013112.html

Star Zeng (2):
  MdeModulePkg DxeCore: Fix double free pages on LoadImage failure path
  MdeModulePkg DxeCore: Enhance "ConvertPages: Incompatible memory
types"

 MdeModulePkg/Core/Dxe/Image/Image.c | 2 ++
 MdeModulePkg/Core/Dxe/Mem/Page.c| 9 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.7.0.windows.1

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

[edk2] [PATCH 2/2] MdeModulePkg DxeCore: Enhance "ConvertPages: Incompatible memory types"

[Star Zeng]

When double free pages by FreePages() or allocate allocated pages by
AllocatePages() with AllocateAddress type, the code will print debug
message "ConvertPages: Incompatible memory types", but the debug
message is not very obvious for the error paths by FreePages() or
AllocatePages().

Refer https://lists.01.org/pipermail/edk2-devel/2017-August/013075.html
for the discussion.

This patch is to enhance the debug message for the error paths by
FreePages() or AllocatePages.

Cc: Liming Gao 
Cc: Andrew Fish 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 3b3b9a8131a2..a142c79ee2ca 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Memory page management functions.
 
-Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2017, 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
@@ -788,7 +788,12 @@ CoreConvertPagesEx (
   // Debug code - verify conversion is allowed
   //
   if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == 
EfiConventionalMemory ? 1 : 0)) {
-DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
types\n"));
+DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
types, "));
+if (Entry->Type == EfiConventionalMemory) {
+  DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "the pages to free have been 
freed\n"));
+} else {
+  DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "the pages to allocate have been 
allocated\n"));
+}
 return EFI_NOT_FOUND;
   }
 
-- 
2.7.0.windows.1

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

[edk2] [Patch] BaseTools: Support TabSpace between section tag in DEC file

[Yonghong Zhu]

From: Yanyan Zhang 

Per DEC spec, multiple section tag use  to separate, and it can
support Tab, so this patch fix the bug to use Tab.

 ::= {} {}
 ::= *

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yanyan Zhang 
---
 BaseTools/Source/Python/Workspace/MetaFileParser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
b/BaseTools/Source/Python/Workspace/MetaFileParser.py
index e98352a..b756361 100644
--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
@@ -1752,11 +1752,11 @@ class DecParser(MetaFileParser):
 self._Scope = []
 self._SectionName = ''
 self._SectionType = []
 ArchList = set()
 PrivateList = set()
-Line = self._CurrentLine.replace("%s%s" % (TAB_COMMA_SPLIT, 
TAB_SPACE_SPLIT), TAB_COMMA_SPLIT)
+Line = re.sub(',[\s]*', TAB_COMMA_SPLIT, self._CurrentLine)
 for Item in Line[1:-1].split(TAB_COMMA_SPLIT):
 if Item == '':
 EdkLogger.error("Parser", FORMAT_UNKNOWN_ERROR,
 "section name can NOT be empty or incorrectly 
use separator comma",
 self.MetaFile, self._LineIndex + 1, 
self._CurrentLine)
-- 
2.6.1.windows.1

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

[edk2] OVMF compile error

[Chen, Farrah]

Hi,

When I build ovmf with commit: 76c6f69ccadc7835c9616b077d9ff1b8e46fe49e, the 
following error occurred:

git clone https://github.com/tianocore/edk2.git
cd edk2
OvmfPkg/build.sh -a X64

/home/fan/edk2/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c: In 
function 'GetImageNameFromHandle':
/home/fan/edk2/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c:220:25: 
error: passing argument 3 of 'gBS->HandleProtocol' from incompatible pointer 
type [-Werror]
 );
 ^
/home/fan/edk2/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c:220:25: 
note: expected 'void **' but argument is of type 'struct 
EFI_FIRMWARE_VOLUME2_PROTOCOL **'
cc1: all warnings being treated as errors
make: *** 
[/home/fan/edk2/Build/OvmfX64/DEBUG_GCC48/X64/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib/OUTPUT/Drivers.obj]
 Error 1


build.py...
: error 7000: Failed to execute command
make tbuild 
[/home/fan/edk2/Build/OvmfX64/DEBUG_GCC48/X64/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib]


build.py...
: error F002: Failed to build module

/home/fan/edk2/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
 [X64, GCC48, DEBUG]

- Failed -
Build end time: 10:45:31, Aug.11 2017
Build total time: 00:01:00


Best Regards,
Fan Chen

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

Re: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra data is erased by EraseBlocks

[Zeng, Star]

Two minor comments, others are good to me.

1. Could the code use (LastLba + 1 - EndGroupLba) instead of (LastLba - 
EndGroupLba + 1)?

2. Could the code have the debug message "DEBUG ((EFI_D_ERROR, 
"EmmcEraseBlocks(): Lba 0x%x BlkNo 0x%x Event %p with %r\n", Lba, BlockNum, 
Token->Event, Status))" for the new return paths the patch added? And the debug 
level should be DEBUG_INFO instead of EFI_D_ERROR?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao Wu
Sent: Thursday, August 10, 2017 9:21 AM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Ni, Ruiyu ; Zeng, Star 

Subject: [edk2] [PATCH v2] MdeModulePkg/EmmcDxe: Make sure no extra data is 
erased by EraseBlocks

V2 changes:

The Trim command is not supported on all eMMC devices. For those devices that 
do not support such command, add codes to handle the scenario.

Commit message:

The current implementation of the Erase Block Protocol service
EraseBlocks() uses the erase command. According to spec eMMC Electrical 
Standard 5.1, Section 6.6.9:

The erasable unit of the eMMC is the "Erase Group"; Erase group is measured in 
write blocks that are the basic writable units of the Device.
...
When the Erase is executed it will apply to all write blocks within an erase 
group.

However, code logic in function EmmcEraseBlocks() does not check whether the 
blocks to be erased form complete erase groups. Missing such checks will lead 
to erasing extra data on the device.

This commit will:
a. If the device support the Trim command, use the Trim command to perform the 
erase operations for eMMC devices.

According to the spec:
Unlike the Erase command, the Trim function applies the erase operation to 
write blocks instead of erase groups.

b. If the device does not support the Trim command, use the Erase command to 
erase the data in the erase groups. And write zeros to those blocks that cannot 
form a complete erase group.

Cc: Star Zeng 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 127 +-
 1 file changed, 125 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c 
b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index c432d26801..916f15d429 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -1851,6 +1851,14 @@ EmmcEraseBlock (
   EraseBlock->SdMmcCmdBlk.CommandIndex = EMMC_ERASE;
   EraseBlock->SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
   EraseBlock->SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
+  if ((Device->ExtCsd.SecFeatureSupport & BIT4) != 0) {
+//
+// Perform a Trim operation which applies the erase operation to write 
blocks
+// instead of erase groups. (Spec JESD84-B51, eMMC Electrical Standard 5.1,
+// Section 6.6.10 and 6.10.4)
+//
+EraseBlock->SdMmcCmdBlk.CommandArgument = 1;  }
 
   EraseBlock->IsEnd = IsEnd;
   EraseBlock->Token = Token;
@@ -1903,6 +1911,43 @@ Error:
 }
 
 /**
+  Write zeros to specified blocks.
+
+  @param[in]  Partition A pointer to the EMMC_PARTITION instance.
+  @param[in]  StartLba  The starting logical block address to write 
zeros.
+  @param[in]  Size  The size in bytes to fill with zeros. This 
must be a multiple of
+the physical block size of the device.
+
+  @retval EFI_SUCCESS   The request is executed successfully.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be executed due to a 
lack of resources.
+  @retval OthersThe request could not be executed successfully.
+
+**/
+EFI_STATUS
+EmmcWriteZeros (
+  IN  EMMC_PARTITION*Partition,
+  IN  EFI_LBA   StartLba,
+  IN  UINTN Size
+  )
+{
+  EFI_STATUS   Status;
+  UINT8*Buffer;
+  UINT32   MediaId;
+
+  Buffer = AllocateZeroPool (Size);
+  if (Buffer == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  MediaId = Partition->BlockMedia.MediaId;
+
+  Status = EmmcReadWrite (Partition, MediaId, StartLba, Buffer, Size, 
+ FALSE, NULL);  FreePool (Buffer);
+
+  return Status;
+}
+
+/**
   Erase a specified number of device blocks.
 
   @param[in]   This   Indicates a pointer to the calling context.
@@ -1943,7 +1988,13 @@ EmmcEraseBlocks (
   EFI_BLOCK_IO_MEDIA*Media;
   UINTN BlockSize;
   UINTN BlockNum;
+  EFI_LBA   FirstLba;
   EFI_LBA   LastLba;
+  EFI_LBA   StartGroupLba;
+  EFI_LBA   EndGroupLba;
+  UINT32   

[edk2] [Patch v2] NetworkPkg/HttpDxe: Handle the HttpVersionUnsupported in the HttpConfigData

[Jiaxin Wu]

v2:
* Refine the patch by changing the '==' to '>='.

Cc: Ye Ting 
Cc: Jin Eric 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/HttpDxe/HttpImpl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index e0fecac..c104b61 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -149,10 +149,14 @@ EfiHttpConfigure (
   HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This);
   ASSERT (HttpInstance != NULL && HttpInstance->Service != NULL);
 
   if (HttpConfigData != NULL) {
 
+if (HttpConfigData->HttpVersion >= HttpVersionUnsupported) {
+  return EFI_UNSUPPORTED;
+}
+
 //
 // Now configure this HTTP instance.
 //
 if (HttpInstance->State != HTTP_STATE_UNCONFIGED) {
   return EFI_ALREADY_STARTED;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch] BaseTools: Don't need to add extra quotes when UI string from file

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Thursday, August 10, 2017 4:58 PM
To: edk2-devel@lists.01.org
Cc: Wang, BinX A ; Gao, Liming 
Subject: [edk2] [Patch] BaseTools: Don't need to add extra quotes when UI 
string from file

From: Bin Wang 

when the UI string is read from files, we don't need to add the extra quotes. 
Otherwise, it will cause UI name has this extra quotes.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Bin Wang 
---
 BaseTools/Source/Python/GenFds/UiSection.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/UiSection.py 
b/BaseTools/Source/Python/GenFds/UiSection.py
index d54e3b2..419e1ee 100644
--- a/BaseTools/Source/Python/GenFds/UiSection.py
+++ b/BaseTools/Source/Python/GenFds/UiSection.py
@@ -1,9 +1,9 @@
 ## @file
 # process UI section generation
 #
-#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2017, 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  # 
 http://opensource.org/licenses/bsd-license.php
@@ -64,11 +64,10 @@ class UiSection (UiSectionClassObject):
 elif self.FileName != None:
 FileNameStr = 
GenFdsGlobalVariable.ReplaceWorkspaceMacro(self.FileName)
 FileNameStr = GenFdsGlobalVariable.MacroExtend(FileNameStr, Dict)
 FileObj = open(FileNameStr, 'r')
 NameString = FileObj.read()
-NameString = '\"' + NameString + "\""
 FileObj.close()
 else:
 NameString = ''
 
 GenFdsGlobalVariable.GenerateSection(OutputFile, None, 
'EFI_SECTION_USER_INTERFACE', Ui=NameString)
--
2.6.1.windows.1

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

[edk2] [PATCH 0/1] BaseTools/tools_def.template: revert to large code model for X64/GCC5/LTO

[Laszlo Ersek]

Patch #1 explains it all (grab a coffee first). For testing, it's likely
most robust to run

  git clean -fdx
  exit

in your terminal, before fetching & checking out my branch, and
rebuilding OVMF.

Repo:   https://github.com/lersek/edk2.git
Branch: gcc7_lto

Cc: Alex Williamson 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Yonghong Zhu 

Thanks
Laszlo

Laszlo Ersek (1):
  BaseTools/tools_def.template: revert to large code model for
X64/GCC5/LTO

 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.13.1.3.g8be5a757fa67

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

[edk2] [PATCH V2] Maintainers.txt: Change maintainer for Intel*Pkg.

[Jiewen Yao]

Since Giri left Intel, we change 3 Intel*Pkg
maintainer.

Cc: Chasel Chiu 
Cc: Amy Chan 
Cc: Rangasai V Chaganty 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 Maintainers.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 39b5b67..e6aac63 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -131,12 +131,12 @@ M: Jeff Fan 
 IntelFsp2Pkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg
 M: Jiewen Yao 
-M: Giri P Mudusuru 
+M: Chasel Chiu 
 
 IntelFsp2WrapperPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg
 M: Jiewen Yao 
-M: Giri P Mudusuru 
+M: Chasel Chiu 
 
 IntelFspPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg
@@ -149,7 +149,7 @@ M: Jiewen Yao 
 IntelSiliconPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg
 M: Jiewen Yao 
-M: Giri P Mudusuru 
+M: Rangasai V Chaganty 
 
 MdeModulePkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
-- 
2.7.4.windows.1

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

Re: [edk2] [PATCH] IntelSiliconPkg: Fix VS2015 NOOPT IA32 build failure in IntelVTdDxe

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Thursday, August 10, 2017 3:18 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen 
> Subject: [PATCH] IntelSiliconPkg: Fix VS2015 NOOPT IA32 build failure in
> IntelVTdDxe
> 
> There are VS2015 NOOPT IA32 build failure like below in IntelVTdDxe.
> XXX.lib(XXX.obj) : error LNK2001: unresolved external symbol __allshl
> XXX.lib(XXX.obj) : error LNK2001: unresolved external symbol __aullshr
> 
> This patch is to update Vtd.h to use UINT32 instead of UINT64 for
> bitfields in structure definition, and also update IntelVTdDxe code
> accordingly.
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  IntelSiliconPkg/Include/IndustryStandard/Vtd.h   | 225
> ---
>  IntelSiliconPkg/IntelVTdDxe/DmaProtection.h  |   2 +
>  IntelSiliconPkg/IntelVTdDxe/PciInfo.c|   4 +-
>  IntelSiliconPkg/IntelVTdDxe/TranslationTable.c   |  39 ++--
>  IntelSiliconPkg/IntelVTdDxe/TranslationTableEx.c |  12 +-
>  IntelSiliconPkg/IntelVTdDxe/VtdReg.c |   2 +-
>  6 files changed, 152 insertions(+), 132 deletions(-)
> 
> diff --git a/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
> b/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
> index 5c6a494eae34..3b7012c5a576 100644
> --- a/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
> +++ b/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
> @@ -26,9 +26,10 @@
> 
>  typedef union {
>struct {
> -UINT64  Present:1;
> -UINT64  Reserved_1:11;
> -UINT64  ContextTablePointer:52;
> +UINT32  Present:1;
> +UINT32  Reserved_1:11;
> +UINT32  ContextTablePointerLo:20;
> +UINT32  ContextTablePointerHi:32;
> 
>  UINT64  Reserved_64;
>} Bits;
> @@ -40,13 +41,15 @@ typedef union {
> 
>  typedef union {
>struct {
> -UINT64  LowerPresent:1;
> -UINT64  Reserved_1:11;
> -UINT64  LowerContextTablePointer:52;
> -
> -UINT64  UpperPresent:1;
> -UINT64  Reserved_65:11;
> -UINT64  UpperContextTablePointer:52;
> +UINT32  LowerPresent:1;
> +UINT32  Reserved_1:11;
> +UINT32  LowerContextTablePointerLo:20;
> +UINT32  LowerContextTablePointerHi:32;
> +
> +UINT32  UpperPresent:1;
> +UINT32  Reserved_65:11;
> +UINT32  UpperContextTablePointerLo:20;
> +UINT32  UpperContextTablePointerHi:32;
>} Bits;
>struct {
>  UINT64  Uint64Lo;
> @@ -56,17 +59,19 @@ typedef union {
> 
>  typedef union {
>struct {
> -UINT64  Present:1;
> -UINT64  FaultProcessingDisable:1;
> -UINT64  TranslationType:2;
> -UINT64  Reserved_4:8;
> -UINT64  SecondLevelPageTranslationPointer:52;
> -
> -UINT64  AddressWidth:3;
> -UINT64  Ignored_67:4;
> -UINT64  Reserved_71:1;
> -UINT64  DomainIdentifier:16;
> -UINT64  Reserved_88:40;
> +UINT32  Present:1;
> +UINT32  FaultProcessingDisable:1;
> +UINT32  TranslationType:2;
> +UINT32  Reserved_4:8;
> +UINT32  SecondLevelPageTranslationPointerLo:20;
> +UINT32  SecondLevelPageTranslationPointerHi:32;
> +
> +UINT32  AddressWidth:3;
> +UINT32  Ignored_67:4;
> +UINT32  Reserved_71:1;
> +UINT32  DomainIdentifier:16;
> +UINT32  Reserved_88:8;
> +UINT32  Reserved_96:32;
>} Bits;
>struct {
>  UINT64  Uint64Lo;
> @@ -76,51 +81,54 @@ typedef union {
> 
>  typedef union {
>struct {
> -UINT64  Present:1;
> -UINT64  FaultProcessingDisable:1;
> -UINT64  TranslationType:3;
> -UINT64  ExtendedMemoryType:3;
> -UINT64  DeferredInvalidateEnable:1;
> -UINT64  PageRequestEnable:1;
> -UINT64  NestedTranslationEnable:1;
> -UINT64  PASIDEnable:1;
> -UINT64  SecondLevelPageTranslationPointer:52;
> -
> -UINT64  AddressWidth:3;
> -UINT64  PageGlobalEnable:1;
> -UINT64  NoExecuteEnable:1;
> -UINT64  WriteProtectEnable:1;
> -UINT64  CacheDisable:1;
> -UINT64  ExtendedMemoryTypeEnable:1;
> -UINT64  DomainIdentifier:16;
> -UINT64  SupervisorModeExecuteProtection:1;
> -UINT64  ExtendedAccessedFlagEnable:1;
> -UINT64  ExecuteRequestsEnable:1;
> -UINT64  SecondLevelExecuteEnable:1;
> -UINT64  Reserved_92:4;
> -UINT64  PageAttributeTable0:3;
> -UINT64  Reserved_Pat0:1;
> -UINT64  PageAttributeTable1:3;
> -UINT64  Reserved_Pat1:1;
> -UINT64  PageAttributeTable2:3;
> -UINT64  Reserved_Pat2:1;
> -UINT64  PageAttributeTable3:3;
> -UINT64  Reserved_Pat3:1;
> -UINT64  PageAttributeTable4:3;
> -UINT64  Reserved_Pat4:1;
> -UINT64  PageAttributeTable5:3;
> -UINT64  Reserved_Pat5:1;
> -UINT64  PageAttributeTable6:3;
> -UINT64  Reserved_Pat6:1;
> -UINT64  PageAttributeTable7:3;
> -UINT64  Reserved_Pat7:1;
> -
> -UINT64  PASIDTableSize:4;
> -UINT64  Reserved_132:8;
> -UINT64  PASIDTablePointer:52;
> -
> - 

Re: [edk2] [PATCH 0/4] read-only UDF file system support

[Paulo Alcantara]

Hi,

(sorry for the late reply)

On 09/08/2017 22:11, Ni, Ruiyu wrote:



Regards,
Ray


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Wednesday, August 9, 2017 10:01 PM
To: Ni, Ruiyu ; Zeng, Star ; 
edk2-devel@lists.01.org
Cc: Dong, Eric ; Wu, Hao A ; Justen, Jordan 
L ;
Andrew Fish ; Gao, Liming ; Kinney, Michael D 
;
Laszlo Ersek 
Subject: Re: [edk2] [PATCH 0/4] read-only UDF file system support

(heh - forgot to answer your question about the GUID :-) )

On 8/9/2017 10:26 AM, Paulo Alcantara wrote:

Hi Ray,

Thanks for the review. My comments below.

On 8/9/2017 3:05 AM, Ni, Ruiyu wrote:

Paulo,
Thanks for enabling the UDF support into EDKII.
Below are several comments:
1. Could you please separate the patch to modify MdePkg and MdeModulePkg?


Sure.


2. UDF_CDROM_VOLUME_IDENTIFIER is also defined in Eltorito.h as CDVOL_ID.
   Maybe we could redefine CDVOL_ID to UDF_CDROM_VOLUME_IDENTIFIER?


The Volume Identifier structure is defined in ECMA-167 specification
(not UDF specific), so perhaps it would be better if we create a
separate header file (e.g., Ecma-167.h) and define
ECMA167_VOLUME_IDENTIFIER structure. That would be used in both ElTorito
and UDF codes. What do you think?



2. Why do you need a PCD to control the UDF support? I prefer no. More
choices
   is not always good


The original idea of including this PCD was to avoid adding unnecessary
overhead in PartitionDxe driver to something (UDF) that wasn't actually
defined in UEFI specification -- leaving it as an optional feature.

But yes, I agree with you that just complicates things and would be
better to remove it.



3.  Is gUdfVolumeSignatureGuid only used in Partition driver to
produce the device
path? Can we just use HARDDRIVE_DEVICE_PATH? Or at least
gUdfVolumeSignatureGuid
can be removed, the GUID macro is enough?


The GUID is used in both Partition and UdfDxe drivers. Do you mean that
we should use HARDDRIVE_DEVICE_PATH and then appending the UDF-specific
GUID to it?


I originally thought UdfDxe driver needs to use this GUID to know it's a UDF 
partition.
But later I found UdfDxe contains a function SupportUdfFileSystem() to parse the
partition content to decide whether to manage that partition.
I wasn't able to find the reference of this GUID.


Yes - you're right. I think it should just check for the installed GUID, 
indeed. So we don't have to parse and look for UDF file systems twice.




I just tried again, still didn't find it. But I did find some improper 
implementation of
driver-model logic.
I saw UdfDxe uses OPEN_PROTOCOL_GET to open the BlockIo and DiskIo.
It should use OPEN_PROTOCOL_BY_DRIVER.
In details, it should firstly try OPEN_BY_DRIVER to open the BlockIo and DiskIo 
in
Supported(), when either one fails, the Supported() returns failure. But please
keep in mind to close the successfully-opened BlockIo/DiskIo. Supported() is a
test function called by DxeCore, and it should not alter the system state.
Start() should then repeat the same open logic as that in Supported(), but it's
find to use ASSERT_EFI_ERROR() to assert the return status of OPEN_BY_DRIVER
is EFI_SUCCESS because per UEFI spec, Start() should only be called when 
Supported()
returns EFI_SUCCESS. Start() will then install the SimpleFileSystem instance on 
the
same Handle that have BlockIo/DiskIo installed. (I noticed that your driver 
creates
a new handle for SimpleFileSystem, which is unnecessary and may break the driver
model chain.)

The OPEN_BY_DRIVER open is necessary because DxeCore driver model core logic
records the special open operation. and when BlockIo or DiskIo is uninstalled,
the Stop() function whose corresponding Start() is opening the same 
BlockIo/DiskIo
OPEN_BY_DRIVER will get called. So you can treat OPEN_BY_DRIVER is a mechanism
to notify the upper layer driver to destroy the newly created 
service(SimpleFileSystem)
when lower layer services (BlockIo/DiskIo) it layers on is destroyed 
(Uninstalled).

Not sure if I explained well. You could refer to
https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-Writer's-Guide
written by Kinney Michael for detailed instructions.


Good catch! You explained it very well. I'll fix that up in the next series.

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

Re: [edk2] [PATCH 1/3] Maintainers.txt: List Tianocore Stewards

[Andrew Fish]

Series

Reviewed-by: Andrew Fish >

> On Aug 10, 2017, at 4:13 PM, Kinney, Michael D  
> wrote:
> 
> Series 
> 
> Reviewed-by: Michael D Kinney 
> 
>> -Original Message-
>> From: Justen, Jordan L
>> Sent: Thursday, August 10, 2017 3:11 PM
>> To: edk2-devel@lists.01.org
>> Cc: Justen, Jordan L ; Andrew Fish
>> ; Leif Lindholm ;
>> Kinney, Michael D 
>> Subject: [PATCH 1/3] Maintainers.txt: List Tianocore Stewards
>> 
>> Cc: Andrew Fish 
>> Cc: Leif Lindholm 
>> Cc: Michael D Kinney 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Jordan Justen 
>> ---
>> Maintainers.txt | 6 ++
>> 1 file changed, 6 insertions(+)
>> 
>> diff --git a/Maintainers.txt b/Maintainers.txt
>> index 9d1b9fb941..a5e2e0af46 100644
>> --- a/Maintainers.txt
>> +++ b/Maintainers.txt
>> @@ -34,6 +34,12 @@ T: git (mirror) -
>> https://bitbucket.org/tianocore/edk2.git
>> T: git (mirror) - http://git.code.sf.net/p/tianocore/edk2
>> T: svn (read-only, deprecated) -
>> https://svn.code.sf.net/p/edk2/code/trunk/edk2
>> 
>> +Tianocore Stewards
>> +--
>> +M: Andrew Fish 
>> +M: Leif Lindholm 
>> +M: Michael D Kinney 
>> +
>> Responsible Disclosure, Reporting Security Issues
>> -
>> W:
>> https://github.com/tianocore/tianocore.github.io/wiki/Security
>> --
>> 2.14.0
> 

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

Re: [edk2] [PATCH 1/3] Maintainers.txt: List Tianocore Stewards

[Kinney, Michael D]

Series 

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: Justen, Jordan L
> Sent: Thursday, August 10, 2017 3:11 PM
> To: edk2-devel@lists.01.org
> Cc: Justen, Jordan L ; Andrew Fish
> ; Leif Lindholm ;
> Kinney, Michael D 
> Subject: [PATCH 1/3] Maintainers.txt: List Tianocore Stewards
> 
> Cc: Andrew Fish 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jordan Justen 
> ---
>  Maintainers.txt | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 9d1b9fb941..a5e2e0af46 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -34,6 +34,12 @@ T: git (mirror) -
> https://bitbucket.org/tianocore/edk2.git
>  T: git (mirror) - http://git.code.sf.net/p/tianocore/edk2
>  T: svn (read-only, deprecated) -
> https://svn.code.sf.net/p/edk2/code/trunk/edk2
> 
> +Tianocore Stewards
> +--
> +M: Andrew Fish 
> +M: Leif Lindholm 
> +M: Michael D Kinney 
> +
>  Responsible Disclosure, Reporting Security Issues
>  -
>  W:
> https://github.com/tianocore/tianocore.github.io/wiki/Security
> --
> 2.14.0

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

Re: [edk2] [PATCH 1/3] Maintainers.txt: List Tianocore Stewards

[Leif Lindholm]

I don't actually have a strong opinion that the number of maintainers
per package need to be kept at 2, and I'm not sure we have actually
codified that anywhere. But I also don't have any issues with any of
the changes suggested in this series.

So, in order to keep the noise down, for the series:
Reviewed-by: Leif Lindholm 

On Thu, Aug 10, 2017 at 03:11:27PM -0700, Jordan Justen wrote:
> Cc: Andrew Fish 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jordan Justen 
> ---
>  Maintainers.txt | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 9d1b9fb941..a5e2e0af46 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -34,6 +34,12 @@ T: git (mirror) - https://bitbucket.org/tianocore/edk2.git
>  T: git (mirror) - http://git.code.sf.net/p/tianocore/edk2
>  T: svn (read-only, deprecated) - 
> https://svn.code.sf.net/p/edk2/code/trunk/edk2
>  
> +Tianocore Stewards
> +--
> +M: Andrew Fish 
> +M: Leif Lindholm 
> +M: Michael D Kinney 
> +
>  Responsible Disclosure, Reporting Security Issues
>  -
>  W: https://github.com/tianocore/tianocore.github.io/wiki/Security
> -- 
> 2.14.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions

[Carsey, Jaben]

Looks good to me.

Daryl?

> -Original Message-
> From: Thomas Palmer [mailto:thomas.pal...@hpe.com]
> Sent: Thursday, August 10, 2017 3:35 PM
> To: edk2-devel@lists.01.org
> Cc: edk2-li...@mc2research.org; Carsey, Jaben ;
> joseph.shiffl...@hpe.com; Thomas Palmer 
> Subject: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event
> functions
> Importance: High
> 
> The gBS->CreateEvent expects a EFI_EVENT_NOTIFY function as the third
> argument. The EFIAPI token is an important component of that prototype. Its
> absence can cause unexpected issues on DEBUG systems built with GCC due
> to
> ABI mismatches.
> 
> Both EslTcp4ConnectComplete and EslTcp6ConnectComplete did not have
> the
> EFIAPI token required of a EFI_EVENT_NOTIFY function. GCC did not catch
> this because of the explicit EFI_EVENT_NOTIFY cast.  By removing the cast,
> a build error ensues.
> 
> This patch removes the cast and updates both functions to comply with
> EFI_EVENT_NOTIFY.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Thomas Palmer 
> ---
>  StdLib/EfiSocketLib/Tcp4.c | 8 ++--
>  StdLib/EfiSocketLib/Tcp6.c | 8 ++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c
> index 68477fba6e70..8125a8d4f5ad 100644
> --- a/StdLib/EfiSocketLib/Tcp4.c
> +++ b/StdLib/EfiSocketLib/Tcp4.c
> @@ -2,6 +2,7 @@
>Implement the TCP4 driver support for the socket layer.
> 
>Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
> +  (C) Copyright 2017 Hewlett Packard Enterprise Development LP
>This program and the accompanying materials are licensed and made
> available
>under the terms and conditions of the BSD License which accompanies this
>distribution.  The full text of the license may be found at
> @@ -192,9 +193,10 @@ EslTcp4Accept (
> 
>  **/
>  VOID
> +EFIAPI
>  EslTcp4ConnectComplete (
>IN EFI_EVENT Event,
> -  IN ESL_PORT * pPort
> +  IN VOID  *Context
>)
>  {
>BOOLEAN bRemoveFirstPort;
> @@ -203,12 +205,14 @@ EslTcp4ConnectComplete (
>ESL_SOCKET * pSocket;
>ESL_TCP4_CONTEXT * pTcp4;
>EFI_STATUS Status;
> +  ESL_PORT * pPort;
> 
>DBG_ENTER ( );
> 
>//
>//  Locate the TCP context
>//
> +  pPort = Context;
>pSocket = pPort->pSocket;
>pTcp4 = >Context.Tcp4;
> 
> @@ -1288,7 +1292,7 @@ EslTcp4PortAllocate (
>  //
>  Status = gBS->CreateEvent (  EVT_NOTIFY_SIGNAL,
>   TPL_SOCKETS,
> - (EFI_EVENT_NOTIFY)EslTcp4ConnectComplete,
> + EslTcp4ConnectComplete,
>   pPort,
>   >ConnectToken.CompletionToken.Event);
>  if ( EFI_ERROR ( Status )) {
> diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c
> index 0f6d2d6ac93c..9f9c00f6dc57 100644
> --- a/StdLib/EfiSocketLib/Tcp6.c
> +++ b/StdLib/EfiSocketLib/Tcp6.c
> @@ -2,6 +2,7 @@
>Implement the TCP6 driver support for the socket layer.
> 
>Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.
> +  (C) Copyright 2017 Hewlett Packard Enterprise Development LP
>This program and the accompanying materials are licensed and made
> available
>under the terms and conditions of the BSD License which accompanies this
>distribution.  The full text of the license may be found at
> @@ -186,9 +187,10 @@ EslTcp6Accept (
> 
>  **/
>  VOID
> +EFIAPI
>  EslTcp6ConnectComplete (
>IN EFI_EVENT Event,
> -  IN ESL_PORT * pPort
> +  IN VOID  *Context
>)
>  {
>BOOLEAN bRemoveFirstPort;
> @@ -197,12 +199,14 @@ EslTcp6ConnectComplete (
>ESL_SOCKET * pSocket;
>ESL_TCP6_CONTEXT * pTcp6;
>EFI_STATUS Status;
> +  ESL_PORT * pPort;
> 
>DBG_ENTER ( );
> 
>//
>//  Locate the TCP context
>//
> +  pPort = Context;
>pSocket = pPort->pSocket;
>pTcp6 = >Context.Tcp6;
> 
> @@ -1339,7 +1343,7 @@ EslTcp6PortAllocate (
>  //
>  Status = gBS->CreateEvent (  EVT_NOTIFY_SIGNAL,
>   TPL_SOCKETS,
> - (EFI_EVENT_NOTIFY)EslTcp6ConnectComplete,
> + EslTcp6ConnectComplete,
>   pPort,
>   >ConnectToken.CompletionToken.Event);
>  if ( EFI_ERROR ( Status )) {
> --
> 2.7.4

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

[edk2] [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions

[Thomas Palmer]

The gBS->CreateEvent expects a EFI_EVENT_NOTIFY function as the third
argument. The EFIAPI token is an important component of that prototype. Its
absence can cause unexpected issues on DEBUG systems built with GCC due to
ABI mismatches.

Both EslTcp4ConnectComplete and EslTcp6ConnectComplete did not have the
EFIAPI token required of a EFI_EVENT_NOTIFY function. GCC did not catch
this because of the explicit EFI_EVENT_NOTIFY cast.  By removing the cast,
a build error ensues.

This patch removes the cast and updates both functions to comply with
EFI_EVENT_NOTIFY.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer 
---
 StdLib/EfiSocketLib/Tcp4.c | 8 ++--
 StdLib/EfiSocketLib/Tcp6.c | 8 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c
index 68477fba6e70..8125a8d4f5ad 100644
--- a/StdLib/EfiSocketLib/Tcp4.c
+++ b/StdLib/EfiSocketLib/Tcp4.c
@@ -2,6 +2,7 @@
   Implement the TCP4 driver support for the socket layer.
 
   Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+  (C) Copyright 2017 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials are licensed and made available
   under the terms and conditions of the BSD License which accompanies this
   distribution.  The full text of the license may be found at
@@ -192,9 +193,10 @@ EslTcp4Accept (
 
 **/
 VOID
+EFIAPI
 EslTcp4ConnectComplete (
   IN EFI_EVENT Event,
-  IN ESL_PORT * pPort
+  IN VOID  *Context
   )
 {
   BOOLEAN bRemoveFirstPort;
@@ -203,12 +205,14 @@ EslTcp4ConnectComplete (
   ESL_SOCKET * pSocket;
   ESL_TCP4_CONTEXT * pTcp4;
   EFI_STATUS Status;
+  ESL_PORT * pPort;
 
   DBG_ENTER ( );
 
   //
   //  Locate the TCP context
   //
+  pPort = Context;
   pSocket = pPort->pSocket;
   pTcp4 = >Context.Tcp4;
 
@@ -1288,7 +1292,7 @@ EslTcp4PortAllocate (
 //
 Status = gBS->CreateEvent (  EVT_NOTIFY_SIGNAL,
  TPL_SOCKETS,
- (EFI_EVENT_NOTIFY)EslTcp4ConnectComplete,
+ EslTcp4ConnectComplete,
  pPort,
  >ConnectToken.CompletionToken.Event);
 if ( EFI_ERROR ( Status )) {
diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c
index 0f6d2d6ac93c..9f9c00f6dc57 100644
--- a/StdLib/EfiSocketLib/Tcp6.c
+++ b/StdLib/EfiSocketLib/Tcp6.c
@@ -2,6 +2,7 @@
   Implement the TCP6 driver support for the socket layer.
 
   Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.
+  (C) Copyright 2017 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials are licensed and made available
   under the terms and conditions of the BSD License which accompanies this
   distribution.  The full text of the license may be found at
@@ -186,9 +187,10 @@ EslTcp6Accept (
 
 **/
 VOID
+EFIAPI
 EslTcp6ConnectComplete (
   IN EFI_EVENT Event,
-  IN ESL_PORT * pPort
+  IN VOID  *Context
   )
 {
   BOOLEAN bRemoveFirstPort;
@@ -197,12 +199,14 @@ EslTcp6ConnectComplete (
   ESL_SOCKET * pSocket;
   ESL_TCP6_CONTEXT * pTcp6;
   EFI_STATUS Status;
+  ESL_PORT * pPort;
 
   DBG_ENTER ( );
 
   //
   //  Locate the TCP context
   //
+  pPort = Context;
   pSocket = pPort->pSocket;
   pTcp6 = >Context.Tcp6;
 
@@ -1339,7 +1343,7 @@ EslTcp6PortAllocate (
 //
 Status = gBS->CreateEvent (  EVT_NOTIFY_SIGNAL,
  TPL_SOCKETS,
- (EFI_EVENT_NOTIFY)EslTcp6ConnectComplete,
+ EslTcp6ConnectComplete,
  pPort,
  >ConnectToken.CompletionToken.Event);
 if ( EFI_ERROR ( Status )) {
-- 
2.7.4

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

[edk2] [PATCH 1/3] Maintainers.txt: List Tianocore Stewards

[Jordan Justen]

Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jordan Justen 
---
 Maintainers.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 9d1b9fb941..a5e2e0af46 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -34,6 +34,12 @@ T: git (mirror) - https://bitbucket.org/tianocore/edk2.git
 T: git (mirror) - http://git.code.sf.net/p/tianocore/edk2
 T: svn (read-only, deprecated) - https://svn.code.sf.net/p/edk2/code/trunk/edk2
 
+Tianocore Stewards
+--
+M: Andrew Fish 
+M: Leif Lindholm 
+M: Michael D Kinney 
+
 Responsible Disclosure, Reporting Security Issues
 -
 W: https://github.com/tianocore/tianocore.github.io/wiki/Security
-- 
2.14.0

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

[edk2] [PATCH 3/3] Maintainers.txt: Add Package Reviewer role

[Jordan Justen]

Although everyone is encouraged to review patches and add their
Reviewed-by reply for a patch, with the Package Reviewer role we
identify additional community members that will be Cc'd for patches
made to a package.

A distinction between a Package Maintainer and Reviewer is that
Maintainers will always have source control push access to the package
whereas Reviewers will not. (The Reviewer may have push access if they
are also Package Maintainer for another package.)

Currently we have an limit of 2 Package Maintainers per package, but
the Package Maintainers for each package decide how to manage the
Package Reviewer list.

Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jordan Justen 
---
 Maintainers.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 92a1fd974f..6317a392e7 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -5,7 +5,8 @@ This file provides information about the primary maintainers for
 EDK II.
 
 In general, you should not privately email the maintainer. You should
-email the edk2-devel list, but you can also Cc the maintainer.
+email the edk2-devel list, and Cc the package maintainers and
+reviewers.
 
 Descriptions of section entries:
 
@@ -13,6 +14,8 @@ Descriptions of section entries:
  Patches and questions should be sent to the email list.
   M: Package Maintainer: Cc address for patches and questions. Responsible
  for reviewing and pushing package changes to source control.
+  R: Package Reviewer: Cc address for patches and questions. Reviewers help
+ maintainers review code, but don't have push access.
   W: Web-page with status/info
   T: SCM tree type and location.  Type is one of: git, svn.
   S: Status, one of the following:
-- 
2.14.0

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

Re: [edk2] Does a double Page free report "ConvertPages: Incompatible memory types", maybe we could do better.

[Andrew Fish]

Also I forgot to mention the double page free was in the DXE Core LoadImage() 
on an error path. 


CoreLoadPeImage()
...
  return EFI_SUCCESS;

Done:

  //
  // Free memory.
  //
  if (DstBufAlocated) {
CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);
  }
...

CoreUnloadAndCloseImage()
...
  if ((Image->ImageBasePage != 0) && FreePage) {
CoreFreePages (Image->ImageBasePage, Image->NumberOfPages);
  }
...


I fixed by setting Image->ImageContext.ImageAddress and Image->ImageBasePage to 
0 after the free in CoreLoadPeImage().

Bug 667 

Thanks,

Andrew Fish


> On Aug 9, 2017, at 6:33 PM, Zeng, Star  wrote:
> 
> Andrew,
> 
> Another path may hit this DEBUG message is AllocatePages() by AllocateAddress 
> type.
> I think it is a good suggestion to enhance the DEBUG message. How about the 
> update like below?
> 
> -DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
> types\n"));
> +DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
> types, "));
> +if (Entry->Type == EfiConventionalMemory) {
> +  DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "the pages to free have been 
> freed\n"));
> +} else {
> +  DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "the pages to allocate have been 
> allocated\n"));
> +}
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Andrew 
> Fish
> Sent: Thursday, August 10, 2017 9:04 AM
> To: edk2-devel 
> Subject: [edk2] Does a double Page free report "ConvertPages: Incompatible 
> memory types", maybe we could do better.
> 
> It looks to me like if you Free pages, after you free pages you hit this 
> DEBUG message. 
> 
> 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Mem/Page.c#L790
> 
>  if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == 
> EfiConventionalMemory ? 1 : 0)) {
>DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
> types\n"));
>return EFI_NOT_FOUND;
>  }
> 
> I'm not sure I've thought out all the paths, but would it make more sense if 
> you are trying to convert EfiConventionalMemory to EfiConventionalMemory that 
> you are trying to free pages that are already freed. That is not very obvious 
> from the above DEBUG print.  Could there be an if in the error path to print 
> a better DEBUG message for a free pages bug? 
> 
> Also to be pedantic the function change names to: CoreConvertPagesEx(). 
> 
> Thanks,
> 
> 
> Andrew Fish
> 
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] [PATCH v2 1/1] OvmfPkg : QemuFwCfgLib: Use BusMasterCommandBuffer to map FW_CFG_DMA_ACCESS

[Laszlo Ersek]

On 08/10/17 20:36, Jordan Justen wrote:
> On 2017-08-04 13:25:09, Laszlo Ersek wrote:
>>>
>>> I have another reason why going beyond 80 is not a good idea for code
>>> that doesn't apply to normal reading. If you need ~120 columns visible
>>> to view some lines, then most lines will end up having a lot of wasted
>>> horizontal whitespace because they can commonly fit into 80 columns.
>>
>> One reason I dislike the "all arguments on separate lines" rule is just
>> this -- it wastes perfectly good horizontal space, and eats up precious
>> vertical space.
>>
>> We have a patch submission process for our documents now,
> 
> I think I agree about the multiple args/multiple lines issue. I guess
> you could try to submit a code style doc change, but I'm not sure how
> controversial it might be.
> 
>> but I don't
>> think maintainers of other Pkgs would bother with following and
>> enforcing a stricter 80 columns rule, even if we got it codified. :/
> 
> I still think we should have it written stricter even if it isn't
> always followed. It could be a step in the right direction.

OK, I'm tagging this for later... Not sure how much later ;)

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

Re: [edk2] [PATCH v1 03/14] OvmfPkg/VirtioPciDeviceDxe: Implement new member functions

[Laszlo Ersek]

On 08/10/17 20:41, Brijesh Singh wrote:
> Hi Laszlo,
> 
> On 08/09/2017 12:09 PM, Laszlo Ersek wrote:
> [Snip]
> 
>>
>>> +
>>> +EFI_STATUS
>>> +EFIAPI
>>> +VirtioPciUnmapSharedBuffer (
>>> +  VIRTIO_DEVICE_PROTOCOL*This,
>>> +  VOID  *Mapping
>>> +  )
>>> +{
>>> +  return EFI_SUCCESS;
>>> +}
>>>
>>
>> (9) Please refresh the function signatures in both "VirtioPciDevice.h"
>> and "VirtioPciFunctions.c", from the protocol definition in
>> "OvmfPkg/Include/Protocol/VirtioDevice.h".
>>
>> In particular, all the IN and OUT decoration is missing here.
> 
> I see that several functions defined in VirtioPciDevice.h is missing IN
> and OUT,

Ouch, you are right!

> do you want me to send a separate patch to fix that too ?

That would be very kind of you.

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

Re: [edk2] [PATCH v2 1/1] OvmfPkg : QemuFwCfgLib: Use BusMasterCommandBuffer to map FW_CFG_DMA_ACCESS

[Jordan Justen]

On 2017-08-04 13:25:09, Laszlo Ersek wrote:
> > 
> > I have another reason why going beyond 80 is not a good idea for code
> > that doesn't apply to normal reading. If you need ~120 columns visible
> > to view some lines, then most lines will end up having a lot of wasted
> > horizontal whitespace because they can commonly fit into 80 columns.
> 
> One reason I dislike the "all arguments on separate lines" rule is just
> this -- it wastes perfectly good horizontal space, and eats up precious
> vertical space.
> 
> We have a patch submission process for our documents now,

I think I agree about the multiple args/multiple lines issue. I guess
you could try to submit a code style doc change, but I'm not sure how
controversial it might be.

> but I don't
> think maintainers of other Pkgs would bother with following and
> enforcing a stricter 80 columns rule, even if we got it codified. :/

I still think we should have it written stricter even if it isn't
always followed. It could be a step in the right direction.

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

Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

[Andrew Fish]

> On Aug 10, 2017, at 3:38 AM, Gao, Liming  wrote:
> 
> Andrew:
>  If this is a mtoc bug, I suggest to update GenFw to always correct it in the 
> generated EFI image. If so, the EFI image is always correct. There is no 
> change requirement in PeCoff library in MdePkg.
> 

Liming,

EFI supports loading PE/COFF images that are not built at the same time as the 
platform firmware (UEFI Shell, OS loader), and that is why I added the fix to 
the PE/COFF library code. 

Thanks,

Andrew Fish

> Thanks
> Liming
> From: af...@apple.com  [mailto:af...@apple.com 
> ]
> Sent: Tuesday, August 8, 2017 12:26 AM
> To: Zhu, Yonghong >
> Cc: edk2-devel@lists.01.org ; Gao, Liming 
> >; Kinney, Michael D 
> >
> Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg 
> with XCODE5
> 
> Should that be:
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> I also noticed the PeCoff lib is going to loop and reload the .debug suction 
> due to this mtoc bug, so it would be good to harden that code too.
> 
> git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
> b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 8d1daba..1e4c67e 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
> }
> 
> return RETURN_SUCCESS;
> +  } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> +return RETURN_SUCCESS;
>   }
> }
>   }
> @@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
> if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
>   ImageContext->DebugDirectoryEntryRva = (UINT32) 
> (DebugDirectoryEntryRva + Index);
>   return RETURN_SUCCESS;
> +} else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
> +  return RETURN_SUCCESS;
> }
>   }
> }
> 
> 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=663
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> On Aug 6, 2017, at 9:00 PM, Yonghong Zhu   >> wrote:
> 
> it is a bug in mtoc setting the size of the debug directory entry to
> the size of the .debug section, not the size of the
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
> get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
> memset() and boom.
> 
> Cc: Liming Gao >
> Cc: Michael D Kinney 
> >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish >
> ---
> BaseTools/Source/C/GenFw/GenFw.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c 
> b/BaseTools/Source/C/GenFw/GenFw.c
> index 246deb0..af60c92 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2813,10 +2813,11 @@ Returns:
>  //
>  // Get Debug, Export and Resource EntryTable RVA address.
>  // Resource Directory entry need to review.
>  //
>  Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof 
> (EFI_IMAGE_FILE_HEADER));
> +  Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof 
> (EFI_IMAGE_FILE_HEADER));
>  if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr +  
> FileHdr->SizeOfOptionalHeader);
>if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT 
> && \
>Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 
> 0) {
>  ExportDirectoryEntryRva = 
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
> @@ -2833,11 +2834,10 @@ Returns:
>Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
>
> Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress 
> = 0;
>  }
>}
>  } else {
> -Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + 
> sizeof (EFI_IMAGE_FILE_HEADER));
>SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr +  
> FileHdr->SizeOfOptionalHeader);
>if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT 
> && \
>Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 
> 0) {
>  ExportDirectoryEntryRva = 
> 

Re: [edk2] [Patch 1/2] QuarkSocPkg/MemoryInit: Remove use of memset()/memcpy()

[Steele, Kelly]

Reviewed by: Kelly Steele 

-Original Message-
From: Kinney, Michael D 
Sent: August 09, 2017 12:40
To: edk2-devel@lists.01.org
Cc: Steele, Kelly ; Gao, Liming 
Subject: [Patch 1/2] QuarkSocPkg/MemoryInit: Remove use of memset()/memcpy()

Map the use of memset() and memcpy() to the BaseMemoryLib functions ZeroMem(), 
SetMem(), and CopyMem().  This fixes GCC build issues with this module.

With the remap of the functions, the [BuildOptions] MSFT CC_FLAGS to enable /Oi 
can also be removed, so the MSFT and GCC builds behave the same.

Cc: Kelly Steele 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf |  6 +-
 QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit_utils.h   | 10 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf 
b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
index 78821f59a3..05766133ed 100644
--- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
+++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
@@ -1,7 +1,7 @@
 ## @file
 # This is the Memory Initialization Driver for Quark  # -# Copyright (c) 
2013-2015 Intel Corporation.
+# Copyright (c) 2013-2017 Intel Corporation.
 #
 # This program and the accompanying materials  # are licensed and made 
available under the terms and conditions of the BSD License @@ -74,7 +74,3 @@
 
 [Depex]
   TRUE
-
-[BuildOptions]
-  # /Oi option to use the intrinsic memset function in source code.
-  MSFT:*_*_*_CC_FLAGS = /Oi
diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit_utils.h 
b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit_utils.h
index 04c59f5af0..dcc40c7782 100644
--- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit_utils.h
+++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit_utils.h
@@ -1,6 +1,6 @@
 /
  *
- * Copyright (c) 2013-2015 Intel Corporation.
+ * Copyright (c) 2013-2017 Intel Corporation.
  *
 * This program and the accompanying materials
 * are licensed and made available under the terms and conditions of the BSD 
License @@ -90,8 +90,12 @@ void restore_timings(MRCParams_t *mrc_params);  void 
default_timings(MRCParams_t *mrc_params);
 
 #ifndef SIM
-void *memset(void *d, int c, size_t n); -void *memcpy(void *d, const void *s, 
size_t n);
+//
+// Map memset() and memcpy() to BaseMemoryLib functions // #include 
+ #define memset(d,c,n) ((c) == 0) ? ZeroMem 
+((d), (n)) : SetMem ((d), (n), (c)) #define memcpy(d,s,n) CopyMem ((d), 
+(s), (n))
 #endif
 
 #endif // _MEMINIT_UTILS_H_
--
2.13.1.windows.2

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

Re: [edk2] [Patch 2/2] QuarkPlatformPkg/Readme.md: edk2-non-osi directory layout

[Steele, Kelly]

Reviewed by: Kelly Steele 

-Original Message-
From: Kinney, Michael D 
Sent: August 09, 2017 12:40
To: edk2-devel@lists.01.org
Cc: Leif Lindholm ; Steele, Kelly 

Subject: [Patch 2/2] QuarkPlatformPkg/Readme.md: edk2-non-osi directory layout

The following commit moved the QuarkSocBinPkg from the root directory to the 
the Silicon/Intel directory.

https://github.com/tianocore/edk2-non-osi/commit/182e85d04566800fe188de4b1c30a50533dd74b7

The following updates are made to Readme.md:

* PACKAGES_PATH setting for edk2-non-osi directory changes
* Remove use of edk2-FatPkg repository
* Remove use of edk2-BaseTools-win32 repository
* Run python build tools from sources

Cc: Leif Lindholm 
Cc: Kelly Steele 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 QuarkPlatformPkg/Readme.md | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/QuarkPlatformPkg/Readme.md b/QuarkPlatformPkg/Readme.md index 
f925f9ef27..aa9d9856bd 100644
--- a/QuarkPlatformPkg/Readme.md
+++ b/QuarkPlatformPkg/Readme.md
@@ -46,12 +46,12 @@
   - Install
 * ASL compiler: Available from http://www.acpica.org
   - Install into ```C:\ASL``` to match default tools_def.txt configuration.
+* Python 2.7: Available from http://www.python.org
 
 Create a new directory for an EDK II WORKSPACE.
 
 The code block below shows the GIT clone operations required to pull the EDK 
II -source tree, the FatPkg sources, the pre-built versions of BaseTools as 
WIN32 -binaries, and the edk2-non-osi repository that provides a binary file 
for the
+source tree and the edk2-non-osi repository that provides a binary file 
+for the
 Quark Remote Management Unit (RMU).
 
 Next it sets environment variables that must be set before running @@ -60,6 
+60,8 @@ the EDK II [Multiple Workspace](
 https://github.com/tianocore/tianocore.github.io/wiki/Multiple_Workspace)
 feature is used.
 
+Next, the EDK II BaseTools required to build firmware images are built.
+
 Next, the ```edksetup.bat``` file is run to complete the initialization of an  
EDK II build environment.  Two example build commands are shown.  The first one 
 in ```QuarkPlatformPlg/Quark.dsc``` builds a full UEFI firmware image that is 
@@ -69,16 +71,17 @@ image that is useful for initial power-on and debug of new 
features.
 
 ```cmd
 git clone https://github.com/tianocore/edk2.git
-git clone https://github.com/tianocore/edk2-FatPkg.git FatPkg -git clone 
https://github.com/tianocore/edk2-BaseTools-win32.git
 git clone https://github.com/tianocore/edk2-non-osi.git
 
+set PYTHON_HOME=c:\Python27
 set WORKSPACE=%CD%
-set PACKAGES_PATH=%WORKSPACE%\edk2;%WORKSPACE%\edk2-non-osi
-set EDK_TOOLS_BIN=%WORKSPACE%\edk2-BaseTools-win32
+set 
+PACKAGES_PATH=%WORKSPACE%\edk2;%WORKSPACE%\edk2-non-osi\Silicon\Intel
+set EDK_TOOLS_PATH=%WORKSPACE%\edk2\BaseTools
+cd %WORKSPACE%\edk2
 
-cd edk2
-edksetup.bat
+BaseTools\toolsetup.bat Rebuild
+
+edksetup.bat Rebuild
 
 build -a IA32 -t VS2015x86 -p QuarkPlatformPkg/Quark.dsc  build -a IA32 -t 
VS2015x86 -p QuarkPlatformPkg/QuarkMin.dsc @@ -91,12 +94,13 @@ build -a IA32 -t 
VS2015x86 -p QuarkPlatformPkg/QuarkMin.dsc
 * GIT client
 * GCC 4.9 compiler
 * ASL compiler: Available from http://www.acpica.org.
+* Python 2.7
 
 Create a new directory for an EDK II WORKSPACE.
 
 The code block below shows the GIT clone operations required to pull the EDK 
II -source tree, the FatPkg sources, and the edk2-non-osi repository that 
provides a -binary file for the Quark Remote Management Unit (RMU).
+source tree and the edk2-non-osi repository that provides a binary file 
+for the Quark Remote Management Unit (RMU).
 
 Next it sets environment variables that must be set before running  
```edksetup.bat```. Since content is being pulled from multiple repositories, 
@@ -106,7 +110,7 @@ feature is used.
 
 Next, the EDK II BaseTools required to build firmware images are built.
 
-Next, the ```edksetup.bat``` file is run to complete the initialization of an
+Next, the ```edksetup.sh``` file is run to complete the initialization 
+of an
 EDK II build environment.  Two example build commands are shown.  The first 
one  in ```QuarkPlatformPlg/Quark.dsc``` builds a full UEFI firmware image that 
is  able to boot the built-in UEFI Shell and Linux from a micro SD FLASH card.  
The @@ -115,17 +119,15 @@ image that is useful for initial power-on and debug 
of new features.
 
 ```sh
 git clone https://github.com/tianocore/edk2.git
-git clone https://github.com/tianocore/edk2-FatPkg.git FatPkg  git clone 
https://github.com/tianocore/edk2-non-osi.git
 
 export WORKSPACE=$PWD
-export PACKAGES_PATH=$WORKSPACE/edk2:$WORKSPACE/edk2-non-osi
+export 
+PACKAGES_PATH=$WORKSPACE/edk2:$WORKSPACE/edk2-non-osi/Silicon/Intel
 export 

Re: [edk2] [Patch 0/2] Update Quark for edk2-non-osi changes

[Steele, Kelly]

Reviewed by: Kelly Steele 

-Original Message-
From: Kinney, Michael D 
Sent: August 09, 2017 12:40
To: edk2-devel@lists.01.org
Cc: Leif Lindholm ; Steele, Kelly 
; Gao, Liming 
Subject: [Patch 0/2] Update Quark for edk2-non-osi changes

Update PACKAGES_PATH requirements Readme.md to match the new directory 
structure in the edk2-non-osi repository and update instructions to run 
python-based build tools from sources and remove edk2-FatPkg repository.

Also fix GCC compatibility issues from use of memset() in the MemoryInit module 
in the QuarkSocPkg that were discovered when verifying the Linux/GCC build 
instructions.

Cc: Leif Lindholm 
Cc: Kelly Steele 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 

Michael D Kinney (2):
  QuarkSocPkg/MemoryInit: Remove use of memset()/memcpy()
  QuarkPlatformPkg/Readme.md: edk2-non-osi directory layout

 QuarkPlatformPkg/Readme.md | 34 --
 .../MemoryInit/Pei/MemoryInitPei.inf   |  6 +---
 .../MemoryInit/Pei/meminit_utils.h | 10 +--
 3 files changed, 26 insertions(+), 24 deletions(-)

--
2.13.1.windows.2

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

Re: [edk2] [PATCH 4/4] Platforms/zx: Add platform build system files

[Leif Lindholm]

On Wed, Aug 09, 2017 at 10:12:39PM +0800, Jun Nie wrote:
> Add platform build system files, including *.dsc *.fdf *.dec
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie 
> ---
>  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec |  33 ++
>  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc | 467 
> +++
>  Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf | 331 +++
>  3 files changed, 831 insertions(+)
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
> 
> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec 
> b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> new file mode 100644
> index 000..078cfdf
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> @@ -0,0 +1,33 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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.
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION  = 0x00010005

0x00010019/1.25.

> +  PACKAGE_NAME   = Zx296718Evb
> +  PACKAGE_GUID   = d6db414d-ea67-4312-94d5-9c9e5b224d25
> +  PACKAGE_VERSION= 0.1
> +
> +
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#   Comments are used for Keywords and Module Types.
> +#
> +# Supported Module Types:
> +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER 
> DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +#
> +
> +[Includes.common]
> +  Include# Root include for the package

This directory does not exist, breaking compilation.

> +
> +[Guids.common]
> +  gZx296718EvbTokenSpaceGuid  =  { 0x91148425, 0xcdd2, 0x4830, { 
> 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }

This does not appear to actually be used anywhere.

> diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc 
> b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
> new file mode 100644
> index 000..d104e7c
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
> @@ -0,0 +1,467 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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.
> +#
> +
> +
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +
> +[Defines]
> +  PLATFORM_NAME  = Zx296718Evb
> +  PLATFORM_GUID  = 8edf1480-da5c-4857-bc02-7530bd8e7b7a
> +  PLATFORM_VERSION   = 0.2
> +  DSC_SPECIFICATION  = 0x00010005
> +  OUTPUT_DIRECTORY   = Build/Zx296718Evb
> +  SUPPORTED_ARCHITECTURES= AARCH64
> +  BUILD_TARGETS  = DEBUG|RELEASE
> +  SKUID_IDENTIFIER   = DEFAULT
> +  FLASH_DEFINITION   = 
> Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
> +
> +[LibraryClasses.common]
> +!if $(TARGET) == RELEASE
> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> +!else
> +  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> +!endif
> +  
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> +
> +  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
> +  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> +  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> +  
> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> +
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  

Re: [edk2] [PATCH 3/4] Platforms/zx: Add boot manager lib and entries

[Leif Lindholm]

On Wed, Aug 09, 2017 at 10:12:38PM +0800, Jun Nie wrote:
> Add boot manager lib and entries, including Android and Grub.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie 
> ---
>  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c| 105 ++
>  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf  |  66 
>  .../Library/PlatformBootManagerLib/PlatformBm.c| 404 
> +
>  .../Library/PlatformBootManagerLib/PlatformBm.h|  30 ++
>  .../PlatformBootManagerLib.inf |  91 +
>  Silicon/Sanchip/SanchipPkg.dec |  29 ++
>  6 files changed, 725 insertions(+)
>  create mode 100644 
> Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
>  create mode 100644 
> Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>  create mode 100644 
> Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
>  create mode 100644 
> Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
>  create mode 100644 
> Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 Silicon/Sanchip/SanchipPkg.dec
> 
> diff --git 
> a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c 
> b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> new file mode 100644
> index 000..47d02bf
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> @@ -0,0 +1,105 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 

Which aspect of BlockIo is used here?
On the whole, several of the above includes look unused - could you
prune them back a bit?

> +
> +#include 
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgAppendKernelArgs (
> +  IN CHAR16*Args,
> +  IN UINTN  Size
> +  )
> +{
> +  UnicodeSPrint (
> +Args + StrLen (Args), Size - StrLen (Args), L" efi=noruntime");

I am not sure what your intent is here, but I am entirely convinced
there is a better way to achieve it. Can you give some background,
please?

> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgUpdateDtb (
> +  IN  EFI_PHYSICAL_ADDRESSOrigFdtBase,
> +  OUT EFI_PHYSICAL_ADDRESS   *NewFdtBase
> +  )
> +{
> +  UINTN FdtSize, NumPages;
> +  INTN  err;
> +  EFI_STATUSStatus;
> +
> +  //
> +  // Store the FDT as Runtime Service Data to prevent the Kernel from
> +  // overwritting its data.
> +  //

This should really not be necessary and has never been needed
elsewhere. Have you seen this cause any change in behaviour in an
actual system?

> +  FdtSize = fdt_totalsize ((VOID *)(UINTN)OrigFdtBase);
> +  NumPages = EFI_SIZE_TO_PAGES (FdtSize) + 20;
> +  Status = gBS->AllocatePages (
> +  AllocateAnyPages, EfiRuntimeServicesData,
> +  NumPages, NewFdtBase);
> +  if (EFI_ERROR (Status)) {
> +return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  CopyMem (
> +(VOID*)(UINTN)*NewFdtBase,
> +(VOID*)(UINTN)OrigFdtBase,
> +FdtSize
> +);
> +
> +  fdt_pack ((VOID*)(UINTN)*NewFdtBase);
> +  err = fdt_check_header ((VOID*)(UINTN)*NewFdtBase);

This does not feel like it belongs here. Check it when you first load
it, by all means, but there is no need to keep checking it did not get
destroyed by normal execution.

> +  if (err != 0) {
> +DEBUG ((DEBUG_ERROR, "ERROR: Device Tree header not valid (err:%d)\n", 
> err));
> +gBS->FreePages (*NewFdtBase, NumPages);
> +return EFI_INVALID_PARAMETER;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +ANDROID_BOOTIMG_PROTOCOL mAndroidBootImg = {
> +  AndroidBootImgAppendKernelArgs,
> +  AndroidBootImgUpdateDtb
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +Zx296718EvbEntryPoint (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS   Status;
> +
> +  Status = gBS->InstallProtocolInterface (
> +  ,
> +  ,
> +  EFI_NATIVE_INTERFACE,
> +  
> +  );
> +  return Status;
> +}
> diff --git 
> a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf 
> b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> new file mode 

Re: [edk2] [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

[Laszlo Ersek]

Thanks for the CC

On 08/10/17 15:04, Leif Lindholm wrote:
> You've reworked this to be targeted for edk2-platforms, which is
> excellent! However:
> - Please update subject line to match.
> - Please also cc edk2-devel@lists.01.org (since you are with Linaro,
>   it makes sense to still cc linaro-uefi)
> 
> To get the appropriate subject line for edk2-devel, as desribed by
> https://github.com/tianocore/edk2-platforms/blob/about/Readme.md it
> would also be helpful to add
> --subject-prefix="edk2-platforms/master][PATCH" to git format-patch
> command line (unless Laszlo can point out a better way of achieving
> the same).

I seem to recall a discussion (not necessarily related to the
edk2-platforms tree, but to another edk2-related tree, still posted to
edk2-devel) that we're fine if you put the tree identified right after
the PATCH word, between the same brackets. So just use

  --subject-prefix="PATCH edk2-platforms/master"

or even (if "master" is quite obvious)

  --subject-prefix="PATCH edk2-platforms"

> 
> On Wed, Aug 09, 2017 at 10:12:36PM +0800, Jun Nie wrote:
>> Add Sanchip Zx296718 basic library files for Zx296718 SoC
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie 
>> ---
>>  .../Library/Zx296718EvbLib/Zx296718Evb.c   | 132 
>> +
>>  .../Library/Zx296718EvbLib/Zx296718EvbHelper.S |  50 
>>  .../Library/Zx296718EvbLib/Zx296718EvbLib.inf  |  53 +
>>  .../Library/Zx296718EvbLib/Zx296718EvbMem.c| 107 +
>>  Silicon/Sanchip/Zx296718/Include/Zx296718.h|  30 +
>>  Silicon/Sanchip/Zx296718/Zx296718.dec  |  32 +
> 
> It is more clear (especially when submitting new ports) to generate
> the patches with --stat=1000, to make the full file paths visible.

Yes, please -- I recommend:

  --stat=1000 --stat-graph-width=20

("--stat=1000" without "--stat-graph-width=20" won't be pleasant.)

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

Re: [edk2] [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC

[Leif Lindholm]

On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote:
> Runtime service is not supported yet.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie 
> ---
>  .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.c | 376 
> +
>  .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.h | 102 ++
>  .../Zx296718RealTimeClock.inf  |  42 +++
>  3 files changed, 520 insertions(+)
>  create mode 100644 
> Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
>  create mode 100644 
> Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
>  create mode 100644 
> Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf
> 
> diff --git 
> a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c 
> b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> new file mode 100644
> index 000..af6e5bd
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
> @@ -0,0 +1,376 @@
> +/** @file
> +  Implement EFI RealTimeClock runtime services via RTC Lib.
> +
> +  Currently this driver does not support runtime virtual calling.
> +
> +  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +  Copyright (c) 2017, Linaro Limited.
> +
> +  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.
> +
> +  Based on the files under 
> ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
> +
> +**/
> +
> +#include 
> +#include 

P before U.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +// Use EfiAtRuntime to check stage
> +#include 

L (UefiRuntimeLib) before S (UefiRuntimeServices...).
No need for a comment explaining why we include headers.

> +#include 
> +#include "Zx296718RealTimeClock.h"
> +
> +STATIC UINTN   RtcBase;

mRtcBase.

> +STATIC BOOLEAN RTCInitialized = FALSE;

mRTCInitialized.

> +
> +BOOLEAN
> +EFIAPI
> +IsTimeValid(
> +  IN EFI_TIME *Time
> +  )
> +{
> +  // Check the input parameters are within the range specified by UEFI
> +  if ((Time->Year  < 2000) ||
> + (Time->Year   > 2099) ||
> + (Time->Month  < 1   ) ||
> + (Time->Month  > 12  ) ||
> + (Time->Day< 1   ) ||
> + (Time->Day> 31  ) ||
> + (Time->Hour   > 23  ) ||
> + (Time->Minute > 59  ) ||
> + (Time->Second > 59  ) ||
> + (Time->Nanosecond > 9) ||
> + (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= 
> -1440) && (Time->TimeZone <= 1440 ||
> + (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT)))
> +  ) {
> +return FALSE;
> +  }
> +
> +  return TRUE;
> +}

This appears a direct copy of the version in
EmbeddedPkg/Library/TimeBaseLib. Can you add that TimeBaseLib library
dependency to decrease duplication?
(This may not have existed as a standalone library at the time you
started this port.)

> +
> +VOID

A lot of the functions in this file could do with a STATIC prefix,
since they are only used internally.

> +Wait4Busy (

Semantically, this name is incorrect.
The function is waiting _while_ the state is RTC_BUSY, so seems to me
it should be called WaitBusy.

> +  VOID
> +  )
> +{
> +  UINT32 Val, Retry = 1000;
> +  do {
> +MicroSecondDelay (200);

Why 200?

> +Val = MmioRead32 (RtcBase + RTCSTS);

MmioRead32 does not imply any barrier semantics.
If this component will only ever be supported on ARM platforms, you
could insert an ArmDataMemoryBarrier (). Otherwise, MemoryFence ().

> +  } while(Val & RTC_BUSY && Retry--);

Space before (.

> +
> +  if (!Retry)
> +DEBUG((DEBUG_ERROR, "%a Rtc busy retry timeout\n", __func__));

Space after DEBUG.

> +}
> +
> +VOID
> +RTCWriteReg (
> +  IN UINT32 Reg,
> +  IN UINT32 Val
> +  )
> +{
> +  Wait4Busy ();
> +  MmioWrite32 (RtcBase + RTCCFGID, CONFIG_PARMETER);
> +  Wait4Busy ();
> +  MmioWrite32 (RtcBase + Reg, Val);
> +}
> +
> +UINT32
> +RTCReadReg (
> +  IN UINT32 Reg
> +  )
> +{
> +  Wait4Busy ();
> +  return MmioRead32 (RtcBase + Reg);
> +}
> +
> +VOID
> +InitializeRTC (
> +  VOID
> +  )
> +{
> +  UINTN Val = (UINTN)FixedPcdGet64 (PcdZxRtcClockFreq);
> +
> +  RTCWriteReg (RTCCLKCNT, Val - 1);
> +  Val = RTCReadReg (RTCPOWERINIT1);
> +  if (RTC_POWER_INI1_PARA != Val) {
> +RTCWriteReg (RTCCTL, 0);
> +MicroSecondDelay (INIT_DELAY);
> +Val = RTCReadReg (RTCCTL);
> +Val |= RTC_CTRL_BIT6_1;
> +RTCWriteReg (RTCCTL, Val);
> +Val = RTCReadReg (RTCCTL);
> +Val &= RTC_CTRL_MODULE24 | 

Re: [edk2] Multiple Device ID support in EfiRom

[Gao, Liming]

This patch is not integrated into edk2 trunk. Could you file this issue in 
bugzillar (https://bugzilla.tianocore.org/) and attach this patch? We will add 
it into edk2 trunk. 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tomas 
> Pilar (tpilar)
> Sent: Monday, August 7, 2017 10:19 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Multiple Device ID support in EfiRom
> 
> Hi,
> 
> I am looking at this patch (attached) from Daniel:
> 
> --
> 
> Hello,
> 
> Attached is a patch to implement writing and dumping of PCI 3.0 Device
> ID lists in EFI option ROMs in the EfiRom tool.
> 
> Using this modification, multiple space-delimited device IDs can be
> specified after -i.  The first device in the list is used for the main
> PCI ROM header Device ID field and is also written in the list.  The
> list is only written when more than one device ID has been specified;
> when only one device ID is given on the command line, the EfiRom output
> should be identical to the current code.
> 
> Let me know if there's anything I missed.
> 
> Thanks,
> -- Daniel Verkamp
> 
> --
> 
> Was this patch ever considered for inclusion in the edk2 trunk? If not,
> what is the current state of multiple device id support for rom creation?
> 
> Cheers,
> Tom
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

[Leif Lindholm]

You've reworked this to be targeted for edk2-platforms, which is
excellent! However:
- Please update subject line to match.
- Please also cc edk2-devel@lists.01.org (since you are with Linaro,
  it makes sense to still cc linaro-uefi)

To get the appropriate subject line for edk2-devel, as desribed by
https://github.com/tianocore/edk2-platforms/blob/about/Readme.md it
would also be helpful to add
--subject-prefix="edk2-platforms/master][PATCH" to git format-patch
command line (unless Laszlo can point out a better way of achieving
the same).

On Wed, Aug 09, 2017 at 10:12:36PM +0800, Jun Nie wrote:
> Add Sanchip Zx296718 basic library files for Zx296718 SoC
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie 
> ---
>  .../Library/Zx296718EvbLib/Zx296718Evb.c   | 132 
> +
>  .../Library/Zx296718EvbLib/Zx296718EvbHelper.S |  50 
>  .../Library/Zx296718EvbLib/Zx296718EvbLib.inf  |  53 +
>  .../Library/Zx296718EvbLib/Zx296718EvbMem.c| 107 +
>  Silicon/Sanchip/Zx296718/Include/Zx296718.h|  30 +
>  Silicon/Sanchip/Zx296718/Zx296718.dec  |  32 +

It is more clear (especially when submitting new ports) to generate
the patches with --stat=1000, to make the full file paths visible.

See Laszlo's guide for helpful steps when submitting patches:
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

>  6 files changed, 404 insertions(+)
>  create mode 100644 
> Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
>  create mode 100644 
> Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbHelper.S
>  create mode 100644 
> Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
>  create mode 100644 
> Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbMem.c
>  create mode 100644 Silicon/Sanchip/Zx296718/Include/Zx296718.h
>  create mode 100644 Silicon/Sanchip/Zx296718/Zx296718.dec

Since this ends up with a .dec, please rename
Silicon/Sanchip/Zx296718/ -> Silicon/Sanchip/Zx296718Pkg/.

That said, I don't see this .dec file providing any information that
is subsequently used enywhere. Is this in preparation for further
patches coming after this initial platform support?

> diff --git 
> a/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c 
> b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
> new file mode 100644
> index 000..4e4eb54
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
> @@ -0,0 +1,132 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.

Just a clarification: Sanchip/Sanechips?
"The Internet" suggests Sanechips is the appropriate company name, so
should the directory name change?

> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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 
> +#include 

Please sort include files alphabetically.

> +
> +#include 
> +
> +#include 
> +
> +ARM_CORE_INFO mZx296718EvbInfoTable[] = {
> +  {
> +// Cluster 0, Core 0
> +0x0, 0x0,
> +
> +// MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +(UINT64)0x
> +  },
> +  {
> +// Cluster 0, Core 1
> +0x0, 0x1,
> +
> +// MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +(UINT64)0x
> +  },
> +  {
> +// Cluster 0, Core 2
> +0x0, 0x2,
> +
> +// MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +(UINT64)0x
> +  },
> +  {
> +// Cluster 0, Core 3
> +0x0, 0x3,
> +
> +// MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +(UINT64)0x
> +  },
> +};
> +
> +/**
> +  Return the current Boot Mode
> +
> +  This function returns the boot reason on the platform
> +
> +  @return   Return the current Boot Mode of the platform
> +
> +**/
> +EFI_BOOT_MODE
> +ArmPlatformGetBootMode (
> +  VOID
> +  )
> +{
> +  return BOOT_WITH_FULL_CONFIGURATION;
> +}
> +
> +/**
> +  Initialize controllers that must setup in the normal world
> +
> +  This function is called by the ArmPlatformPkg/Pei or 
> ArmPlatformPkg/Pei/PlatformPeim
> +  in the PEI phase.
> +
> +**/
> +RETURN_STATUS
> +ArmPlatformInitialize (
> +  IN  UINTN MpId
> +  )
> +{
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  Initialize the system (or sometimes called permanent) memory
> +
> +  This memory is generally represented by the DRAM.
> +
> 

Re: [edk2] [PATCH v6 2/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Leif Lindholm]

On Wed, Aug 09, 2017 at 06:39:23PM +0800, Jun Nie wrote:
> Add an android kernel loader that could load kernel from storage
> device.
> This android boot image BDS add addtitional cmdline/dtb/ramfs
> support besides kernel that is introduced by Android boot header.
> 
> This patch is derived from Haojian's code as below link.
> https://patches.linaro.org/patch/94683/
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie 
> ---
>  .../Application/AndroidBoot/AndroidBootApp.c   | 140 ++
>  .../Application/AndroidBoot/AndroidBootApp.inf |  64 +++
>  EmbeddedPkg/EmbeddedPkg.dec|   2 +
>  EmbeddedPkg/EmbeddedPkg.dsc|   2 +
>  EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  13 +
>  EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 ++
>  .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 471 
> +
>  .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
>  8 files changed, 787 insertions(+)
>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>  create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
>  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
>  create mode 100644 
> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> 

> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c 
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> new file mode 100644
> index 000..cb4fb67
> --- /dev/null
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -0,0 +1,471 @@
> +/** @file
> +
> +  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
> +  Copyright (c) 2017, Linaro. 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 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include 
> +
> +#define FDT_ADDITIONAL_ENTRIES_SIZE 0x400
> +
> +typedef struct {
> +  MEMMAP_DEVICE_PATH  Node1;
> +  EFI_DEVICE_PATH_PROTOCOLEnd;
> +} MEMORY_DEVICE_PATH;
> +
> +STATIC ANDROID_BOOTIMG_PROTOCOL *mAndroidBootImg;
> +
> +STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate =
> +{
> +  {
> +{
> +  HARDWARE_DEVICE_PATH,
> +  HW_MEMMAP_DP,
> +  {
> +(UINT8)(sizeof (MEMMAP_DEVICE_PATH)),
> +(UINT8)((sizeof (MEMMAP_DEVICE_PATH)) >> 8),
> +  },
> +}, // Header
> +0, // StartingAddress (set at runtime)
> +0  // EndingAddress   (set at runtime)
> +  }, // Node1
> +  {
> +END_DEVICE_PATH_TYPE,
> +END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +{ sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
> +  } // End
> +};
> +
> +EFI_STATUS
> +AndroidBootImgGetImgSize (
> +  IN  VOID*BootImg,
> +  OUT UINTN   *ImgSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +ANDROID_BOOT_MAGIC_LENGTH) != 0) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  /* The page size is not specified, but it should be power of 2 at least */
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
> +
> +  /* Get real size of abootimg */
> +  *ImgSize = ALIGN_VALUE (Header->KernelSize, Header->PageSize) +
> + ALIGN_VALUE (Header->RamdiskSize, Header->PageSize) +
> + ALIGN_VALUE (Header->SecondStageBootloaderSize, 
> Header->PageSize) +
> + Header->PageSize;
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AndroidBootImgGetKernelInfo (
> +  IN  VOID*BootImg,
> +  OUT VOID   **Kernel,
> +  OUT UINTN   *KernelSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +ANDROID_BOOT_MAGIC_LENGTH) != 0) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Header->KernelSize == 0) {
> +return EFI_NOT_FOUND;
> +  }
> +
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
> +
> +  *KernelSize = Header->KernelSize;
> +  *Kernel = BootImg + Header->PageSize;
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AndroidBootImgGetRamdiskInfo (
> +  IN  VOID*BootImg,
> +  OUT VOID   **Ramdisk,
> +  OUT UINTN   *RamdiskSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +  UINT8  

Re: [edk2] [Patch] NetworkPkg/HttpBootDxe: Refine the coding style.

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, August 08, 2017 3:25 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Bi, Dandan ; Wu, Jiaxin 

Subject: [Patch] NetworkPkg/HttpBootDxe: Refine the coding style.

Cc: Ye Ting 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/HttpBootDxe/HttpBootSupport.c | 2 +-  
NetworkPkg/HttpBootDxe/HttpBootSupport.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c 
b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index 4e78cf3..d508e2c 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -1329,11 +1329,11 @@ HttpBootRegisterRamDisk (
 
 **/
 BOOLEAN
 HttpBootIsHttpRedirectStatusCode (
   IN   EFI_HTTP_STATUS_CODEStatusCode
- )
+  )
 {
   if (StatusCode == HTTP_STATUS_301_MOVED_PERMANENTLY ||
   StatusCode == HTTP_STATUS_302_FOUND ||
   StatusCode == HTTP_STATUS_307_TEMPORARY_REDIRECT ||
   StatusCode == HTTP_STATUS_308_PERMANENT_REDIRECT) { diff --git 
a/NetworkPkg/HttpBootDxe/HttpBootSupport.h 
b/NetworkPkg/HttpBootDxe/HttpBootSupport.h
index c10b2cf..9b7acd9 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.h
@@ -455,7 +455,7 @@ HttpBootRegisterRamDisk (
 
 **/
 BOOLEAN
 HttpBootIsHttpRedirectStatusCode (
   IN   EFI_HTTP_STATUS_CODEStatusCode
- );
+  );
 #endif
--
1.9.5.msysgit.1

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

Re: [edk2] [Patch V2 3/4] IntelFrameworkModulePkg: Fix Xcode 9 Beta treating 32-bit left shift as undefined

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Zhu, Yonghong
> Sent: Thursday, August 3, 2017 5:00 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Kinney, Michael D 
> ; Fan, Jeff ; Andrew
> Fish 
> Subject: [Patch V2 3/4] IntelFrameworkModulePkg: Fix Xcode 9 Beta treating 
> 32-bit left shift as undefined
> 
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=635
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Jeff Fan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish 
> ---
>  .../BaseUefiTianoCustomDecompressLib.c  | 6 
> +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git 
> a/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
> b/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
> index e0ba053..5d64f02 100644
> --- 
> a/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
> +++ 
> b/IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.c
> @@ -28,18 +28,18 @@ FillBuf (
>)
>  {
>//
>// Left shift NumOfBits of bits in advance
>//
> -  Sd->mBitBuf = (UINT32) (Sd->mBitBuf << NumOfBits);
> +  Sd->mBitBuf = (UINT32) LShiftU64 (((UINT64)Sd->mBitBuf), NumOfBits);
> 
>//
>// Copy data needed in bytes into mSbuBitBuf
>//
>while (NumOfBits > Sd->mBitCount) {
> -
> -Sd->mBitBuf |= (UINT32) (Sd->mSubBitBuf << (NumOfBits = (UINT16) 
> (NumOfBits - Sd->mBitCount)));
> +NumOfBits = (UINT16) (NumOfBits - Sd->mBitCount);
> +Sd->mBitBuf |= (UINT32) LShiftU64 (((UINT64)Sd->mSubBitBuf), NumOfBits);
> 
>  if (Sd->mCompSize > 0) {
>//
>// Get 1 byte into SubBitBuf
>//
> --
> 2.6.1.windows.1

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

Re: [edk2] [Patch V2 4/4] MdePkg: Fix Xcode 9 Beta treating 32-bit left shift as undefined

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Zhu, Yonghong
> Sent: Thursday, August 3, 2017 5:00 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Kinney, Michael D 
> ; Andrew Fish 
> Subject: [Patch V2 4/4] MdePkg: Fix Xcode 9 Beta treating 32-bit left shift 
> as undefined
> 
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=635
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish 
> ---
>  MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
> b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
> index e3b2846..e818543 100644
> --- a/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
> +++ b/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.c
> @@ -38,18 +38,18 @@ FillBuf (
>)
>  {
>//
>// Left shift NumOfBits of bits in advance
>//
> -  Sd->mBitBuf = (UINT32) (Sd->mBitBuf << NumOfBits);
> +  Sd->mBitBuf = (UINT32) LShiftU64 (((UINT64)Sd->mBitBuf), NumOfBits);
> 
>//
>// Copy data needed in bytes into mSbuBitBuf
>//
>while (NumOfBits > Sd->mBitCount) {
> -
> -Sd->mBitBuf |= (UINT32) (Sd->mSubBitBuf << (NumOfBits = (UINT16) 
> (NumOfBits - Sd->mBitCount)));
> +NumOfBits = (UINT16) (NumOfBits - Sd->mBitCount);
> +Sd->mBitBuf |= (UINT32) LShiftU64 (((UINT64)Sd->mSubBitBuf), NumOfBits);
> 
>  if (Sd->mCompSize > 0) {
>//
>// Get 1 byte into SubBitBuf
>//
> --
> 2.6.1.windows.1

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

Re: [edk2] [PATCH v3 1/2] MdePkg/BaseLib: Add IsNodeInList() function.

[Gao, Liming]

Marvin:
  I have no other comments. Reviewed-by: Liming Gao 

Thanks
Liming
> -Original Message-
> From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> Sent: Friday, August 4, 2017 3:52 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming 
> 
> Subject: [PATCH v3 1/2] MdePkg/BaseLib: Add IsNodeInList() function.
> 
> This patch adds IsNodeInList() to BaseLib, which verifies the given
> Node is part of the doubly-linked List provided.
> 
> V2:
>   - Rename "List" to "FirstEntry" and "Node" to "SecondEntry" to clarify that
> "FirstEntry" does not need to be the doubly-linked list's head node.
> 
> V3:
>   - Remove ASSERTs from IsNodeInList() which are present in
> InternalBaseLibIsListValid().
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser 
> ---
>  MdePkg/Library/BaseLib/LinkedList.c   | 66 +++-
>  MdePkg/Include/Library/BaseLib.h  | 27 
>  MdePkg/Library/BaseLib/BaseLibInternals.h | 28 -
>  3 files changed, 92 insertions(+), 29 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/LinkedList.c 
> b/MdePkg/Library/BaseLib/LinkedList.c
> index ba373f4b7be3..af27b0dd9a5c 100644
> --- a/MdePkg/Library/BaseLib/LinkedList.c
> +++ b/MdePkg/Library/BaseLib/LinkedList.c
> @@ -1,7 +1,7 @@
>  /** @file
>Linked List Library Functions.
> 
> -  Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
> +  Copyright (c) 2006 - 2017, 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
> @@ -113,6 +113,70 @@ InternalBaseLibIsNodeInList (
>  }
> 
>  /**
> +  Checks whether FirstEntry and SecondEntry are part of the same 
> doubly-linked
> +  list.
> +
> +  If FirstEntry is NULL, then ASSERT().
> +  If FirstEntry->ForwardLink is NULL, then ASSERT().
> +  If FirstEntry->BackLink is NULL, then ASSERT().
> +  If SecondEntry is NULL, then ASSERT();
> +  If PcdMaximumLinkedListLength is not zero, and List contains more than
> +  PcdMaximumLinkedListLength nodes, then ASSERT().
> +
> +  @param  FirstEntry   A pointer to a node in a linked list.
> +  @param  SecondEntry  A pointer to the node to locate.
> +
> +  @retval TRUE   SecondEntry is in the same doubly-linked list as FirstEntry.
> +  @retval FALSE  SecondEntry isn't in the same doubly-linked list as 
> FirstEntry,
> + or FirstEntry is invalid.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +IsNodeInList (
> +  IN  CONST LIST_ENTRY  *FirstEntry,
> +  IN  CONST LIST_ENTRY  *SecondEntry
> +  )
> +{
> +  UINTN Count;
> +  CONST LIST_ENTRY  *Ptr;
> +
> +  //
> +  // ASSERT List not too long
> +  //
> +  ASSERT (InternalBaseLibIsListValid (FirstEntry));
> +
> +  ASSERT (SecondEntry != NULL);
> +
> +  Count = 0;
> +  Ptr   = FirstEntry;
> +
> +  //
> +  // Check to see if SecondEntry is a member of FirstEntry.
> +  // Exit early if the number of nodes in List >= PcdMaximumLinkedListLength
> +  //
> +  do {
> +Ptr = Ptr->ForwardLink;
> +if (PcdGet32 (PcdMaximumLinkedListLength) > 0) {
> +  Count++;
> +
> +  //
> +  // Return if the linked list is too long
> +  //
> +  if (Count == PcdGet32 (PcdMaximumLinkedListLength)) {
> +return (BOOLEAN)(Ptr == SecondEntry);
> +  }
> +}
> +
> +if (Ptr == SecondEntry) {
> +  return TRUE;
> +}
> +  } while (Ptr != FirstEntry);
> +
> +  return FALSE;
> +}
> +
> +/**
>Initializes the head node of a doubly-linked list, and returns the pointer 
> to
>the head node of the doubly-linked list.
> 
> diff --git a/MdePkg/Include/Library/BaseLib.h 
> b/MdePkg/Include/Library/BaseLib.h
> index 791849b80406..40b96b761a85 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -2869,6 +2869,33 @@ PathCleanUpDirectories(
> 
> 
>  /**
> +  Checks whether FirstEntry and SecondEntry are part of the same 
> doubly-linked
> +  list.
> +
> +  If FirstEntry is NULL, then ASSERT().
> +  If FirstEntry->ForwardLink is NULL, then ASSERT().
> +  If FirstEntry->BackLink is NULL, then ASSERT().
> +  If SecondEntry is NULL, then ASSERT();
> +  If PcdMaximumLinkedListLength is not zero, and List contains more than
> +  PcdMaximumLinkedListLength nodes, then ASSERT().
> +
> +  @param  FirstEntry   A pointer to a node in a linked list.
> +  @param  SecondEntry  A pointer to the node to locate.
> +
> +  @retval TRUE   SecondEntry is in the same doubly-linked list as FirstEntry.
> +  @retval FALSE  SecondEntry isn't in the same doubly-linked list as 
> FirstEntry,
> + or FirstEntry is invalid.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +IsNodeInList (
> +  IN  CONST LIST_ENTRY  

Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

[Gao, Liming]

Andrew:
  If this is a mtoc bug, I suggest to update GenFw to always correct it in the 
generated EFI image. If so, the EFI image is always correct. There is no change 
requirement in PeCoff library in MdePkg.

Thanks
Liming
From: af...@apple.com [mailto:af...@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong 
Cc: edk2-devel@lists.01.org; Gao, Liming ; Kinney, 
Michael D 
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg 
with XCODE5

Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1

I also noticed the PeCoff lib is going to loop and reload the .debug suction 
due to this mtoc bug, so it would be good to harden that code too.

git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
 }

 return RETURN_SUCCESS;
+  } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+return RETURN_SUCCESS;
   }
 }
   }
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
 if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
   ImageContext->DebugDirectoryEntryRva = (UINT32) 
(DebugDirectoryEntryRva + Index);
   return RETURN_SUCCESS;
+} else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+  return RETURN_SUCCESS;
 }
   }
 }



https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1

Thanks,

Andrew Fish


On Aug 6, 2017, at 9:00 PM, Yonghong Zhu 
> wrote:

it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.

Cc: Liming Gao >
Cc: Michael D Kinney 
>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish >
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
  //
  // Get Debug, Export and Resource EntryTable RVA address.
  // Resource Directory entry need to review.
  //
  Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof 
(EFI_IMAGE_FILE_HEADER));
+  Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof 
(EFI_IMAGE_FILE_HEADER));
  if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr +  
FileHdr->SizeOfOptionalHeader);
if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT 
&& \
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 
0) {
  ExportDirectoryEntryRva = 
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;

Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 
0;
  }
}
  } else {
-Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof 
(EFI_IMAGE_FILE_HEADER));
SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr +  
FileHdr->SizeOfOptionalHeader);
if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT 
&& \
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 
0) {
  ExportDirectoryEntryRva = 
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
}
@@ -2907,10 +2907,20 @@ Returns:
  RsdsEntry->Unknown  = 0;
  RsdsEntry->Unknown2 = 0;
  RsdsEntry->Unknown3 = 0;
  RsdsEntry->Unknown4 = 0;
  RsdsEntry->Unknown5 = 0;
+} else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+  // MTOC sets DebugDirectoryEntrySize to size of the .debug section, 
so fix it.
+  if (!ZeroDebugFlag) {
+if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  
Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof 
(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+} else {
+  

Re: [edk2] [Patch V2 1/4] BaseTools: Fix Xcode 9 Beta treating 32-bit left shift as undefined

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Thursday, August 3, 2017 5:00 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Andrew Fish 
> ; Gao, Liming 
> Subject: [edk2] [Patch V2 1/4] BaseTools: Fix Xcode 9 Beta treating 32-bit 
> left shift as undefined
> 
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=635
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Andrew Fish 
> ---
>  BaseTools/Source/C/Common/Decompress.c   | 4 ++--
>  BaseTools/Source/C/TianoCompress/TianoCompress.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/Decompress.c 
> b/BaseTools/Source/C/Common/Decompress.c
> index 4b83e88..b2049bd 100644
> --- a/BaseTools/Source/C/Common/Decompress.c
> +++ b/BaseTools/Source/C/Common/Decompress.c
> @@ -87,15 +87,15 @@ Arguments:
> 
>  Returns: (VOID)
> 
>  --*/
>  {
> -  Sd->mBitBuf = (UINT32) (Sd->mBitBuf << NumOfBits);
> +  Sd->mBitBuf = (UINT32) (((UINT64)Sd->mBitBuf) << NumOfBits);
> 
>while (NumOfBits > Sd->mBitCount) {
> 
> -Sd->mBitBuf |= (UINT32) (Sd->mSubBitBuf << (NumOfBits = (UINT16) 
> (NumOfBits - Sd->mBitCount)));
> +Sd->mBitBuf |= (UINT32) (((UINT64)Sd->mSubBitBuf) << (NumOfBits = 
> (UINT16) (NumOfBits - Sd->mBitCount)));
> 
>  if (Sd->mCompSize > 0) {
>//
>// Get 1 byte into SubBitBuf
>//
> diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c 
> b/BaseTools/Source/C/TianoCompress/TianoCompress.c
> index f810511..046fb36 100644
> --- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
> +++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
> @@ -2064,15 +2064,15 @@ Arguments:
> 
>  Returns: (VOID)
> 
>  --*/
>  {
> -  Sd->mBitBuf = (UINT32) (Sd->mBitBuf << NumOfBits);
> +  Sd->mBitBuf = (UINT32) (((UINT64)Sd->mBitBuf) << NumOfBits);
> 
>while (NumOfBits > Sd->mBitCount) {
> 
> -Sd->mBitBuf |= (UINT32) (Sd->mSubBitBuf << (NumOfBits = (UINT16) 
> (NumOfBits - Sd->mBitCount)));
> +Sd->mBitBuf |= (UINT32) (((UINT64)Sd->mSubBitBuf) << (NumOfBits = 
> (UINT16) (NumOfBits - Sd->mBitCount)));
> 
>  if (Sd->mCompSize > 0) {
>//
>// Get 1 byte into SubBitBuf
>//
> --
> 2.6.1.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Does a double Page free report "ConvertPages: Incompatible memory types", maybe we could do better.

[Laszlo Ersek]

On 08/10/17 03:03, Andrew Fish wrote:
> It looks to me like if you Free pages, after you free pages you hit this 
> DEBUG message. 
> 
> 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Mem/Page.c#L790
> 
>   if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == 
> EfiConventionalMemory ? 1 : 0)) {
> DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory 
> types\n"));
> return EFI_NOT_FOUND;
>   }
> 
> I'm not sure I've thought out all the paths, but would it make more sense if 
> you are trying to convert EfiConventionalMemory to EfiConventionalMemory that 
> you are trying to free pages that are already freed. That is not very obvious 
> from the above DEBUG print.  Could there be an if in the error path to print 
> a better DEBUG message for a free pages bug? 
> 
> Also to be pedantic the function change names to: CoreConvertPagesEx(). 

(Reacting only to this side question:)

I generally prefer:

  DEBUG ((
DebugMask,
"%a: ...",
__FUNCTION__,
...
));

This way when the function is renamed, or the DEBUG is moved to another
function, the log will update itself.

Taking it one step further: if the DEBUG is in a widely used library
instance, then I like

  DEBUG ((
DebugMask,
"%a:%a: ...",
gEfiCallerBaseName,
__FUNCTION__,
...
));

Because this identifies the calling driver module as well (using the
BASE_NAME from the module's INF file).

("%g" with  would be more robust than "%a" with
gEfiCallerBaseName, but the log should also be readable...)

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

Re: [edk2] [PATCH 1/2] ShellPkg/driver: Show Image Name in non-SFO mode

[Leif Lindholm]

At a quick glance, the fix should probably be

diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
index 4d876bb108..26b785c563 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
@@ -216,7 +216,7 @@ GetImageNameFromHandle (
 Status = gBS->HandleProtocol (
 LoadedImage->DeviceHandle,
 ,
-
+(VOID **)
 );
 if (!EFI_ERROR (Status)) {
   Status = Fv2->ReadSection (

/
Leif

On Thu, Aug 10, 2017 at 08:53:11AM +0100, Ard Biesheuvel wrote:
> This patch is breaking the GCC build:
> 
> ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c:
>  In function 'GetImageNameFromHandle':
> 220:25: error: passing argument 3 of 'gBS->HandleProtocol' from
> incompatible pointer type [-Werror]
> 220:25: note: expected 'void **' but argument is of type 'struct
> EFI_FIRMWARE_VOLUME2_PROTOCOL **'
> 
> Please fix.
> 
> Regards,
> Ard.
> 
> 
> On 9 August 2017 at 08:49, Ruiyu Ni  wrote:
> > From: Huajing Li 
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Huajing Li 
> > Reviewed-by: Ruiyu Ni 
> > ---
> >  .../Library/UefiShellDriver1CommandsLib/Drivers.c  | 99 
> > +-
> >  .../UefiShellDriver1CommandsLib.h  |  4 +
> >  .../UefiShellDriver1CommandsLib.uni|  2 +-
> >  3 files changed, 100 insertions(+), 5 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c 
> > b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
> > index ffdef04352..f3c1476872 100644
> > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
> > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
> > @@ -2,7 +2,7 @@
> >Main file for Drivers shell Driver1 function.
> >
> >(C) Copyright 2012-2015 Hewlett-Packard Development Company, L.P.
> > -  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2010 - 2017, 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
> > @@ -161,6 +161,92 @@ ReturnDriverVersion(
> >  }
> >
> >  /**
> > +  Get image name from Image Handle.
> > +
> > +  @param[in] Handle  Image Handle
> > +
> > +  @return A pointer to the image name as a string.
> > +**/
> > +CHAR16 *
> > +GetImageNameFromHandle (
> > +  IN CONST EFI_HANDLE Handle
> > +  )
> > +{
> > +  EFI_STATUSStatus;
> > +  EFI_DRIVER_BINDING_PROTOCOL   *DriverBinding;
> > +  EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
> > +  EFI_DEVICE_PATH_PROTOCOL  *DevPathNode;
> > +  EFI_GUID  *NameGuid;
> > +  CHAR16*ImageName;
> > +  UINTN BufferSize;
> > +  UINT32AuthenticationStatus;
> > +  EFI_FIRMWARE_VOLUME2_PROTOCOL *Fv2;
> > +
> > +  LoadedImage   = NULL;
> > +  DriverBinding = NULL;
> > +  ImageName = NULL;
> > +
> > +  Status = gBS->OpenProtocol (
> > +  Handle,
> > +  ,
> > +  (VOID **) ,
> > +  NULL,
> > +  NULL,
> > +  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> > +  );
> > +  if (EFI_ERROR (Status)) {
> > +  return NULL;
> > +  }
> > +  Status = gBS->OpenProtocol (
> > +  DriverBinding->ImageHandle,
> > +  ,
> > +  (VOID**),
> > +  gImageHandle,
> > +  NULL,
> > +  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> > +  );
> > +  if (!EFI_ERROR (Status)) {
> > +DevPathNode = LoadedImage->FilePath;
> > +if (DevPathNode == NULL) {
> > +  return NULL;
> > +}
> > +while (!IsDevicePathEnd (DevPathNode)) {
> > +  NameGuid = EfiGetNameGuidFromFwVolDevicePathNode 
> > ((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)DevPathNode);
> > +  if (NameGuid != NULL) {
> > +Status = gBS->HandleProtocol (
> > +LoadedImage->DeviceHandle,
> > +,
> > +
> > +);
> > +if (!EFI_ERROR (Status)) {
> > +  Status = Fv2->ReadSection (
> > +  Fv2,
> > +  NameGuid,
> > +  EFI_SECTION_USER_INTERFACE,
> > +  0,
> > +  (VOID **),
> > +  ,
> > +  
> > +  

Re: [edk2] [PATCH v1 00/14] OvmfPkg/Virtio: Add APIs to map system physical to device address

[Laszlo Ersek]

On 08/10/17 00:44, Brijesh Singh wrote:
> 
> 
> On 08/09/2017 05:38 PM, Laszlo Ersek wrote:
>> On 08/07/17 13:58, Brijesh Singh wrote:
>>> Currently, virtio drivers provides the system physical address to the
>>> device.
>>> However, some systems may feature an IOMMU that requires the drivers
>>> to pass
>>> the device addresses to the device - which are then translated by the
>>> IOMMU
>>> into physical addresses in memory. The patch series introduces new
>>> member
>>> functions in VIRTIO_DEVICE_PROTOCOL which can be used for mapping a
>>> system
>>> physical address to device address.
>>>
>>> The approach that this patch series takes is to maps the system physical
>>> address to device address for buffers (including rings, device specifc
>>> request and response  pointed by vring descriptor, and any further
>>> memory
>>> reference by those request and response).
>>>
>>> Patch 1 - 3:
>>>   Defines and implements new member functions to map a system
>>> physical address
>>>   to device address. The patch implements Laszlo's suggestion [1].
>>>
>>> Patch 4 - 7:
>>>   Allocates the vring buffer using newly added member functions and
>>> provides
>>>   some helper functions.
>>>
>>> Patch 8:
>>>   Update the virtio-blk driver to use newly added member functions to
>>> map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>  -drive file=${IMAGE},if=none,id=disk0 \
>>>  -device virtio-blk-pci,drive=disk0
>>>
>>>   # $QEMU \
>>>  -drive file=${IMAGE},if=none,id=disk0 \
>>>  -device virtio-blk-pci,drive=disk0,disable-legacy=on
>>>
>>> Patch 9:
>>>   Update the virtio-scsi driver to use newly added member functions
>>> to map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>  -drive file=${IMAGE},if=none,id=disk0 \
>>>  -device scsi-hd,drive=disk0
>>>  -device virtio-scsi-pci,id=scsi \
>>>
>>>   # $QEMU \
>>>  -drive file=${IMAGE},if=none,id=disk0 \
>>>  -device scsi-hd,drive=disk0 \
>>>  -device virtio-scsi-pci,id=scsi,disable-legacy=on \
>>>
>>> Patch 10 - 13:
>>>   Update the virtio-net driver to use newly added member functions to
>>> map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>  -netdev type=tap,id=net0 \
>>>  -device virtio-net-pci,netdev=net0,romfile=
>>>
>>>   # $QEMU \
>>>  -netdev type=tap,id=net0 \
>>>  -device virtio-net-pci,netdev=net0,disable-legacy=on,romfile=
>>>
>>> Patch 14:
>>>   Update the virtio-rng driver to use newly added member functions to
>>> map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>  -device virtio-rng-pci
>>>
>>>   # $QEMU \
>>>  -device virtio-rng-pci,disable-legacy=on
>>>
>>>   And succesfully ran RngTest.efi from SecurityPkg/Application
>>>
>>> Repo: https://github.com/codomania/edk2
>>> Branch: virtio-support
>>>
>>> Cc: Ard Biesheuvel 
>>> Cc: Jordan Justen 
>>> Cc: Tom Lendacky 
>>> Cc: Laszlo Ersek 
>>>
>>> Brijesh Singh (14):
>>>OvmfPkg/Virtio: Introduce new member functions in
>>>  VIRTIO_DEVICE_PROTOCOL
>>>OvmfPkg/Virtio10Dxe: Implement new member functions
>>>OvmfPkg/VirtioPciDeviceDxe: Implement new member functions
>>>OvmfPkg/VirtioLib: Add SharedBuffer helper functions
>>>OvmfPkg/VirtioLib: Pass VirtIo instance in VringInit/Uinit()
>>>OvmfPkg/VirtioLib: Add functions to map/unmap VRING
>>>OvmfPkg/VirtioLib: Use AllocateShared() to allocate vring buffer
>>>OvmfPkg/VirtioBlkDxe: Use DeviceAddresses in vring descriptors
>>>OvmfPkg/VirtioScsiDxe: Use DeviceAddresses in vring descriptors
>>>OvmfPkg/VirtioNetDxe: Allocate Tx and Rx ring using
>>>  AllocateSharedPage()
>>>OvmfPkg/VirtioNetDxe: Allocate RxBuf using AllocateSharedPages()
>>>OvmfPkg/VirtioNetDxe: Dynamically allocate transmit header
>>>OvmfPkg/VirtioNetDxe: Use DeviceAddress in transmit vring descriptors
>>>OvmfPkg/VirtioRngDxe: Use DeviceAddresses in vring descriptors
>>>
>>>   OvmfPkg/Include/Library/VirtioLib.h | 198 -
>>>   OvmfPkg/Include/Protocol/VirtioDevice.h | 121 
>>>   OvmfPkg/VirtioBlkDxe/VirtioBlk.h|   1 +
>>>   OvmfPkg/VirtioNetDxe/VirtioNet.h|  25 +-
>>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h|  34 +++
>>>   OvmfPkg/VirtioRngDxe/VirtioRng.h|   1 +
>>>   OvmfPkg/VirtioScsiDxe/VirtioScsi.h  |   1 +
>>>   OvmfPkg/Library/VirtioLib/VirtioLib.c   | 296
>>> +++-
>>>   OvmfPkg/Virtio10Dxe/Virtio10.c  | 114 +++-
>>>   OvmfPkg/VirtioBlkDxe/VirtioBlk.c| 104 ++-
>>>   OvmfPkg/VirtioGpuDxe/Commands.c |   7 +-
>>>   OvmfPkg/VirtioNetDxe/Events.c   |  24 ++
>>>   OvmfPkg/VirtioNetDxe/SnpGetStatus.c |  

Re: [edk2] [PATCH 4/4] OvmfPkg: Introduce UDF_ENABLE build flag

[Laszlo Ersek]

On 08/09/17 19:51, Andrew Fish wrote:
> 
>> On Aug 9, 2017, at 10:33 AM, Laszlo Ersek  wrote:
>>
>> On 08/09/17 17:45, Andrew Fish wrote:
>>>
 On Aug 9, 2017, at 2:44 AM, Laszlo Ersek  wrote:

 CC Ard and Andrew

 On 08/08/17 21:31, Paulo Alcantara wrote:
> By defining this build flag, OVMF will support booting from UDF file
> systems.
>
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara 
> ---
> OvmfPkg/OvmfPkgIa32.dsc| 7 +++
> OvmfPkg/OvmfPkgIa32.fdf| 3 +++
> OvmfPkg/OvmfPkgIa32X64.dsc | 7 +++
> OvmfPkg/OvmfPkgIa32X64.fdf | 3 +++
> OvmfPkg/OvmfPkgX64.dsc | 7 +++
> OvmfPkg/OvmfPkgX64.fdf | 3 +++
> 6 files changed, 30 insertions(+)

 Ray already mentioned that PcdUdfFileSystemSupport is not needed. I
 agree. Similarly, I think UDF_ENABLE is also not needed, the new driver
 should be added to the DSC and FDF files right after "Fat.inf" (like you
 are doing it now, just unconditionally).

 Furthermore, can you please do the same in the ArmVirtPkg DSC and FDF
 files? (Just grep the tree for "Fat.inf".) EmulatorPkg and Nt32Pkg are
 further emulation platforms that might want to include this.

 My reason for suggesting the unconditional inclusion is the following
 sentence from the UEFI 2.7 spec:

 13 Protocols — Media Access
 13.3 File System Format
 13.3.2 Partition Discovery
 13.3.2.1 ISO-9660 and El Torito

 [...] DVD-ROM images formatted as required by the UDF 2.0
 specification (OSTA Universal Disk Format Specification, Revision 2.0)
 can be booted by EFI. [...]

 It does not say "may be bootable", it says "can be booted".

 It would be interesting to see the Mantis ticket (if any) that got this
 language into the spec, without the edk2 reference implementation
 providing a UDF driver.
 - Using the Mantis simple search function, "UDF" brings up nothing.
 - From some googling, this sentence appears to go back to EFI 1.10 at
 the least.

 Andrew, do you remember the history of the quoted sentence?

>>>
>>> UDF defines a "UDF Bridge" disk that is "El Torito" compatible. So UDF 
>>> punted on boot ability by allowing compatibility with CD-ROMs. 
>>>
>>> EFI supports booting from an ISO-9660 file system that conforms to the “El 
>>> Torito” Bootable CD-ROM Format Specification on a DVD- ROM. A DVD-ROM that 
>>> contains an ISO-9660 file system is defined as a “UDF Bridge” disk. Booting 
>>> from CD-ROM and DVD-ROM is accomplished using the same methods.
>>>
>>> I'm fine with adding a UDF file system driver, but it is not required from 
>>> a UEFI Spec conformance point of view.
>>
>> But the one sentence that I quoted above (from the spec) gives rise to
>> this exact impression:
>>
>>  [...] DVD-ROM images formatted as required by the UDF 2.0
>>  specification (OSTA Universal Disk Format Specification, Revision 2.0)
>>  can be booted by EFI. [...]
>>
>> Yes, it goes on to talk about "UDF Bridge", but this sentence per se
>> seems to require booting UDF optical media. The next sentence ("EFI
>> supports booting from an ISO-9660 file system...") does not start with
>> "Namely,".
>>
>> So is this a spec bug?
>>
>> I'd like to clarify this, because the intent of the above phrase
>> determines whether:
>> - edk2, as-is, conforms or does not conform to that passage of the spec,
>> - optical media that is formatted with UDF (and no ISO9660/ElTorito
>>  compat) qualifies as EFI-compatible (that edk2 currently cannot boot).
>>
>> Could you recommend improved wording for the spec? I'd be happy to file
>> a Mantis item on your behalf.
>>
> 
> Sure feel free to file a mantis. I think the missing chunk of info is that 
> the only definition of booting in the UDF spec is the "UDF Bridge" format to 
> be compatible with El-Torito. Thus the statement is factual. I think the UDF 
> spec basically states to be bootable it must be a UEF Bridge disk and thus be 
> compatible with El-Torito. 
> 
> Maybe turning things around a bit helps. 
> 
> The UDF 2.0 specification (..) requires a bootable DVD-ROM be formatted as a 
> "UDF Bridge" disk to be bootable...
> 
> Anyway feel free to start a thread on the spec mailing list.

I've filed

  https://mantis.uefi.org/mantis/view.php?id=1835

Thank you for your help!
Laszlo



>>> PS "El Torito" was the restaurant that Curtis and Stan wrote out the 
>>> proposal on a napkin. Luckily device paths are not called "House of 
>>> Teriyaki", or even worse the nickname "rats and rice".  
>>>
 Paulo, I'll check if I can test your driver with some 3rd party media
 (i.e., a DVD image that I don't prepare myself).

 Thank you!
 Laszlo

>
> diff --git 

[edk2] [Patch] BaseTools: Don't need to add extra quotes when UI string from file

[Yonghong Zhu]

From: Bin Wang 

when the UI string is read from files, we don't need to add the extra
quotes. Otherwise, it will cause UI name has this extra quotes.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Bin Wang 
---
 BaseTools/Source/Python/GenFds/UiSection.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/UiSection.py 
b/BaseTools/Source/Python/GenFds/UiSection.py
index d54e3b2..419e1ee 100644
--- a/BaseTools/Source/Python/GenFds/UiSection.py
+++ b/BaseTools/Source/Python/GenFds/UiSection.py
@@ -1,9 +1,9 @@
 ## @file
 # process UI section generation
 #
-#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2017, 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
 #  http://opensource.org/licenses/bsd-license.php
@@ -64,11 +64,10 @@ class UiSection (UiSectionClassObject):
 elif self.FileName != None:
 FileNameStr = 
GenFdsGlobalVariable.ReplaceWorkspaceMacro(self.FileName)
 FileNameStr = GenFdsGlobalVariable.MacroExtend(FileNameStr, Dict)
 FileObj = open(FileNameStr, 'r')
 NameString = FileObj.read()
-NameString = '\"' + NameString + "\""
 FileObj.close()
 else:
 NameString = ''
 
 GenFdsGlobalVariable.GenerateSection(OutputFile, None, 
'EFI_SECTION_USER_INTERFACE', Ui=NameString)
-- 
2.6.1.windows.1

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

Re: [edk2] [Patch 2/2] QuarkPlatformPkg/Readme.md: edk2-non-osi directory layout

[Leif Lindholm]

On Wed, Aug 09, 2017 at 12:39:57PM -0700, Michael D Kinney wrote:
> The following commit moved the QuarkSocBinPkg from the root
> directory to the the Silicon/Intel directory.
> 
> https://github.com/tianocore/edk2-non-osi/commit/182e85d04566800fe188de4b1c30a50533dd74b7
> 
> The following updates are made to Readme.md:
> 
> * PACKAGES_PATH setting for edk2-non-osi directory changes
> * Remove use of edk2-FatPkg repository
> * Remove use of edk2-BaseTools-win32 repository
> * Run python build tools from sources

I don't think you need my r-b on this, but I appreciate the cc.

If I was nitpicking, I would point out there are three logically
unrelated changes going on here. However, since it all consist of
updates to a single documentation file, the only change I would
request would be to the subject line to say something a bit broader
like "bring Readme.md up to date".

With that:
Reviewed-by: Leif Lindholm 

> Cc: Leif Lindholm 
> Cc: Kelly Steele 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney 
> ---
>  QuarkPlatformPkg/Readme.md | 34 ++
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/QuarkPlatformPkg/Readme.md b/QuarkPlatformPkg/Readme.md
> index f925f9ef27..aa9d9856bd 100644
> --- a/QuarkPlatformPkg/Readme.md
> +++ b/QuarkPlatformPkg/Readme.md
> @@ -46,12 +46,12 @@
>- Install
>  * ASL compiler: Available from http://www.acpica.org
>- Install into ```C:\ASL``` to match default tools_def.txt configuration.
> +* Python 2.7: Available from http://www.python.org
>  
>  Create a new directory for an EDK II WORKSPACE.
>  
>  The code block below shows the GIT clone operations required to pull the EDK 
> II
> -source tree, the FatPkg sources, the pre-built versions of BaseTools as WIN32
> -binaries, and the edk2-non-osi repository that provides a binary file for the
> +source tree and the edk2-non-osi repository that provides a binary file for 
> the
>  Quark Remote Management Unit (RMU).
>  
>  Next it sets environment variables that must be set before running
> @@ -60,6 +60,8 @@ the EDK II [Multiple Workspace](
>  https://github.com/tianocore/tianocore.github.io/wiki/Multiple_Workspace)
>  feature is used.
>  
> +Next, the EDK II BaseTools required to build firmware images are built.
> +
>  Next, the ```edksetup.bat``` file is run to complete the initialization of an
>  EDK II build environment.  Two example build commands are shown.  The first 
> one
>  in ```QuarkPlatformPlg/Quark.dsc``` builds a full UEFI firmware image that is
> @@ -69,16 +71,17 @@ image that is useful for initial power-on and debug of 
> new features.
>  
>  ```cmd
>  git clone https://github.com/tianocore/edk2.git
> -git clone https://github.com/tianocore/edk2-FatPkg.git FatPkg
> -git clone https://github.com/tianocore/edk2-BaseTools-win32.git
>  git clone https://github.com/tianocore/edk2-non-osi.git
>  
> +set PYTHON_HOME=c:\Python27
>  set WORKSPACE=%CD%
> -set PACKAGES_PATH=%WORKSPACE%\edk2;%WORKSPACE%\edk2-non-osi
> -set EDK_TOOLS_BIN=%WORKSPACE%\edk2-BaseTools-win32
> +set PACKAGES_PATH=%WORKSPACE%\edk2;%WORKSPACE%\edk2-non-osi\Silicon\Intel
> +set EDK_TOOLS_PATH=%WORKSPACE%\edk2\BaseTools
> +cd %WORKSPACE%\edk2
>  
> -cd edk2
> -edksetup.bat
> +BaseTools\toolsetup.bat Rebuild
> +
> +edksetup.bat Rebuild
>  
>  build -a IA32 -t VS2015x86 -p QuarkPlatformPkg/Quark.dsc
>  build -a IA32 -t VS2015x86 -p QuarkPlatformPkg/QuarkMin.dsc
> @@ -91,12 +94,13 @@ build -a IA32 -t VS2015x86 -p 
> QuarkPlatformPkg/QuarkMin.dsc
>  * GIT client
>  * GCC 4.9 compiler
>  * ASL compiler: Available from http://www.acpica.org.
> +* Python 2.7
>  
>  Create a new directory for an EDK II WORKSPACE.
>  
>  The code block below shows the GIT clone operations required to pull the EDK 
> II
> -source tree, the FatPkg sources, and the edk2-non-osi repository that 
> provides a
> -binary file for the Quark Remote Management Unit (RMU).
> +source tree and the edk2-non-osi repository that provides a binary file for 
> the
> +Quark Remote Management Unit (RMU).
>  
>  Next it sets environment variables that must be set before running
>  ```edksetup.bat```. Since content is being pulled from multiple repositories,
> @@ -106,7 +110,7 @@ feature is used.
>  
>  Next, the EDK II BaseTools required to build firmware images are built.
>  
> -Next, the ```edksetup.bat``` file is run to complete the initialization of an
> +Next, the ```edksetup.sh``` file is run to complete the initialization of an
>  EDK II build environment.  Two example build commands are shown.  The first 
> one
>  in ```QuarkPlatformPlg/Quark.dsc``` builds a full UEFI firmware image that is
>  able to boot the built-in UEFI Shell and Linux from a micro SD FLASH card.  
> The
> @@ -115,17 +119,15 @@ image that is useful for initial power-on and debug of 
> new features.
>  
>  ```sh
>  git 

[edk2] [patch] BaseTools/UPT: Support Multiple Installation

[hesschen]

Add a new feature to UPT to support installing
multiple DIST packages in one time.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: hesschen 
---
 .../Source/Python/UPT/Core/DependencyRules.py  | 18 -
 .../Python/UPT/GenMetaFile/GenMetaFileMisc.py  | 90 +
 BaseTools/Source/Python/UPT/InstallPkg.py  | 94 +++---
 BaseTools/Source/Python/UPT/Library/GlobalData.py  |  9 ++-
 BaseTools/Source/Python/UPT/Logger/StringTable.py  |  4 +-
 BaseTools/Source/Python/UPT/ReplacePkg.py  |  8 +-
 BaseTools/Source/Python/UPT/TestInstall.py |  8 +-
 BaseTools/Source/Python/UPT/UPT.py | 16 ++--
 8 files changed, 145 insertions(+), 102 deletions(-)

diff --git a/BaseTools/Source/Python/UPT/Core/DependencyRules.py 
b/BaseTools/Source/Python/UPT/Core/DependencyRules.py
index 57f7b40..909c584 100644
--- a/BaseTools/Source/Python/UPT/Core/DependencyRules.py
+++ b/BaseTools/Source/Python/UPT/Core/DependencyRules.py
@@ -44,12 +44,24 @@ DEPEX_CHECK_PACKAGE_NOT_FOUND, DEPEX_CHECK_DP_NOT_FOUND) = 
(0, 1, 2, 3)
 # @param object:  Inherited from object class
 #
 class DependencyRules(object):
-def __init__(self, Datab):
+def __init__(self, Datab, ToBeInstalledPkgList=None):
 self.IpiDb = Datab
 self.WsPkgList = GetWorkspacePackage()
 self.WsModuleList = GetWorkspaceModule()
-self.PkgsToBeDepend = []
+
+self.PkgsToBeDepend = [(PkgInfo[1], PkgInfo[2]) for PkgInfo in 
self.WsPkgList]
+
+# Add package info from the DIST to be installed.
+
self.PkgsToBeDepend.extend(self.GenToBeInstalledPkgList(ToBeInstalledPkgList))
 
+def GenToBeInstalledPkgList(self, ToBeInstalledPkgList):
+RtnList = []
+for Dist in ToBeInstalledPkgList:
+for Package in Dist.PackageSurfaceArea:
+RtnList.append((Package[0], Package[1]))
+
+return RtnList
+
 ## Check whether a module exists by checking the Guid+Version+Name+Path 
combination
 #
 # @param Guid:  Guid of a module
@@ -182,7 +194,6 @@ class DependencyRules(object):
 #  False else
 #
 def CheckInstallDpDepexSatisfied(self, DpObj):
-self.PkgsToBeDepend = [(PkgInfo[1], PkgInfo[2]) for PkgInfo in 
self.WsPkgList]
 return self.CheckDpDepexSatisfied(DpObj)
 
 # # Check whether multiple DP depex satisfied by current workspace for 
Install
@@ -192,7 +203,6 @@ class DependencyRules(object):
 #  False else
 #
 def CheckTestInstallPdDepexSatisfied(self, DpObjList):
-self.PkgsToBeDepend = [(PkgInfo[1], PkgInfo[2]) for PkgInfo in 
self.WsPkgList]
 for DpObj in DpObjList:
 if self.CheckDpDepexSatisfied(DpObj):
 for PkgKey in DpObj.PackageSurfaceArea.keys():
diff --git a/BaseTools/Source/Python/UPT/GenMetaFile/GenMetaFileMisc.py 
b/BaseTools/Source/Python/UPT/GenMetaFile/GenMetaFileMisc.py
index 3c6c9ee..ae8dc85 100644
--- a/BaseTools/Source/Python/UPT/GenMetaFile/GenMetaFileMisc.py
+++ b/BaseTools/Source/Python/UPT/GenMetaFile/GenMetaFileMisc.py
@@ -79,6 +79,10 @@ def AddExternToDefineSec(SectionDict, Arch, ExternList):
 # Using TokenSpaceGuidValue and Token to obtain PcdName from DEC file
 #
 def ObtainPcdName(Packages, TokenSpaceGuidValue, Token):
+TokenSpaceGuidName = ''
+PcdCName = ''
+TokenSpaceGuidNameFound = False
+
 for PackageDependency in Packages:
 #
 # Generate generic comment
@@ -86,6 +90,7 @@ def ObtainPcdName(Packages, TokenSpaceGuidValue, Token):
 Guid = PackageDependency.GetGuid()
 Version = PackageDependency.GetVersion()
 
+Path = None
 #
 # find package path/name
 # 
@@ -95,41 +100,58 @@ def ObtainPcdName(Packages, TokenSpaceGuidValue, Token):
 Path = PkgInfo[3]
 break
 
-DecFile = None
-if Path not in GlobalData.gPackageDict:
-DecFile = Dec(Path)
-GlobalData.gPackageDict[Path] = DecFile
-else:
-DecFile = GlobalData.gPackageDict[Path]
-
-DecGuidsDict = DecFile.GetGuidSectionObject().ValueDict
-DecPcdsDict = DecFile.GetPcdSectionObject().ValueDict
-
-TokenSpaceGuidName = ''
-PcdCName = ''
-TokenSpaceGuidNameFound = False
-
-#
-# Get TokenSpaceGuidCName from Guids section 
-#
-for GuidKey in DecGuidsDict:
-GuidList = DecGuidsDict[GuidKey]
-for GuidItem in GuidList:
-if TokenSpaceGuidValue.upper() == GuidItem.GuidString.upper():
-TokenSpaceGuidName = GuidItem.GuidCName
-TokenSpaceGuidNameFound = True
+# The dependency package in workspace
+if Path:
+DecFile = None
+if Path not in GlobalData.gPackageDict:
+DecFile = Dec(Path)
+

Re: [edk2] [Patch 0/3] Support Ip4Config2/Ip6Config.SetData interface to clear specific configuration

[Ye, Ting]

Series Reviewed-by: Ye Ting 


-Original Message-
From: Wu, Jiaxin 
Sent: Wednesday, July 26, 2017 2:28 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Wu, Jiaxin 

Subject: [Patch 0/3] Support Ip4Config2/Ip6Config.SetData interface to clear 
specific configuration

UEFI Spec 2.7 adds the clarification on SetData interface usage to clear 
specific individual data types. The series patches are used to support this 
feature.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 

Jiaxin Wu (3):
  MdePkg: Update the comments of Ip4Config2/Ip6Config Protocol
  MdeModulePkg/Ip4Dxe: Support SetData interface to clear specific
configuration
  NetworkPkg/Ip6Dxe: Support SetData interface to clear specific
configuration

 .../Universal/Network/Ip4Dxe/Ip4Config2Impl.c  | 285 ++---
 MdePkg/Include/Protocol/Ip4Config2.h   |  17 +-
 MdePkg/Include/Protocol/Ip6Config.h|  15 +-
 NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c  | 692 -
 4 files changed, 610 insertions(+), 399 deletions(-)

--
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 1/2] ShellPkg/driver: Show Image Name in non-SFO mode

[Ard Biesheuvel]

This patch is breaking the GCC build:

ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c:
 In function 'GetImageNameFromHandle':
220:25: error: passing argument 3 of 'gBS->HandleProtocol' from
incompatible pointer type [-Werror]
220:25: note: expected 'void **' but argument is of type 'struct
EFI_FIRMWARE_VOLUME2_PROTOCOL **'

Please fix.

Regards,
Ard.


On 9 August 2017 at 08:49, Ruiyu Ni  wrote:
> From: Huajing Li 
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Huajing Li 
> Reviewed-by: Ruiyu Ni 
> ---
>  .../Library/UefiShellDriver1CommandsLib/Drivers.c  | 99 
> +-
>  .../UefiShellDriver1CommandsLib.h  |  4 +
>  .../UefiShellDriver1CommandsLib.uni|  2 +-
>  3 files changed, 100 insertions(+), 5 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c 
> b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
> index ffdef04352..f3c1476872 100644
> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
> @@ -2,7 +2,7 @@
>Main file for Drivers shell Driver1 function.
>
>(C) Copyright 2012-2015 Hewlett-Packard Development Company, L.P.
> -  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2010 - 2017, 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
> @@ -161,6 +161,92 @@ ReturnDriverVersion(
>  }
>
>  /**
> +  Get image name from Image Handle.
> +
> +  @param[in] Handle  Image Handle
> +
> +  @return A pointer to the image name as a string.
> +**/
> +CHAR16 *
> +GetImageNameFromHandle (
> +  IN CONST EFI_HANDLE Handle
> +  )
> +{
> +  EFI_STATUSStatus;
> +  EFI_DRIVER_BINDING_PROTOCOL   *DriverBinding;
> +  EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
> +  EFI_DEVICE_PATH_PROTOCOL  *DevPathNode;
> +  EFI_GUID  *NameGuid;
> +  CHAR16*ImageName;
> +  UINTN BufferSize;
> +  UINT32AuthenticationStatus;
> +  EFI_FIRMWARE_VOLUME2_PROTOCOL *Fv2;
> +
> +  LoadedImage   = NULL;
> +  DriverBinding = NULL;
> +  ImageName = NULL;
> +
> +  Status = gBS->OpenProtocol (
> +  Handle,
> +  ,
> +  (VOID **) ,
> +  NULL,
> +  NULL,
> +  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +  );
> +  if (EFI_ERROR (Status)) {
> +  return NULL;
> +  }
> +  Status = gBS->OpenProtocol (
> +  DriverBinding->ImageHandle,
> +  ,
> +  (VOID**),
> +  gImageHandle,
> +  NULL,
> +  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +  );
> +  if (!EFI_ERROR (Status)) {
> +DevPathNode = LoadedImage->FilePath;
> +if (DevPathNode == NULL) {
> +  return NULL;
> +}
> +while (!IsDevicePathEnd (DevPathNode)) {
> +  NameGuid = EfiGetNameGuidFromFwVolDevicePathNode 
> ((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)DevPathNode);
> +  if (NameGuid != NULL) {
> +Status = gBS->HandleProtocol (
> +LoadedImage->DeviceHandle,
> +,
> +
> +);
> +if (!EFI_ERROR (Status)) {
> +  Status = Fv2->ReadSection (
> +  Fv2,
> +  NameGuid,
> +  EFI_SECTION_USER_INTERFACE,
> +  0,
> +  (VOID **),
> +  ,
> +  
> +  );
> +  if (!EFI_ERROR (Status)) {
> +break;
> +  }
> +  ImageName = NULL;
> +}
> +  }
> +  //
> +  // Next device path node
> +  //
> +  DevPathNode = NextDevicePathNode (DevPathNode);
> +}
> +if (ImageName == NULL) {
> +  ImageName = ConvertDevicePathToText (LoadedImage->FilePath, TRUE, 
> TRUE);
> +}
> +  }
> +  return ImageName;
> +}
> +
> +/**
>Function for 'drivers' command.
>
>@param[in] ImageHandle  Handle to the Image (NULL if Internal).
> @@ -186,6 +272,7 @@ ShellCommandRunDrivers (
>CHAR16  *Temp2;
>CONST CHAR16*FullDriverName;
>CHAR16  *TruncatedDriverName;
> +  CHAR16  *ImageName;
>CHAR16  *FormatString;
>UINT32  DriverVersion;
>BOOLEAN DriverConfig;
> @@ -274,6 +361,7 @@ ShellCommandRunDrivers (
>  DriverConfig   = 

[edk2] [PATCH] IntelSiliconPkg: Fix VS2015 NOOPT IA32 build failure in IntelVTdDxe

[Star Zeng]

There are VS2015 NOOPT IA32 build failure like below in IntelVTdDxe.
XXX.lib(XXX.obj) : error LNK2001: unresolved external symbol __allshl
XXX.lib(XXX.obj) : error LNK2001: unresolved external symbol __aullshr

This patch is to update Vtd.h to use UINT32 instead of UINT64 for
bitfields in structure definition, and also update IntelVTdDxe code
accordingly.

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 IntelSiliconPkg/Include/IndustryStandard/Vtd.h   | 225 ---
 IntelSiliconPkg/IntelVTdDxe/DmaProtection.h  |   2 +
 IntelSiliconPkg/IntelVTdDxe/PciInfo.c|   4 +-
 IntelSiliconPkg/IntelVTdDxe/TranslationTable.c   |  39 ++--
 IntelSiliconPkg/IntelVTdDxe/TranslationTableEx.c |  12 +-
 IntelSiliconPkg/IntelVTdDxe/VtdReg.c |   2 +-
 6 files changed, 152 insertions(+), 132 deletions(-)

diff --git a/IntelSiliconPkg/Include/IndustryStandard/Vtd.h 
b/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
index 5c6a494eae34..3b7012c5a576 100644
--- a/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
+++ b/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
@@ -26,9 +26,10 @@
 
 typedef union {
   struct {
-UINT64  Present:1;
-UINT64  Reserved_1:11;
-UINT64  ContextTablePointer:52;
+UINT32  Present:1;
+UINT32  Reserved_1:11;
+UINT32  ContextTablePointerLo:20;
+UINT32  ContextTablePointerHi:32;
 
 UINT64  Reserved_64;
   } Bits;
@@ -40,13 +41,15 @@ typedef union {
 
 typedef union {
   struct {
-UINT64  LowerPresent:1;
-UINT64  Reserved_1:11;
-UINT64  LowerContextTablePointer:52;
-
-UINT64  UpperPresent:1;
-UINT64  Reserved_65:11;
-UINT64  UpperContextTablePointer:52;
+UINT32  LowerPresent:1;
+UINT32  Reserved_1:11;
+UINT32  LowerContextTablePointerLo:20;
+UINT32  LowerContextTablePointerHi:32;
+
+UINT32  UpperPresent:1;
+UINT32  Reserved_65:11;
+UINT32  UpperContextTablePointerLo:20;
+UINT32  UpperContextTablePointerHi:32;
   } Bits;
   struct {
 UINT64  Uint64Lo;
@@ -56,17 +59,19 @@ typedef union {
 
 typedef union {
   struct {
-UINT64  Present:1;
-UINT64  FaultProcessingDisable:1;
-UINT64  TranslationType:2;
-UINT64  Reserved_4:8;
-UINT64  SecondLevelPageTranslationPointer:52;
-
-UINT64  AddressWidth:3;
-UINT64  Ignored_67:4;
-UINT64  Reserved_71:1;
-UINT64  DomainIdentifier:16;
-UINT64  Reserved_88:40;
+UINT32  Present:1;
+UINT32  FaultProcessingDisable:1;
+UINT32  TranslationType:2;
+UINT32  Reserved_4:8;
+UINT32  SecondLevelPageTranslationPointerLo:20;
+UINT32  SecondLevelPageTranslationPointerHi:32;
+
+UINT32  AddressWidth:3;
+UINT32  Ignored_67:4;
+UINT32  Reserved_71:1;
+UINT32  DomainIdentifier:16;
+UINT32  Reserved_88:8;
+UINT32  Reserved_96:32;
   } Bits;
   struct {
 UINT64  Uint64Lo;
@@ -76,51 +81,54 @@ typedef union {
 
 typedef union {
   struct {
-UINT64  Present:1;
-UINT64  FaultProcessingDisable:1;
-UINT64  TranslationType:3;
-UINT64  ExtendedMemoryType:3;
-UINT64  DeferredInvalidateEnable:1;
-UINT64  PageRequestEnable:1;
-UINT64  NestedTranslationEnable:1;
-UINT64  PASIDEnable:1;
-UINT64  SecondLevelPageTranslationPointer:52;
-
-UINT64  AddressWidth:3;
-UINT64  PageGlobalEnable:1;
-UINT64  NoExecuteEnable:1;
-UINT64  WriteProtectEnable:1;
-UINT64  CacheDisable:1;
-UINT64  ExtendedMemoryTypeEnable:1;
-UINT64  DomainIdentifier:16;
-UINT64  SupervisorModeExecuteProtection:1;
-UINT64  ExtendedAccessedFlagEnable:1;
-UINT64  ExecuteRequestsEnable:1;
-UINT64  SecondLevelExecuteEnable:1;
-UINT64  Reserved_92:4;
-UINT64  PageAttributeTable0:3;
-UINT64  Reserved_Pat0:1;
-UINT64  PageAttributeTable1:3;
-UINT64  Reserved_Pat1:1;
-UINT64  PageAttributeTable2:3;
-UINT64  Reserved_Pat2:1;
-UINT64  PageAttributeTable3:3;
-UINT64  Reserved_Pat3:1;
-UINT64  PageAttributeTable4:3;
-UINT64  Reserved_Pat4:1;
-UINT64  PageAttributeTable5:3;
-UINT64  Reserved_Pat5:1;
-UINT64  PageAttributeTable6:3;
-UINT64  Reserved_Pat6:1;
-UINT64  PageAttributeTable7:3;
-UINT64  Reserved_Pat7:1;
-
-UINT64  PASIDTableSize:4;
-UINT64  Reserved_132:8;
-UINT64  PASIDTablePointer:52;
-
-UINT64  Reserved_192:12;
-UINT64  PASIDStateTablePointer:52;
+UINT32  Present:1;
+UINT32  FaultProcessingDisable:1;
+UINT32  TranslationType:3;
+UINT32  ExtendedMemoryType:3;
+UINT32  DeferredInvalidateEnable:1;
+UINT32  PageRequestEnable:1;
+UINT32  NestedTranslationEnable:1;
+UINT32  PASIDEnable:1;
+UINT32  SecondLevelPageTranslationPointerLo:20;
+UINT32  SecondLevelPageTranslationPointerHi:32;
+
+UINT32  AddressWidth:3;
+UINT32  PageGlobalEnable:1;
+UINT32  NoExecuteEnable:1;
+UINT32  WriteProtectEnable:1;
+UINT32