[edk2] [PATCH] EmbeddedPkg/PrePiLib: add support for v2 sections

[Michael Zimmermann]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann 
---
 EmbeddedPkg/Library/PrePiLib/FwVol.c | 63 
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/EmbeddedPkg/Library/PrePiLib/FwVol.c
b/EmbeddedPkg/Library/PrePiLib/FwVol.c
index 530fc15dc..d513de351 100644
--- a/EmbeddedPkg/Library/PrePiLib/FwVol.c
+++ b/EmbeddedPkg/Library/PrePiLib/FwVol.c
@@ -296,35 +296,61 @@ FfsProcessSection (
   UINT32  SectionLength;
   UINT32  ParsedLength;
   EFI_COMPRESSION_SECTION *CompressionSection;
+  EFI_COMPRESSION_SECTION2*CompressionSection2;
   UINT32  DstBufferSize;
   VOID*ScratchBuffer;
   UINT32  ScratchBufferSize;
   VOID*DstBuffer;
   UINT16  SectionAttribute;
   UINT32  AuthenticationStatus;
+  CHAR8   *CompressedData;
+  UINTN   CompressedDataLength;


   *OutputBuffer = NULL;
   ParsedLength  = 0;
   Status= EFI_NOT_FOUND;
   while (ParsedLength < SectionSize) {
+if (IS_SECTION2 (Section)) {
+  ASSERT (SECTION2_SIZE (Section) > 0x00FF);
+}
+
 if (Section->Type == SectionType) {
-  *OutputBuffer = (VOID *)(Section + 1);
+  if (IS_SECTION2 (Section)) {
+*OutputBuffer = (VOID *)((UINT8 *) Section + sizeof
(EFI_COMMON_SECTION_HEADER2));
+  } else {
+*OutputBuffer = (VOID *)((UINT8 *) Section + sizeof
(EFI_COMMON_SECTION_HEADER));
+  }

   return EFI_SUCCESS;
 } else if ((Section->Type == EFI_SECTION_COMPRESSION) ||
(Section->Type == EFI_SECTION_GUID_DEFINED)) {

   if (Section->Type == EFI_SECTION_COMPRESSION) {
-CompressionSection  = (EFI_COMPRESSION_SECTION *) Section;
-SectionLength   = *(UINT32 *)Section->Size & 0x00FF;
-
-if (CompressionSection->CompressionType !=
EFI_STANDARD_COMPRESSION) {
-  return EFI_UNSUPPORTED;
+if (IS_SECTION2 (Section)) {
+  CompressionSection2 = (EFI_COMPRESSION_SECTION2 *) Section;
+  SectionLength   = SECTION2_SIZE (Section);
+
+  if (CompressionSection2->CompressionType !=
EFI_STANDARD_COMPRESSION) {
+return EFI_UNSUPPORTED;
+  }
+
+  CompressedData = (CHAR8 *) ((EFI_COMPRESSION_SECTION2 *) Section
+ 1);
+  CompressedDataLength = (UINT32) SectionLength - sizeof
(EFI_COMPRESSION_SECTION2);
+} else {
+  CompressionSection  = (EFI_COMPRESSION_SECTION *) Section;
+  SectionLength   = SECTION_SIZE (Section);
+
+  if (CompressionSection->CompressionType !=
EFI_STANDARD_COMPRESSION) {
+return EFI_UNSUPPORTED;
+  }
+
+  CompressedData = (CHAR8 *) ((EFI_COMPRESSION_SECTION *) Section
+ 1);
+  CompressedDataLength = (UINT32) SectionLength - sizeof
(EFI_COMPRESSION_SECTION);
 }

 Status = UefiDecompressGetInfo (
-   (UINT8 *) ((EFI_COMPRESSION_SECTION *) Section + 1),
-   (UINT32) SectionLength - sizeof
(EFI_COMPRESSION_SECTION),
+   CompressedData,
+   CompressedDataLength,
,

);
@@ -362,13 +388,23 @@ FfsProcessSection (
   // DstBuffer still is one section. Adjust DstBuffer offset, skip EFI
section header
   // to make section data at page alignment.
   //
-  DstBuffer = (UINT8 *)DstBuffer + EFI_PAGE_SIZE - sizeof
(EFI_COMMON_SECTION_HEADER);
+  if (IS_SECTION2 (Section))
+DstBuffer = (UINT8 *)DstBuffer + EFI_PAGE_SIZE - sizeof
(EFI_COMMON_SECTION_HEADER2);
+  else
+DstBuffer = (UINT8 *)DstBuffer + EFI_PAGE_SIZE - sizeof
(EFI_COMMON_SECTION_HEADER);
   //
   // Call decompress function
   //
   if (Section->Type == EFI_SECTION_COMPRESSION) {
+if (IS_SECTION2 (Section)) {
+  CompressedData = (CHAR8 *) ((EFI_COMPRESSION_SECTION2 *) Section
+ 1);
+}
+else {
+  CompressedData = (CHAR8 *) ((EFI_COMPRESSION_SECTION *) Section
+ 1);
+}
+
 Status = UefiDecompress (
-(CHAR8 *) ((EFI_COMPRESSION_SECTION *) Section + 1),
+CompressedData,
 DstBuffer,
 ScratchBuffer
 );
@@ -397,12 +433,15 @@ FfsProcessSection (
}
 }

+if (IS_SECTION2 (Section)) {
+  SectionLength = SECTION2_SIZE (Section);
+} else {
+  SectionLength = SECTION_SIZE (Section);
+}
 //
-// Size is 24 bits wide so mask upper 8 bits.
 // SectionLength is adjusted it is 4 byte aligned.
 // Go to the next section
 //
-SectionLength = 

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

without the big file at the end it looks very similar:
  c4 cc 4a 17 00 00 00 00  00 00 00 00 00 00 00 00  |..J.|
0010  00 00 00 00 78 e5 8c 8c  3d 8a 1c 4f 99 35 89 61  |x...=..O.5.a|
0020  85 c3 2d d3 c0 cc 4a 00  00 00 00 00 5f 46 56 48  |..-...J._FVH|
0030  ff fe 04 00 48 00 f1 fd  60 00 00 02 33 2b 01 00  |H...`...3+..|
0040  40 00 00 00 00 00 00 00  00 00 00 00 ff ff ff ff  |@...|
0050  ff ff ff ff ff ff ff ff  ff ff ff ff f4 aa f0 00  ||
0060  2c 00 00 f8 bd 1d f5 8f  56 b8 cb 4a b8 59 85 36  |,...V..J.Y.6|
0070  d8 92 ed 3a 14 00 00 00  ff ff ff ff e7 0e 51 fc  |...:..Q.|
0080  dc ff d4 11 bd 41 00 80  c7 3c 88 81 e6 aa 02 00  |.A...<..|
0090  8c 00 00 f8 74 00 00 19  57 72 cf 80 ab 87 f9 47  |t...Wr.G|

with the big file (when it's broken) it looks like this:
  ff ff ff 17 08 cd 03 01  00 00 00 00 00 00 00 00  ||
0010  00 00 00 00 00 00 00 00  78 e5 8c 8c 3d 8a 1c 4f  |x...=..O|
0020  99 35 89 61 85 c3 2d d3  00 cd 03 01 00 00 00 00  |.5.a..-.|
0030  5f 46 56 48 ff fe 04 00  48 00 f4 18 60 00 00 02  |_FVHH...`...|
0040  34 0f 04 00 40 00 00 00  00 00 00 00 00 00 00 00  |4...@...|
0050  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  ||
0060  f4 aa f0 00 2c 00 00 f8  bd 1d f5 8f 56 b8 cb 4a  |,...V..J|
0070  b8 59 85 36 d8 92 ed 3a  14 00 00 00 ff ff ff ff  |.Y.6...:|
0080  e7 0e 51 fc dc ff d4 11  bd 41 00 80 c7 3c 88 81  |..Q..A...<..|
0090  e6 aa 02 00 8c 00 00 f8  74 00 00 19 57 72 cf 80  |t...Wr..|

While looking for the header format and checking what these ff's mean
I found that this is a EFI_COMMON_SECTION_HEADER2 which is used
because the max size of EFI_COMMON_SECTION_HEADER is 0xFF.
MdeModulePkg's FwVol.c also seems to have some code for handling v2 sections.
I'll send a patch to fix this in a bit.



On Mon, Dec 11, 2017 at 12:08 AM, Ard Biesheuvel
 wrote:
> On 10 December 2017 at 22:18, Michael Zimmermann
>  wrote:
>> Exactly. If I shift the pointer by 4 bytes from within PrePiLib the device
>> boots just fine.
>>
>> I'm not sure if the size is the root cause but right now it only happens
>> when adding a ~15mb binary efi to the end of fvmain.
>>
>
> It appears so.
>
> So it would be good to check where the disparity originates.
>
> The Ffs section containing the compressed FV looks like this for ArmVirtQemu
>
> $ hexdump -C 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792SEC1.1fv.sec |head
>
>   84 a8 48 17 00 00 00 00  00 00 00 00 00 00 00 00  |..H.|
> 0010  00 00 00 00 78 e5 8c 8c  3d 8a 1c 4f 99 35 89 61  |x...=..O.5.a|
> 0020  85 c3 2d d3 80 a8 48 00  00 00 00 00 5f 46 56 48  |..-...H._FVH|
> 0030  ff fe 04 00 48 00 24 2b  00 00 00 02 a2 22 01 00  |H.$+."..|
> 0040  40 00 00 00 00 00 00 00  00 00 00 00 7f cb a2 d6  |@...|
> 0050  18 6a 2f 4e b4 3b 99 20  a7 33 70 0a 4d aa 05 00  |.j/N.;. .3p.M...|
> 0060  30 c0 01 f8 04 c0 01 10  4d 5a 00 00 00 00 00 00  |0...MZ..|
> 0070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
> *
> 00a0  00 00 00 00 58 0e 00 00  00 00 00 00 00 00 00 00  |X...|
>
> Could you compare with your build?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] Correct function description for ALLOCATE_BUFFER

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Monday, December 11, 2017 2:54 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ; Ni,
> Ruiyu ; Gao, Liming ; Kinney,
> Michael D 
> Subject: [PATCH 0/3] Correct function description for ALLOCATE_BUFFER
> 
> According to UEFI spec, DUAL_ADDRESS_CYCLE is missing in the
> EFI_UNSUPPORTED return status description for ALLOCATE_BUFFER interface.
> Same issue is also in IOMMU interface.
> 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> 
> Star Zeng (3):
>   MdePkg PciIo.h: Correct function description for ALLOCATE_BUFFER
>   MdeModulePkg: Correct function description for AllocateBuffer
>   IntelSilicon: Correct function description for AllocateBuffer
> 
>  IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c | 2 +-
>  IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c   | 2 +-
>  IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c | 2 +-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c  | 4 ++--
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h  | 4 ++--
>  MdeModulePkg/Include/Ppi/IoMmu.h| 2 +-
>  MdeModulePkg/Include/Protocol/IoMmu.h   | 2 +-
>  MdePkg/Include/Protocol/PciIo.h | 4 ++--
>  8 files changed, 11 insertions(+), 11 deletions(-)
> 
> --
> 2.7.0.windows.1

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

Re: [edk2] [PATCH 0/3] Correct function description for ALLOCATE_BUFFER

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Zeng, Star
> Sent: Monday, December 11, 2017 2:54 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Ni, Ruiyu ; Gao, Liming ; Kinney,
> Michael D 
> Subject: [PATCH 0/3] Correct function description for ALLOCATE_BUFFER
> 
> According to UEFI spec, DUAL_ADDRESS_CYCLE is missing in the
> EFI_UNSUPPORTED return status description for ALLOCATE_BUFFER
> interface.
> Same issue is also in IOMMU interface.
> 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> 
> Star Zeng (3):
>   MdePkg PciIo.h: Correct function description for ALLOCATE_BUFFER
>   MdeModulePkg: Correct function description for AllocateBuffer
>   IntelSilicon: Correct function description for AllocateBuffer
> 
>  IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c | 2 +-
>  IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c   | 2 +-
>  IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c | 2 +-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c  | 4 ++--
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h  | 4 ++--
>  MdeModulePkg/Include/Ppi/IoMmu.h| 2 +-
>  MdeModulePkg/Include/Protocol/IoMmu.h   | 2 +-
>  MdePkg/Include/Protocol/PciIo.h | 4 ++--
>  8 files changed, 11 insertions(+), 11 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 3/3] IntelSilicon: Correct function description for AllocateBuffer

[Star Zeng]

DUAL_ADDRESS_CYCLE is missing in the EFI_UNSUPPORTED
return status description.

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c | 2 +-
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c   | 2 +-
 IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c 
b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
index 7a5f3619a47d..e8685666e79a 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
@@ -300,7 +300,7 @@ IoMmuUnmap (
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c 
b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
index 64693a8c6ec3..89d9bea3fc0f 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
@@ -87,7 +87,7 @@ IoMmuUnmap (
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c 
b/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
index 240a6d281737..7147a19f538a 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
@@ -284,7 +284,7 @@ PeiIoMmuUnmap (
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
-- 
2.7.0.windows.1

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

[edk2] [PATCH 0/3] Correct function description for ALLOCATE_BUFFER

[Star Zeng]

According to UEFI spec, DUAL_ADDRESS_CYCLE is missing in the
EFI_UNSUPPORTED return status description for ALLOCATE_BUFFER interface.
Same issue is also in IOMMU interface.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Liming Gao 
Cc: Michael D Kinney 

Star Zeng (3):
  MdePkg PciIo.h: Correct function description for ALLOCATE_BUFFER
  MdeModulePkg: Correct function description for AllocateBuffer
  IntelSilicon: Correct function description for AllocateBuffer

 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c | 2 +-
 IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c   | 2 +-
 IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c | 2 +-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c  | 4 ++--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h  | 4 ++--
 MdeModulePkg/Include/Ppi/IoMmu.h| 2 +-
 MdeModulePkg/Include/Protocol/IoMmu.h   | 2 +-
 MdePkg/Include/Protocol/PciIo.h | 4 ++--
 8 files changed, 11 insertions(+), 11 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/3] MdeModulePkg: Correct function description for AllocateBuffer

[Star Zeng]

DUAL_ADDRESS_CYCLE is missing in the EFI_UNSUPPORTED
return status description.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 4 ++--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h | 4 ++--
 MdeModulePkg/Include/Ppi/IoMmu.h   | 2 +-
 MdeModulePkg/Include/Protocol/IoMmu.h  | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index cc7125e4fc3c..190f4b0dc7ed 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1081,7 +1081,7 @@ PciIoUnmap (
 
 /**
   Allocates pages that are suitable for an 
EfiPciIoOperationBusMasterCommonBuffer
-  mapping.
+  or EfiPciOperationBusMasterCommonBuffer64 mapping.
 
   @param  This  A pointer to the EFI_PCI_IO_PROTOCOL instance.
   @param  Type  This parameter is not used and must be ignored.
@@ -1094,7 +1094,7 @@ PciIoUnmap (
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h
index da3de3938e4b..b7e38ded3fa4 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h
@@ -411,7 +411,7 @@ PciIoUnmap (
 
 /**
   Allocates pages that are suitable for an 
EfiPciIoOperationBusMasterCommonBuffer
-  mapping.
+  or EfiPciOperationBusMasterCommonBuffer64 mapping.
 
   @param  This  A pointer to the EFI_PCI_IO_PROTOCOL instance.
   @param  Type  This parameter is not used and must be ignored.
@@ -424,7 +424,7 @@ PciIoUnmap (
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
diff --git a/MdeModulePkg/Include/Ppi/IoMmu.h b/MdeModulePkg/Include/Ppi/IoMmu.h
index 5303d68b0774..ed8cd540a21f 100644
--- a/MdeModulePkg/Include/Ppi/IoMmu.h
+++ b/MdeModulePkg/Include/Ppi/IoMmu.h
@@ -141,7 +141,7 @@ EFI_STATUS
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
diff --git a/MdeModulePkg/Include/Protocol/IoMmu.h 
b/MdeModulePkg/Include/Protocol/IoMmu.h
index 9d25c17a5e94..1e06efeb6ae8 100644
--- a/MdeModulePkg/Include/Protocol/IoMmu.h
+++ b/MdeModulePkg/Include/Protocol/IoMmu.h
@@ -203,7 +203,7 @@ EFI_STATUS
 
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
-- 
2.7.0.windows.1

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

[edk2] [PATCH 1/3] MdePkg PciIo.h: Correct function description for ALLOCATE_BUFFER

[Star Zeng]

DUAL_ADDRESS_CYCLE is missing in the EFI_UNSUPPORTED
return status description.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Liming Gao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdePkg/Include/Protocol/PciIo.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Protocol/PciIo.h b/MdePkg/Include/Protocol/PciIo.h
index 42bcc8183077..d6492fcf0436 100644
--- a/MdePkg/Include/Protocol/PciIo.h
+++ b/MdePkg/Include/Protocol/PciIo.h
@@ -331,7 +331,7 @@ EFI_STATUS
 
 /** 
   Allocates pages that are suitable for an 
EfiPciIoOperationBusMasterCommonBuffer
-  mapping. 
  
+  or EfiPciOperationBusMasterCommonBuffer64 mapping.
 
   @param  This  A pointer to the EFI_PCI_IO_PROTOCOL instance.
   @param  Type  This parameter is not used and must be ignored.
@@ -344,7 +344,7 @@ EFI_STATUS
   
   @retval EFI_SUCCESS   The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED   Attributes is unsupported. The only legal 
attribute bits are
-MEMORY_WRITE_COMBINE and MEMORY_CACHED.
 
+MEMORY_WRITE_COMBINE, MEMORY_CACHED and 
DUAL_ADDRESS_CYCLE.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.  

-- 
2.7.0.windows.1

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

Re: [edk2] [PATCH] SecurityPkg:Tcg2Smm: Add MSFT copyright

[Long, Qin]

Reviewed-by: Long Qin 


Best Regards & Thanks,
LONG, Qin

-Original Message-
From: Zhang, Chao B 
Sent: Monday, December 11, 2017 9:34 AM
To: edk2-devel@lists.01.org
Cc: Long, Qin ; Yao, Jiewen ; Zhang, 
Chao B 
Subject: [PATCH] SecurityPkg:Tcg2Smm: Add MSFT copyright

Add MSFT copyright for TPM SIRQ feature.

Cc: Long Qin 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl 
index 68b5073..76a8a13 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
@@ -4,6 +4,7 @@
 
 Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.  
(c)Copyright 2016 HP Development Company, L.P.
+Copyright (c) 2017, Microsoft 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
--
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] Build spec: Add clarification for the binary cache files

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Monday, December 04, 2017 11:45 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Kinney, Michael D
>; Shaw, Kevin W 
>Subject: [Patch] Build spec: Add clarification for the binary cache files
>
>Add clarification for the detail files that would be copied into the
>directory specified by --binary-destination.
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Cc: Kevin W Shaw 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> 8_pre-build_autogen_stage/82_auto-generation_process.md | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/8_pre-build_autogen_stage/82_auto-generation_process.md
>b/8_pre-build_autogen_stage/82_auto-generation_process.md
>index f2ddf32..f610185 100644
>--- a/8_pre-build_autogen_stage/82_auto-generation_process.md
>+++ b/8_pre-build_autogen_stage/82_auto-generation_process.md
>@@ -1043,13 +1043,13 @@ option use md5 method to get every hash value,
>DSC/FDF, tools_def.txt, build_rul
> and build command are calculated as global hash value, Package DEC and its
>include
> header files are calculated as package hash value, Module source files and its
>INF
> file are calculated as module hash value. Library hash value will combine the
>global
> hash value and its dependent package hash value. Driver hash value will
>combine the
> global hash value, its dependent package hash value and its linked library
>hash value.
>-When --hash and --binary-destination are specified, build tool will copy the
>generated
>-binary files for each module into the directory specified by 
>binary-destination
>at the
>-build phase. Binary-destination directory caches all the generated binary 
>files.
>+When --hash and --binary-destination are specified, build tool will copy each
>module's
>+"As Built" inf file, binary files that in "As built" inf file's [Binaries] 
>section and
>+hash value file into the directory specified by binary-destination at the 
>build
>phase.
> When --hash and --binary-source are specified, build tool will try to get the
>binary
> files from the binary source directory at the build phase. If the cached 
> binary
>has
> the same hash value, it will be directly used. Otherwise, build tool will 
> compile
>the
> source files and generate the binary files.
>
>--
>2.6.1.windows.1

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

[edk2] [PATCH 2/2] MdeModulePkg/PiSmmCore: Fix issues in Heap Guard

[Jian J Wang]

One issue is that macros defined in HeapGuard.h

GUARD_HEAP_TYPE_PAGE
GUARD_HEAP_TYPE_POOL

doesn't match the definition of PCD PcdHeapGuardPropertyMask in
MdeModulePkg.dec. This patch fixed it by exchanging the BIT0 and BIT1
of them.

Another is that method AdjustMemoryF() will return a bigger NumberOfPages than
the value passed in. This is caused by counting twice of a shared Guard page
which can be used for both head and tail Guard of the memory before it and
after it. This happens only when partially freeing just one page in the middle
of a bunch of allocated pages. The freed page should be turned into a new
Guard page.

Cc: Jie Lin 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 91 +++--
 MdeModulePkg/Core/PiSmmCore/HeapGuard.h |  4 +-
 2 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c 
b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
index 1d5fb8cdb5..04fa1747a1 100644
--- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
+++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
@@ -776,6 +776,7 @@ UnsetGuardForMemory (
   )
 {
   EFI_PHYSICAL_ADDRESS  GuardPage;
+  UINT64GuardBitmap;
 
   if (NumberOfPages == 0) {
 return;
@@ -784,16 +785,29 @@ UnsetGuardForMemory (
   //
   // Head Guard must be one page before, if any.
   //
+  //  MSB-> 1 0 <-LSB
+  //  ---
+  //  Head Guard -> 0 1 -> Don't free Head Guard  (shared Guard)
+  //  Head Guard -> 0 0 -> Free Head Guard either (not shared Guard)
+  //1 X -> Don't free first page  (need a new Guard)
+  //   (it'll be turned into a Guard page later)
+  //  ---
+  //  Start -> -1-2
+  //
   GuardPage = Memory - EFI_PAGES_TO_SIZE (1);
-  if (IsHeadGuard (GuardPage)) {
-if (!IsMemoryGuarded (GuardPage - EFI_PAGES_TO_SIZE (1))) {
+  GuardBitmap = GetGuardedMemoryBits (Memory - EFI_PAGES_TO_SIZE (2), 2);
+  if ((GuardBitmap & BIT1) == 0) {
+//
+// Head Guard exists.
+//
+if ((GuardBitmap & BIT0) == 0) {
   //
   // If the head Guard is not a tail Guard of adjacent memory block,
   // unset it.
   //
   UnsetGuardPage (GuardPage);
 }
-  } else if (IsMemoryGuarded (GuardPage)) {
+  } else {
 //
 // Pages before memory to free are still in Guard. It's a partial free
 // case. Turn first page of memory block to free into a new Guard.
@@ -804,16 +818,29 @@ UnsetGuardForMemory (
   //
   // Tail Guard must be the page after this memory block to free, if any.
   //
+  //   MSB-> 1 0 <-LSB
+  //  
+  // 1 0 <- Tail Guard -> Don't free Tail Guard  (shared Guard)
+  // 0 0 <- Tail Guard -> Free Tail Guard either (not shared Guard)
+  // X 1   -> Don't free last page   (need a new Guard)
+  // (it'll be turned into a Guard page later)
+  //  
+  //+1+0 <- End
+  //
   GuardPage = Memory + EFI_PAGES_TO_SIZE (NumberOfPages);
-  if (IsTailGuard (GuardPage)) {
-if (!IsMemoryGuarded (GuardPage + EFI_PAGES_TO_SIZE (1))) {
+  GuardBitmap = GetGuardedMemoryBits (GuardPage, 2);
+  if ((GuardBitmap & BIT0) == 0) {
+//
+// Tail Guard exists.
+//
+if ((GuardBitmap & BIT1) == 0) {
   //
   // If the tail Guard is not a head Guard of adjacent memory block,
   // free it; otherwise, keep it.
   //
   UnsetGuardPage (GuardPage);
 }
-  } else if (IsMemoryGuarded (GuardPage)) {
+  } else {
 //
 // Pages after memory to free are still in Guard. It's a partial free
 // case. We need to keep one page to be a head Guard.
@@ -903,6 +930,7 @@ AdjustMemoryF (
   EFI_PHYSICAL_ADDRESS  Start;
   EFI_PHYSICAL_ADDRESS  MemoryToTest;
   UINTN PagesToFree;
+  UINT64GuardBitmap;
 
   if (Memory == NULL || NumberOfPages == NULL || *NumberOfPages == 0) {
 return;
@@ -914,9 +942,22 @@ AdjustMemoryF (
   //
   // Head Guard must be one page before, if any.
   //
-  MemoryToTest = Start - EFI_PAGES_TO_SIZE (1);
-  if (IsHeadGuard (MemoryToTest)) {
-if (!IsMemoryGuarded (MemoryToTest - EFI_PAGES_TO_SIZE (1))) {
+  //  MSB-> 1 0 <-LSB
+  //  ---
+  //  Head Guard -> 0 1 -> Don't free Head Guard  (shared Guard)
+  //  Head Guard -> 0 0 -> Free Head Guard either (not shared Guard)
+  //1 X -> Don't free first page  (need a new Guard)
+  //   (it'll be turned into a Guard page later)
+  //  ---
+  //  Start -> -1-2
+  //
+  MemoryToTest = Start - EFI_PAGES_TO_SIZE (2);
+  GuardBitmap = GetGuardedMemoryBits 

[edk2] [PATCH 0/2] Fix issues in Heap Guard

[Jian J Wang]

One issue is that macros defined in HeapGuard.h

GUARD_HEAP_TYPE_PAGE
GUARD_HEAP_TYPE_POOL

doesn't match the definition of PCD PcdHeapGuardPropertyMask in
MdeModulePkg.dec. This patch fixed it by exchanging the BIT0 and BIT1
of them.

Another is that method AdjustMemoryF() will return a bigger NumberOfPages than
the value passed in. This is caused by counting twice of a shared Guard page
which can be used for both head and tail Guard of the memory before it and
after it. This happens only when partially freeing just one page in the middle
of a bunch of allocated pages. The freed page should be turned into a new
Guard page.

Since the most part code of Heap Guard feature are almost the same,
PiSmmCore and DxeCore have both above issues. This patch series fix them all.

Jian J Wang (2):
  MdeModulePkg/DxeCore: Fix issues in Heap Guard
  MdeModulePkg/PiSmmCore: Fix issues in Heap Guard

 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c   | 93 ++---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h   |  4 +-
 MdeModulePkg/Core/Dxe/Mem/Page.c| 15 +++---
 MdeModulePkg/Core/Dxe/Mem/Pool.c|  4 +-
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 91 ++--
 MdeModulePkg/Core/PiSmmCore/HeapGuard.h |  4 +-
 6 files changed, 164 insertions(+), 47 deletions(-)

-- 
2.15.1.windows.2

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

[edk2] [PATCH 1/2] MdeModulePkg/DxeCore: Fix issues in Heap Guard

[Jian J Wang]

One issue is that macros defined in HeapGuard.h

GUARD_HEAP_TYPE_PAGE
GUARD_HEAP_TYPE_POOL

doesn't match the definition of PCD PcdHeapGuardPropertyMask in
MdeModulePkg.dec. This patch fixed it by exchanging the BIT0 and BIT1
of them.

Another is that method AdjustMemoryF() will return a bigger NumberOfPages than
the value passed in. This is caused by counting twice of a shared Guard page
which can be used for both head and tail Guard of the memory before it and
after it. This happens only when partially freeing just one page in the middle
of a bunch of allocated pages. The freed page should be turned into a new
Guard page.

Cc: Jie Lin 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 93 ---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  4 +-
 MdeModulePkg/Core/Dxe/Mem/Page.c  | 15 +++---
 MdeModulePkg/Core/Dxe/Mem/Pool.c  |  4 +-
 4 files changed, 88 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 3a829854af..bee229f4c8 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -768,6 +768,7 @@ UnsetGuardForMemory (
   )
 {
   EFI_PHYSICAL_ADDRESS  GuardPage;
+  UINT64GuardBitmap;
 
   if (NumberOfPages == 0) {
 return;
@@ -776,16 +777,29 @@ UnsetGuardForMemory (
   //
   // Head Guard must be one page before, if any.
   //
+  //  MSB-> 1 0 <-LSB
+  //  ---
+  //  Head Guard -> 0 1 -> Don't free Head Guard  (shared Guard)
+  //  Head Guard -> 0 0 -> Free Head Guard either (not shared Guard)
+  //1 X -> Don't free first page  (need a new Guard)
+  //   (it'll be turned into a Guard page later)
+  //  ---
+  //  Start -> -1-2
+  //
   GuardPage = Memory - EFI_PAGES_TO_SIZE (1);
-  if (IsHeadGuard (GuardPage)) {
-if (!IsMemoryGuarded (GuardPage - EFI_PAGES_TO_SIZE (1))) {
+  GuardBitmap = GetGuardedMemoryBits (Memory - EFI_PAGES_TO_SIZE (2), 2);
+  if ((GuardBitmap & BIT1) == 0) {
+//
+// Head Guard exists.
+//
+if ((GuardBitmap & BIT0) == 0) {
   //
   // If the head Guard is not a tail Guard of adjacent memory block,
   // unset it.
   //
   UnsetGuardPage (GuardPage);
 }
-  } else if (IsMemoryGuarded (GuardPage)) {
+  } else {
 //
 // Pages before memory to free are still in Guard. It's a partial free
 // case. Turn first page of memory block to free into a new Guard.
@@ -796,16 +810,29 @@ UnsetGuardForMemory (
   //
   // Tail Guard must be the page after this memory block to free, if any.
   //
+  //   MSB-> 1 0 <-LSB
+  //  
+  // 1 0 <- Tail Guard -> Don't free Tail Guard  (shared Guard)
+  // 0 0 <- Tail Guard -> Free Tail Guard either (not shared Guard)
+  // X 1   -> Don't free last page   (need a new Guard)
+  // (it'll be turned into a Guard page later)
+  //  
+  //+1+0 <- End
+  //
   GuardPage = Memory + EFI_PAGES_TO_SIZE (NumberOfPages);
-  if (IsTailGuard (GuardPage)) {
-if (!IsMemoryGuarded (GuardPage + EFI_PAGES_TO_SIZE (1))) {
+  GuardBitmap = GetGuardedMemoryBits (GuardPage, 2);
+  if ((GuardBitmap & BIT0) == 0) {
+//
+// Tail Guard exists.
+//
+if ((GuardBitmap & BIT1) == 0) {
   //
   // If the tail Guard is not a head Guard of adjacent memory block,
   // free it; otherwise, keep it.
   //
   UnsetGuardPage (GuardPage);
 }
-  } else if (IsMemoryGuarded (GuardPage)) {
+  } else {
 //
 // Pages after memory to free are still in Guard. It's a partial free
 // case. We need to keep one page to be a head Guard.
@@ -895,6 +922,7 @@ AdjustMemoryF (
   EFI_PHYSICAL_ADDRESS  Start;
   EFI_PHYSICAL_ADDRESS  MemoryToTest;
   UINTN PagesToFree;
+  UINT64GuardBitmap;
 
   if (Memory == NULL || NumberOfPages == NULL || *NumberOfPages == 0) {
 return;
@@ -906,9 +934,22 @@ AdjustMemoryF (
   //
   // Head Guard must be one page before, if any.
   //
-  MemoryToTest = Start - EFI_PAGES_TO_SIZE (1);
-  if (IsHeadGuard (MemoryToTest)) {
-if (!IsMemoryGuarded (MemoryToTest - EFI_PAGES_TO_SIZE (1))) {
+  //  MSB-> 1 0 <-LSB
+  //  ---
+  //  Head Guard -> 0 1 -> Don't free Head Guard  (shared Guard)
+  //  Head Guard -> 0 0 -> Free Head Guard either (not shared Guard)
+  //1 X -> Don't free first page  (need a new Guard)
+  //   (it'll be turned into a Guard page later)
+  //  ---
+  //  Start -> -1-2
+  //
+  

[edk2] [PATCH] SecurityPkg:Tcg2Smm: Add MSFT copyright

[Zhang, Chao B]

Add MSFT copyright for TPM SIRQ feature.

Cc: Long Qin 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
index 68b5073..76a8a13 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
@@ -4,6 +4,7 @@
 
 Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
 (c)Copyright 2016 HP Development Company, L.P.
+Copyright (c) 2017, Microsoft 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 
-- 
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] MdeModulePkg: Free NET_BUF data after it is sent out to avoid memory leak

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Wang, Fan
> Sent: Monday, November 27, 2017 2:44 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting ; Fu,
> Siyuan ; Wang, Fan 
> Subject: [Patch v2] MdeModulePkg: Free NET_BUF data after it is sent out
> to avoid memory leak
> 
> V2:
> * Since packet has already been referred by DhcpSb->LastPacket, and will
> be
> freed when sending another packet or clean up, there is no need to add an
> extra free function in NetbufFromExt.
> 
> Cc: Jiaxin Wu 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h | 12 
>  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c   | 13 +
>  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h
> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h
> index e546a08..57f6d5e 100644
> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h
> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h
> @@ -182,10 +182,22 @@ VOID
>  DhcpCleanConfigure (
>IN OUT EFI_DHCP4_CONFIG_DATA  *Config
>);
> 
>  /**
> +  Callback of Dhcp packet. Does nothing.
> +
> +  @param Arg   The context.
> +
> +**/
> +VOID
> +EFIAPI
> +DhcpDummyExtFree (
> +  IN VOID   *Arg
> +  );
> +
> +/**
>Set the elapsed time based on the given instance and the pointer to the
>elapsed time option.
> 
>@param[in]  Elapsed   The pointer to the position to append.
>@param[in]  Instance  The pointer to the Dhcp4 instance.
> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> index 3898223..54a610a 100644
> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> @@ -1357,17 +1357,16 @@ DhcpSendMessage (
>  >ClientAddressSendOut[0],
>  >Dhcp4.Header.ClientHwAddr[0],
>  Packet->Dhcp4.Header.HwAddrLen
>  );
> 
> -
>//
>// Wrap it into a netbuf then send it.
>//
>Frag.Bulk = (UINT8 *) >Dhcp4.Header;
>Frag.Len  = Packet->Length;
> -  Wrap  = NetbufFromExt (, 1, 0, 0, DhcpReleasePacket, Packet);
> +  Wrap  = NetbufFromExt (, 1, 0, 0, DhcpDummyExtFree, NULL);
> 
>if (Wrap == NULL) {
>  FreePool (Packet);
>  return EFI_OUT_OF_RESOURCES;
>}
> @@ -1397,11 +1396,10 @@ DhcpSendMessage (
>  EndPoint.LocalAddr.Addr[0]  = DhcpSb->ClientAddr;
>  UdpIo   = DhcpSb->LeaseIoPort;
>}
> 
>ASSERT (UdpIo != NULL);
> -  NET_GET_REF (Wrap);
> 
>Status = UdpIoSendDatagram (
>   UdpIo,
>   Wrap,
>   ,
> @@ -1409,11 +1407,11 @@ DhcpSendMessage (
>   DhcpOnPacketSent,
>   DhcpSb
>   );
> 
>if (EFI_ERROR (Status)) {
> -NET_PUT_REF (Wrap);
> +NetbufFree (Wrap);
>  return EFI_ACCESS_DENIED;
>}
> 
>return EFI_SUCCESS;
>  }
> @@ -1452,16 +1450,16 @@ DhcpRetransmit (
>//
>// Wrap it into a netbuf then send it.
>//
>Frag.Bulk = (UINT8 *) >LastPacket->Dhcp4.Header;
>Frag.Len  = DhcpSb->LastPacket->Length;
> -  Wrap  = NetbufFromExt (, 1, 0, 0, DhcpReleasePacket, DhcpSb-
> >LastPacket);
> +  Wrap  = NetbufFromExt (, 1, 0, 0, DhcpDummyExtFree, NULL);
> 
>if (Wrap == NULL) {
>  return EFI_OUT_OF_RESOURCES;
>}
> -
> +
>//
>// Broadcast the message, unless we know the server address.
>//
>EndPoint.RemotePort = DHCP_SERVER_PORT;
>EndPoint.LocalPort  = DHCP_CLIENT_PORT;
> @@ -1475,22 +1473,21 @@ DhcpRetransmit (
>  UdpIo   = DhcpSb->LeaseIoPort;
>}
> 
>ASSERT (UdpIo != NULL);
> 
> -  NET_GET_REF (Wrap);
>Status = UdpIoSendDatagram (
>   UdpIo,
>   Wrap,
>   ,
>   NULL,
>   DhcpOnPacketSent,
>   DhcpSb
>   );
> 
>if (EFI_ERROR (Status)) {
> -NET_PUT_REF (Wrap);
> +NetbufFree (Wrap);
>  return EFI_ACCESS_DENIED;
>}
> 
>return EFI_SUCCESS;
>  }
> --
> 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] MdeModulePkg/ScsiDisk: Return EFI_NO_MEDIA when no media presents

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, December 7, 2017 5:55 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Zeng, Star 
Subject: [edk2] [PATCH] MdeModulePkg/ScsiDisk: Return EFI_NO_MEDIA when no 
media presents

Current code always return EFI_MEDIA_CHANGED no matter the media is removed 
from CD/DVD drive or the media is changed.
It doesn't strictly follow the UEFI Spec.
Update code to return EFI_NO_MEDIA when media is removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Star Zeng 
Cc: Hao A Wu 
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 41 
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index 2289f20152..6a0a193556 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -568,6 +568,7 @@ ScsiDiskReadBlocks (
   MediaChange= FALSE;
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO (This);
+  Media  = ScsiDiskDevice->BlkIo.Media;
 
   if (!IS_DEVICE_FIXED(ScsiDiskDevice)) {
 
@@ -598,14 +599,17 @@ ScsiDiskReadBlocks (
>EraseBlock
);
   }
-  Status = EFI_MEDIA_CHANGED;
+  if (Media->MediaPresent) {
+Status = EFI_MEDIA_CHANGED;
+  } else {
+Status = EFI_NO_MEDIA;
+  }
   goto Done;
 }
   }
   //
   // Get the intrinsic block size
   //
-  Media   = ScsiDiskDevice->BlkIo.Media;
   BlockSize   = Media->BlockSize;
 
   NumberOfBlocks  = BufferSize / BlockSize; @@ -700,6 +704,7 @@ 
ScsiDiskWriteBlocks (
   MediaChange= FALSE;
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO (This);
+  Media  = ScsiDiskDevice->BlkIo.Media;
 
   if (!IS_DEVICE_FIXED(ScsiDiskDevice)) {
 
@@ -730,14 +735,17 @@ ScsiDiskWriteBlocks (
>EraseBlock
);
   }
-  Status = EFI_MEDIA_CHANGED;
+  if (Media->MediaPresent) {
+Status = EFI_MEDIA_CHANGED;
+  } else {
+Status = EFI_NO_MEDIA;
+  }
   goto Done;
 }
   }
   //
   // Get the intrinsic block size
   //
-  Media   = ScsiDiskDevice->BlkIo.Media;
   BlockSize   = Media->BlockSize;
 
   NumberOfBlocks  = BufferSize / BlockSize; @@ -922,6 +930,7 @@ 
ScsiDiskReadBlocksEx (
   MediaChange= FALSE;
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
+  Media  = ScsiDiskDevice->BlkIo.Media;
 
   if (!IS_DEVICE_FIXED(ScsiDiskDevice)) {
 
@@ -952,14 +961,17 @@ ScsiDiskReadBlocksEx (
>EraseBlock
);
   }
-  Status = EFI_MEDIA_CHANGED;
+  if (Media->MediaPresent) {
+Status = EFI_MEDIA_CHANGED;
+  } else {
+Status = EFI_NO_MEDIA;
+  }
   goto Done;
 }
   }
   //
   // Get the intrinsic block size
   //
-  Media   = ScsiDiskDevice->BlkIo2.Media;
   BlockSize   = Media->BlockSize;
 
   NumberOfBlocks  = BufferSize / BlockSize; @@ -1081,6 +1093,7 @@ 
ScsiDiskWriteBlocksEx (
   MediaChange= FALSE;
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
+  Media  = ScsiDiskDevice->BlkIo.Media;
 
   if (!IS_DEVICE_FIXED(ScsiDiskDevice)) {
 
@@ -,14 +1124,17 @@ ScsiDiskWriteBlocksEx (
>EraseBlock
);
   }
-  Status = EFI_MEDIA_CHANGED;
+  if (Media->MediaPresent) {
+Status = EFI_MEDIA_CHANGED;
+  } else {
+Status = EFI_NO_MEDIA;
+  }
   goto Done;
 }
   }
   //
   // Get the intrinsic block size
   //
-  Media   = ScsiDiskDevice->BlkIo2.Media;
   BlockSize   = Media->BlockSize;
 
   NumberOfBlocks  = BufferSize / BlockSize; @@ -1230,6 +1246,7 @@ 
ScsiDiskFlushBlocksEx (
   MediaChange= FALSE;
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
+  Media  = ScsiDiskDevice->BlkIo.Media;
 
   if (!IS_DEVICE_FIXED(ScsiDiskDevice)) {
 
@@ -1260,13 +1277,15 @@ ScsiDiskFlushBlocksEx (
>EraseBlock
);
   }
-  Status = EFI_MEDIA_CHANGED;
+  if (Media->MediaPresent) {
+Status = EFI_MEDIA_CHANGED;
+  } else {
+Status = EFI_NO_MEDIA;
+  }
   goto Done;
 }
   }
 
-  Media = ScsiDiskDevice->BlkIo2.Media;
-
   if (!(Media->MediaPresent)) {
 Status = EFI_NO_MEDIA;
 goto Done;
--
2.15.0.gvfs.1.preview.4

___
edk2-devel mailing list
edk2-devel@lists.01.org

Re: [edk2] [PATCH] MdeModulePkg: loose VA_COPY with no matching VA_END on a return path

[Zeng, Star]

Reviewed-by: Star Zeng  and pushed at 
a0460be7e88539649cd10139491dd6955c54fb14.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zenith432
Sent: Sunday, December 10, 2017 1:05 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg: loose VA_COPY with no matching VA_END on 
a return path

In CheckRemainingSpaceForConsistencyInternal, one of the return paths leaves a 
loose VA_COPY with no matching VA_END.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zenith432 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index f39be6b0..969df955 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -1765,6 +1765,7 @@ CheckRemainingSpaceForConsistencyInternal (
   //
   // No enough space for Variable[Index].
   //
+  VA_END (Args);
   return FALSE;
 }
 //
--
2.14.3 (Apple Git-98)
___
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] PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 22:18, Michael Zimmermann
 wrote:
> Exactly. If I shift the pointer by 4 bytes from within PrePiLib the device
> boots just fine.
>
> I'm not sure if the size is the root cause but right now it only happens
> when adding a ~15mb binary efi to the end of fvmain.
>

It appears so.

So it would be good to check where the disparity originates.

The Ffs section containing the compressed FV looks like this for ArmVirtQemu

$ hexdump -C 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792SEC1.1fv.sec |head

  84 a8 48 17 00 00 00 00  00 00 00 00 00 00 00 00  |..H.|
0010  00 00 00 00 78 e5 8c 8c  3d 8a 1c 4f 99 35 89 61  |x...=..O.5.a|
0020  85 c3 2d d3 80 a8 48 00  00 00 00 00 5f 46 56 48  |..-...H._FVH|
0030  ff fe 04 00 48 00 24 2b  00 00 00 02 a2 22 01 00  |H.$+."..|
0040  40 00 00 00 00 00 00 00  00 00 00 00 7f cb a2 d6  |@...|
0050  18 6a 2f 4e b4 3b 99 20  a7 33 70 0a 4d aa 05 00  |.j/N.;. .3p.M...|
0060  30 c0 01 f8 04 c0 01 10  4d 5a 00 00 00 00 00 00  |0...MZ..|
0070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
*
00a0  00 00 00 00 58 0e 00 00  00 00 00 00 00 00 00 00  |X...|

Could you compare with your build?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

Exactly. If I shift the pointer by 4 bytes from within PrePiLib the device
boots just fine.

I'm not sure if the size is the root cause but right now it only happens
when adding a ~15mb binary efi to the end of fvmain.

On Dec 10, 2017 11:13 PM, "Ard Biesheuvel" 
wrote:

On 10 December 2017 at 21:58, Michael Zimmermann
 wrote:
> VolInfo doesn't seem to complain about FVMAIN_COMPACT.Fv:
> https://pastebin.com/ueUnepXF
>
> Does VolInfo support Fs's as well? because it fails on mine(even on a
> working one).
>

You mean to dissect the FFS that contains the FV? Not sure.

 So the _FVH signature is there but shifted by 4 bytes, right? Does
that only happen with large FVs?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 21:58, Michael Zimmermann
 wrote:
> VolInfo doesn't seem to complain about FVMAIN_COMPACT.Fv:
> https://pastebin.com/ueUnepXF
>
> Does VolInfo support Fs's as well? because it fails on mine(even on a
> working one).
>

You mean to dissect the FFS that contains the FV? Not sure.

 So the _FVH signature is there but shifted by 4 bytes, right? Does
that only happen with large FVs?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

VolInfo doesn't seem to complain about FVMAIN_COMPACT.Fv:
https://pastebin.com/ueUnepXF

Does VolInfo support Fs's as well? because it fails on mine(even on a
working one).

On Sun, Dec 10, 2017 at 10:49 PM, Ard Biesheuvel
 wrote:
> On 10 December 2017 at 21:45, Michael Zimmermann
>  wrote:
>> The layout is basically the same as in ArmVirtQemuKernel.fdf but I've
>> changed the size to 8MB and these are the usage stats:
>> FV Space Information
>> FVMAINDEVICE [99%Full] 57856 total, 57800 used, 56 free
>> FVMAINMSM8960 [99%Full] 222336 total, 222312 used, 24 free
>> FVMAIN_COMPACT [63%Full] 8380416 total, 5305944 used, 3074472 free
>> FVMAIN [99%Full] 17026304 total, 17026264 used, 40 free
>> FVMAINQCOM [99%Full] 37184 total, 37152 used, 32 free
>>
>> and here's an uefitool report:
>> https://pastebin.com/pnHNSFz4
>> The padding in line 13 is the one causing trouble. The interesting
>> part is that it's not zero. it's value is '08 CD 03 01'.
>>
>> PrePiLib fails in this line:
>> https://github.com/tianocore/edk2/blob/5a44a766b597e4c9960ac1936e6d18001c5e7ce2/EmbeddedPkg/Library/PrePiLib/FwVol.c#L682
>> The reason is simply that VolumeHandle is offset by 4 bytes.
>>
>
> That code is identical to PeiFfsFvPpiGetVolumeInfo() in PEI core so
> this does not look specific to PrePiLib.
>
> Does VolInfo complain as well?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 21:45, Michael Zimmermann
 wrote:
> The layout is basically the same as in ArmVirtQemuKernel.fdf but I've
> changed the size to 8MB and these are the usage stats:
> FV Space Information
> FVMAINDEVICE [99%Full] 57856 total, 57800 used, 56 free
> FVMAINMSM8960 [99%Full] 222336 total, 222312 used, 24 free
> FVMAIN_COMPACT [63%Full] 8380416 total, 5305944 used, 3074472 free
> FVMAIN [99%Full] 17026304 total, 17026264 used, 40 free
> FVMAINQCOM [99%Full] 37184 total, 37152 used, 32 free
>
> and here's an uefitool report:
> https://pastebin.com/pnHNSFz4
> The padding in line 13 is the one causing trouble. The interesting
> part is that it's not zero. it's value is '08 CD 03 01'.
>
> PrePiLib fails in this line:
> https://github.com/tianocore/edk2/blob/5a44a766b597e4c9960ac1936e6d18001c5e7ce2/EmbeddedPkg/Library/PrePiLib/FwVol.c#L682
> The reason is simply that VolumeHandle is offset by 4 bytes.
>

That code is identical to PeiFfsFvPpiGetVolumeInfo() in PEI core so
this does not look specific to PrePiLib.

Does VolInfo complain as well?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

On 10 December 2017 at 21:22, Ard Biesheuvel  wrote:
> cc BaseTools maintainers
>
> On 10 December 2017 at 20:58, Michael Zimmermann
>  wrote:
>> 'uefitool' shows me that there are 4 bytes of padding right before
>> FVMAIN when adding large(20MB) uncompressed FV's.
>>
>> FwVol fails detecting that and complains about the signature not being 
>> correct.
>>

I missed the 'PrePiLib' in the subject, apologies.

So could you elaborate please? What is the layout of the FV, and
when/how does PrePiLib complain about it?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Ard Biesheuvel]

cc BaseTools maintainers

On 10 December 2017 at 20:58, Michael Zimmermann
 wrote:
> 'uefitool' shows me that there are 4 bytes of padding right before
> FVMAIN when adding large(20MB) uncompressed FV's.
>
> FwVol fails detecting that and complains about the signature not being 
> correct.
>
> Thanks
> Michael
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] PrePiLib's FwVol.c can't handle padding before volume header

[Michael Zimmermann]

'uefitool' shows me that there are 4 bytes of padding right before
FVMAIN when adding large(20MB) uncompressed FV's.

FwVol fails detecting that and complains about the signature not being correct.

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

Re: [edk2] [PATCH 4/4] BaseTools: resolve initialization order errors in VfrFormPkg.h

[Zenith432]

On 10/12/2017 03:52 PM, Gao, Liming wrote:

I think these patches resolves CLANG build issues in BaseTools. Do you verify 
them with GCC or VS tool chain?


GCC 7.2 does not give any of the warnings generated by clang while compiling 
BaseTools.
After applying the 4 BaseTools patches, it still does not give any warnings (or 
errors).
I don't have VS toolchain to try on.

For VfrFormPkg.h, there is simpler solution to surround all the offending code 
in

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wuninitialized"

#pragma clang diagnostic pop

I sent you this option last week, you said you prefer to resolve the reason for the 
"-Wuninitialized".
The reason is the attempt in 100 or so derived classes to force an initialization order different than C++.  It's 
undefined behavior.  Any resolution for the undefined behavior involves changing those 100 derived classes, so a massive 
code change.  The existing code works, it's just an attempt to get around standard C++ initialization order.  Clang 
wants to give pedantic warnings, but can be silenced if the "undefined behavioral" code is known to work.


The other 3 BaseTools patches are rather small.

Also, with the VA_START(Args, BOOLEAN) bug (bug 741), there is an option of silencing clang locally around the offending 
statement

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wvarargs"
VA_START(Args, Iso639Language);
#pragma clang diagnostic pop

And then remove global silencing of this warning in tools_def.txt.  That way any other not yet considered warnings will 
be caught in the future.


It's up to you how to go about it...  big code changes, silencing warning 
locally or silencing warning globally.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: correct and clarify documentation of VA_LIST in Base.h

[Zenith432]

On 10/12/2017 03:53 PM, Gao, Liming wrote:

Could you add bug 457 link in the commit message?

---
Subject: [PATCH] MdePkg: correct and clarify documentation of VA_LIST in Base.h

This is to resolve bug 457.
https://bugzilla.tianocore.org/show_bug.cgi?id=457

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zenith432 
---
  MdePkg/Include/Base.h | 24 
  1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 02140a5a..19f36872 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -560,13 +560,14 @@ struct _LIST_ENTRY {
  //  VA_LIST  - typedef for argument list.
  //  VA_START (VA_LIST Marker, argument before the ...) - Init Marker for use.
  //  VA_END (VA_LIST Marker) - Clear Marker
-//  VA_ARG (VA_LIST Marker, var arg size) - Use Marker to get an argument from
-//the ... list. You must know the size and pass it in this macro.
+//  VA_ARG (VA_LIST Marker, var arg type) - Use Marker to get an argument from
+//the ... list. You must know the type and pass it in this macro.
  //  VA_COPY (VA_LIST Dest, VA_LIST Start) - Initialize Dest as a copy of 
Start.
  //
-//  example:
+//  Example:
  //
  //  UINTN
+//  EFIAPI
  //  ExampleVarArg (
  //IN UINTN  NumberOfArgs,
  //...
@@ -582,7 +583,7 @@ struct _LIST_ENTRY {
  //VA_START (Marker, NumberOfArgs);
  //for (Index = 0, Result = 0; Index < NumberOfArgs; Index++) {
  //  //
-//  // The ... list is a series of UINTN values, so average them up.
+//  // The ... list is a series of UINTN values, so sum them up.
  //  //
  //  Result += VA_ARG (Marker, UINTN);
  //}
@@ -591,6 +592,21 @@ struct _LIST_ENTRY {
  //return Result
  //  }
  //
+//  Notes:
+//
+//  This set of macros is intended to support variadic functions that
+//  use the EFIAPI calling convention.  Variadic functions that use a
+//  native calling convention should use stdarg.h.
+//  In particular:
+//  -- VA_START may only be used in a variadic EFIAPI function.
+//  -- va_start may only be used in a variadic native function.
+//  -- VA_START, VA_END, VA_ARG and VA_COPY may only be used on a VA_LIST.
+//  -- va_start, va_end, va_arg and va_copy may only be used on a va_list.
+//  -- Both VA_LIST or va_list may be passed as arguments to functions
+// of either EFIAPI or native calling conventions.
+//  -- VA_END, VA_ARG, VA_COPY, va_end, va_arg, and va_copy may be used
+// in functions of either calling conventions.
+//

  /**
Return the size of argument that has been aligned to sizeof (UINTN).
--
2.14.3
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: resolve bug 741

[Zenith432]

On 10/12/2017 15:52, Gao, Liming wrote:
> For 4, 'unsigned char' goes default argument promotion to int. This 
is CLANG compiler behavior. Does GCC and VS compiler follow this rule?

>
> Disable varargs warning is the temp solution. For long term, we 
expect to figure out the compatible solution. If all supported compilers 
follow this rule, I think this is a compatible change.


It is STD C default argument promotion.  For example, C99 document n1570

section 6.5.2.2 (Function Calls)
"If the expression that denotes the called function has a type that does 
not include a prototype, the integer promotions are performed on each 
argument, and arguments that have type float are promoted to double. 
These are called the default argument promotions."


section 6.3.1.1 (Booleans, characters and integers)
"If an int can represent all values of the original type (as restricted 
by the width, for a bit-field), the value is converted to an int; 
otherwise, it is converted to an unsigned int. These are called the 
integer promotions."


section 7.16.1.4 (The va_start macro)
"If the parameter parmN is declared with the register storage class, 
with a function or array type, or with a type that is not compatible 
with the type that results after application of the default argument 
promotions, the behavior is undefined."


Technically, when there's a prototype that says the parameter is BOOLEAN 
(i.e. unsigned char), there is no "promotion" from the point of view of 
STDC.  Instead, it tries to convert the argument to BOOLEAN (if 
possible.)  Then the BOOLEAN is passed using whatever mechanism the 
implementation has to pass BOOLEANs (which is to fit them in a 4 or 
8-byte granular stack).  However, va_start wants a parameter type that 
is the outcome of a default argument promotion, and the outcome of a 
default argument promotion of 'unsigned char' is always int.


There's more discussion with longer quotes from the STD here
https://stackoverflow.com/questions/1255775/default-argument-promotions-in-c-function-calls
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: correct and clarify documentation of VA_LIST in Base.h

[Gao, Liming]

Could you add bug 457 link in the commit message? 

> -Original Message-
> From: Zenith432 [mailto:zenith...@users.sourceforge.net]
> Sent: Sunday, December 10, 2017 5:32 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming 
> 
> Subject: [PATCH] MdePkg: correct and clarify documentation of VA_LIST in 
> Base.h
> 
> This is to resolve bug 457.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 
> ---
>  MdePkg/Include/Base.h | 24 
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 02140a5a..19f36872 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -560,13 +560,14 @@ struct _LIST_ENTRY {
>  //  VA_LIST  - typedef for argument list.
>  //  VA_START (VA_LIST Marker, argument before the ...) - Init Marker for use.
>  //  VA_END (VA_LIST Marker) - Clear Marker
> -//  VA_ARG (VA_LIST Marker, var arg size) - Use Marker to get an argument 
> from
> -//the ... list. You must know the size and pass it in this macro.
> +//  VA_ARG (VA_LIST Marker, var arg type) - Use Marker to get an argument 
> from
> +//the ... list. You must know the type and pass it in this macro.
>  //  VA_COPY (VA_LIST Dest, VA_LIST Start) - Initialize Dest as a copy of 
> Start.
>  //
> -//  example:
> +//  Example:
>  //
>  //  UINTN
> +//  EFIAPI
>  //  ExampleVarArg (
>  //IN UINTN  NumberOfArgs,
>  //...
> @@ -582,7 +583,7 @@ struct _LIST_ENTRY {
>  //VA_START (Marker, NumberOfArgs);
>  //for (Index = 0, Result = 0; Index < NumberOfArgs; Index++) {
>  //  //
> -//  // The ... list is a series of UINTN values, so average them up.
> +//  // The ... list is a series of UINTN values, so sum them up.
>  //  //
>  //  Result += VA_ARG (Marker, UINTN);
>  //}
> @@ -591,6 +592,21 @@ struct _LIST_ENTRY {
>  //return Result
>  //  }
>  //
> +//  Notes:
> +//
> +//  This set of macros is intended to support variadic functions that
> +//  use the EFIAPI calling convention.  Variadic functions that use a
> +//  native calling convention should use stdarg.h.
> +//  In particular:
> +//  -- VA_START may only be used in a variadic EFIAPI function.
> +//  -- va_start may only be used in a variadic native function.
> +//  -- VA_START, VA_END, VA_ARG and VA_COPY may only be used on a VA_LIST.
> +//  -- va_start, va_end, va_arg and va_copy may only be used on a va_list.
> +//  -- Both VA_LIST or va_list may be passed as arguments to functions
> +// of either EFIAPI or native calling conventions.
> +//  -- VA_END, VA_ARG, VA_COPY, va_end, va_arg, and va_copy may be used
> +// in functions of either calling conventions.
> +//
> 
>  /**
>Return the size of argument that has been aligned to sizeof (UINTN).
> --
> 2.14.3
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: resolve bug 741

[Gao, Liming]

For 4, 'unsigned char' goes default argument promotion to int. This is CLANG 
compiler behavior. Does GCC and VS compiler follow this rule?

Disable varargs warning is the temp solution. For long term, we expect to 
figure out the compatible solution. If all supported compilers follow this 
rule, I think this is a compatible change. 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Zenith432
> Sent: Sunday, December 10, 2017 3:44 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdePkg: resolve bug 741
> 
> This is a proposal to resolve bug 741: UefiLib.c compilation failure with 
> clang
> error: passing an object that undergoes default argument promotion to 
> 'va_start' has undefined behavior [-Werror,-Wvarargs]"
> 
> Rationale:
> 1. Default argument promotion causes the sizeof a function's argument to be 
> different than the corresponding
> parameter's sizeof.  This may break a permissible implemenation of stdarg.h, 
> which is why it is considered
> undefined behavior in C.  The condition should be repaired rather than 
> silenced with -Wno-varargs.
> 2. The warning is due to the last non-variadic parameter of GetBestLanguage 
> having type BOOLEAN.
> 3. BOOLEAN is typedef'd to 'unsigned char' in all ProcessorBind.h.
> 4. 'unsigned char' goes default argument promotion to int.
> 5. All ProcessorBind.h typedef the type INT32 to either 'int' or some 32-bit 
> equivalent.
> 6. As a result it is safe to replace the type of the parameter from BOOLEAN 
> to INT32 in all
> current supported architectures.
> 7. The change is consistent with the BOOLEAN argument being converted to 
> INT32 at the caller site. The
> function GetBestLanguage currently converts the argument from INT32 back to 
> BOOLEAN, however
> the function's logic is not disturbed by treating the argument as an INT32.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 
> ---
>  MdePkg/Include/Library/UefiLib.h | 2 +-
>  MdePkg/Library/UefiLib/UefiLib.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h 
> b/MdePkg/Include/Library/UefiLib.h
> index 0b14792a..5d98eb26 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -818,7 +818,7 @@ CHAR8 *
>  EFIAPI
>  GetBestLanguage (
>IN CONST CHAR8  *SupportedLanguages,
> -  IN BOOLEAN  Iso639Language,
> +  IN INT32Iso639Language,
>...
>);
> 
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c 
> b/MdePkg/Library/UefiLib/UefiLib.c
> index a7eee012..57236511 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1514,7 +1514,7 @@ CHAR8 *
>  EFIAPI
>  GetBestLanguage (
>IN CONST CHAR8  *SupportedLanguages,
> -  IN BOOLEAN  Iso639Language,
> +  IN INT32Iso639Language,
>...
>)
>  {
> --
> 2.14.3 (Apple Git-98)
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 4/4] BaseTools: resolve initialization order errors in VfrFormPkg.h

[Gao, Liming]

I think these patches resolves CLANG build issues in BaseTools. Do you verify 
them with GCC or VS tool chain?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Zenith432
> Sent: Saturday, December 9, 2017 10:45 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 4/4] BaseTools: resolve initialization order errors in 
> VfrFormPkg.h
> 
> clang generates many warnings
> warning: field 'XXX' is uninitialized when used here [-Wuninitialized]
> for VfrFormPkg.h.
> 
> VfrFormPkg.h defines many classes derived from CIfrObj (along with other base 
> classes.)
> Each of these derived classes defines a non-static member field that serves 
> as a duplicate pointer
> to an original pointer defined in the CIfrObj base class, but cast to a 
> different pointer type.
> The derived class constructor passes the duplicate pointer to base class 
> constructors:
> 1) Once passes the address of the duplicate pointer to the CIfrObj 
> constructor to have it initialized.
> 2) Then passes the duplicate pointer to one or more subsequent base class 
> constructors to be used.
> Both 1) and 2) constitute undefined behavior in C++.  C++ prescribes that 
> base classes are initialized
> before non-static members when initializing a derived class.  So when base 
> class constructors are executing,
> it is not permitted to assume any non-static members of the derived class 
> exist (even to the stage of having
> their storage allocated.)
> 
> clang does not issue warnings for 1), but issues warnings -Wuninitialized for 
> 2).
> 
> This coding methodology is resolved as follows:
> a) The CIfrObj object accessor method for retrieving the original pointer is 
> revised to a template member
> function that returns the original pointer cast to a desired target type.
> b) The call to CIfrObj constructor is no longer used to initialize the 
> duplicate pointer in the derived class.
> c) Any subsequent calls to a base class constructor that need to use the 
> pointer, retrieve it from the CIfrObj
> base class using the template accessor method.
> d) If the derived class makes no further use of the pointer, then the 
> duplicate pointer defined in it is eliminated.
> e) If the derived class needs the duplicate pointer for other use, the 
> duplicate pointer remains in
> the derived class and is initialized in proper order from the original 
> pointer in CIfrObj.
> f) Existing source code that previously used the CIfrObj pointer accessor 
> method is revised to use the template method.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 
> ---
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp |   4 +-
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.h   | 659 
> ++-
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g|   8 +-
>  3 files changed, 257 insertions(+), 414 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp 
> b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
> index b8350623..3985da22 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
> @@ -839,7 +839,7 @@ CFormPkg::DeclarePendingQuestion (
>// DisableIf
>CIfrDisableIf DIObj;
>DIObj.SetLineNo (LineNo);
> -  *InsertOpcodeAddr = DIObj.GetObjBinAddr ();
> +  *InsertOpcodeAddr = DIObj.GetObjBinAddr();
> 
>//TrueOpcode
>CIfrTrue TObj (LineNo);
> @@ -1920,7 +1920,7 @@ CIfrRecordInfoDB::IfrCreateDefaultForQuestion (
>Obj = new CIfrObj (pOpHead->OpCode, NULL, pSNode->mBinBufLen, 
> FALSE);
>assert (Obj != NULL);
>Obj->SetLineNo (pSNode->mLineNo);
> -  ObjBinBuf = Obj->GetObjBinAddr();
> +  ObjBinBuf = Obj->GetObjBinAddr();
>memcpy (ObjBinBuf, pSNode->mIfrBinBuf, (UINTN)pSNode->mBinBufLen);
>delete Obj;
>pSNode = pSNode->mNext;
> diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h 
> b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
> index 8a22cb24..822f8619 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
> +++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
> @@ -278,8 +278,9 @@ public:
>  mLineNo = LineNo;
>}
> 
> -  inline CHAR8 * GetObjBinAddr (VOID) {
> -return mObjBinBuf;
> +  template
> +  inline T * GetObjBinAddr (VOID) {
> +return reinterpret_cast(mObjBinBuf);
>}
> 
>inline UINT32 GetObjBinOffset (VOID) {
> @@ -665,8 +666,8 @@ private:
>EFI_GUID *mClassGuid;
> 
>  public:
> -  CIfrFormSet (UINT8 Size) : CIfrObj (EFI_IFR_FORM_SET_OP, (CHAR8 
> **), Size),
> -   CIfrOpHeader (EFI_IFR_FORM_SET_OP, >Header, 
> Size) {
> +  CIfrFormSet (UINT8 Size) : CIfrObj (EFI_IFR_FORM_SET_OP, (CHAR8 **)NULL, 
> Size),
> +   CIfrOpHeader (EFI_IFR_FORM_SET_OP, 
> &(GetObjBinAddr())->Header, Size),
> mFormSet(GetObjBinAddr()) {
>  mFormSet->Help = EFI_STRING_ID_INVALID;
>  

Re: [edk2] [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds

[Gao, Liming]

Include ArmPkg maintainers for review. 

> -Original Message-
> From: Pete Batard [mailto:p...@akeo.ie]
> Sent: Friday, December 8, 2017 10:06 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [PATCH v3 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable 
> VS2017/ARM builds
> 
> Introduce CRT assembly replacements for __rt_sdiv, __rt_udiv,
> __rt_udiv64, __rt_sdiv64, __rt_srsh, memcpy and memset.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Pete Batard 
> ---
>  ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm | 255 
> 
>  ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtsrsh.asm|  45 
>  ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf |  13 +-
>  ArmPkg/Library/CompilerIntrinsicsLib/memcpy_ms.c   |  34 +++
>  ArmPkg/Library/CompilerIntrinsicsLib/memset_ms.c   |  33 +++
>  5 files changed, 378 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm 
> b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm
> new file mode 100644
> index ..096dc6317318
> --- /dev/null
> +++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/rtdiv.asm
> @@ -0,0 +1,255 @@
> +///** @file
> +//
> +//  This code provides replacement for MSVC CRT division functions
> +//
> +//  Copyright (c) 2017, Pete Batard. All rights reserved.
> +//  Based on generated assembly of ReactOS' 
> sdk/lib/crt/math/arm/__rt_###div.c,
> +//  Copyright (c) Timo Kreuzer. 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.
> +//
> +//**/
> +
> +  EXPORT _fltused
> +  EXPORT __brkdiv0
> +
> +  EXPORT __rt_sdiv
> +  EXPORT __rt_udiv
> +  EXPORT __rt_udiv64
> +  EXPORT __rt_sdiv64
> +
> +  AREA  Math, CODE, READONLY
> +
> +_fltused
> +dcd 0x9875
> +
> +__brkdiv0
> +udf #249
> +
> +//
> +// uint64_t __rt_udiv(uint32_t divisor, uint32_t dividend)
> +//
> +
> +__rt_udiv
> +  cmp r0, #0
> +  beq __brkdiv0
> +  push{r3-r5,lr}
> +  mov r5,r0
> +  mov r4,r1
> +  cmp r5,r4
> +  it  hi
> +  movhi   r0,#0
> +  bhi __rt_udiv_label3
> +  clz r2,r5
> +  clz r3,r4
> +  subsr3,r2,r3
> +  movsr1,#1
> +  lsl r2,r5,r3
> +  lsl r3,r1,r3
> +  movsr0,#0
> +__rt_udiv_label1
> +  cmp r4,r2
> +  bcc __rt_udiv_label2
> +  orrsr0,r0,r3
> +  subsr4,r4,r2
> +__rt_udiv_label2
> +  lsrsr2,r2,#1
> +  lsrsr3,r3,#1
> +  bne __rt_udiv_label1
> +__rt_udiv_label3
> +  mov r1,r4
> +  pop {r3-r5,pc}
> +
> +//
> +// uint64_t __rt_sdiv(int32_t divisor, int32_t dividend)
> +//
> +
> +__rt_sdiv
> +  cmp r0, #0
> +  beq __brkdiv0
> +  push{r4-r6,lr}
> +  mov r4,r1
> +  andsr6,r0,#0x8000
> +  it  ne
> +  rsbne   r4,r4,#0
> +  mov r5,r0
> +  rsbsr5,r5,#0
> +  cmp r5,r4
> +  it  hi
> +  movhi   r0,#0
> +  bhi __rt_sdiv_label3
> +  clz r2,r5
> +  clz r3,r4
> +  subsr3,r2,r3
> +  movsr1,#1
> +  lsl r2,r5,r3
> +  lsl r3,r1,r3
> +  movsr0,#0
> +__rt_sdiv_label1
> +  cmp r4,r2
> +  bcc __rt_sdiv_label2
> +  orrsr0,r0,r3
> +  subsr4,r4,r2
> +__rt_sdiv_label2
> +  lsrsr2,r2,#1
> +  lsrsr3,r3,#1
> +  bne __rt_sdiv_label1
> +__rt_sdiv_label3
> +  cbz r6,__rt_sdiv_label4
> +  rsbsr4,r4,#0
> +__rt_sdiv_label4
> +  mov r1,r4
> +  pop {r4-r6,pc}
> +
> +//
> +// typedef struct {
> +//   uint64_t quotient;
> +//   uint64_t modulus;
> +// } udiv64_result_t;
> +//
> +// void __rt_udiv64_internal(udiv64_result_t *result, uint64_t divisor, 
> uint64_t dividend)
> +//
> +
> +__rt_udiv64_internal
> +  orrsr1,r2,r3
> +  beq __brkdiv0
> +  push{r4-r8,lr}
> +  mov r7,r3
> +  mov r6,r2
> +  mov r4,r0
> +  ldrdr0,r5,[sp,#0x18]
> +  cmp r7,r5
> +  bcc __rt_udiv64_internal_label2
> +  bhi __rt_udiv64_internal_label1
> +  cmp r6,r0
> +  bls __rt_udiv64_internal_label2
> +__rt_udiv64_internal_label1
> +  movsr3,#0
> +  strdr3,r3,[r4]
> +  b   __rt_udiv64_internal_label8
> +__rt_udiv64_internal_label2
> +  clz r2,r7
> +  cmp r2,#0x20
> +  bne __rt_udiv64_internal_label3
> +  clz r3,r6
> +  add 

Re: [edk2] [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add support for Watchdog driver

[Gao, Liming]

Leif:
  I have no strong opinion. PI spec doesn't require WdogRegisterHandler must be 
supported. So, this implementation doesn't break spec. For this platform, if 
there is no register handler or no critical register handler, this Watchdog 
driver can be used. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
> Lindholm
> Sent: Thursday, December 7, 2017 11:34 PM
> To: Gao, Liming 
> Cc: ard.biesheu...@linaro.org; edk2-devel@lists.01.org; Kinney, Michael D 
> 
> Subject: Re: [edk2] [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add 
> support for Watchdog driver
> 
> Liming,
> 
> https://www.mail-archive.com/edk2-devel@lists.01.org/msg32761.html
> Search for WdogRegisterHandler.
> 
> This topic is entirely unrelated to any _usage_ of watchdog timer
> protocol.
> 
> The topic is only whether it is reasonable to _implement_
> EFI_WATCHDOG_TIMER_ARCH_PROTOCOL for a hardware watchdog that *cannot*
> cause a callback to a handler function.
> Because when the hardware watchdog times out, it triggers a hard
> system reset, without any software interaction.
> 
> /
> Leif
> 
> On Thu, Dec 07, 2017 at 02:54:08PM +, Gao, Liming wrote:
> > Leif:
> >   I don't review the whole patch serial. Could you point your usage
> >   case on watch dog timer protocol?
> >
> > Thanks
> > Liming
> > > -Original Message-
> > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > > Sent: Thursday, December 7, 2017 7:04 PM
> > > To: Gao, Liming 
> > > Cc: Udit Kumar ; Kinney, Michael D 
> > > ; Meenakshi Aggarwal
> > > ; ard.biesheu...@linaro.org; 
> > > edk2-devel@lists.01.org; Varun Sethi 
> > > Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add 
> > > support for Watchdog driver
> > >
> > > Hi Liming,
> > >
> > > On Thu, Dec 07, 2017 at 07:11:38AM +, Gao, Liming wrote:
> > > > Leif:
> > > >   I don't see the core driver uses
> > > >   WatchdogTimer->RegisterHandler(). When it returns unsupported, it
> > > >   means the additional handler can't be registered. DxeCore uses
> > > >   WatchdogTimer->SetTimerPeriod(). This service is implemented in
> > > >   your driver.
> > > >
> > > >   Watchdog protocol is defined in PI spec. Spec describes that this
> > > >   protocol provides the services required to implement the Boot
> > > >   Service SetWatchdogTimer(). It provides a service to set the
> > > >   amount of time to wait before firing the watchdog timer, and it
> > > >   also provides a service to register a handler that is invoked when
> > > >   the watchdog timer fires. This protocol can implement the watchdog
> > > >   timer by using the event and timer Boot Services, or it can make
> > > >   use of custom hardware. If no handler has been registered, or the
> > > >   registered handler returns, then the system will be reset by
> > > >   calling the Runtime Service ResetSystem(). So, this protocol is
> > > >   required.
> > >
> > > I am not disputing that the protocol is not required. I am suggesting
> > > that this hardware watchdog _cannot_ be used to register a handler.
> > >
> > > If this hardware watchdog does not get updated in time, that causes an
> > > immediate hardware reset of the processor.
> > >
> > > Because of this, I believe EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is not the
> > > appropriate way to make use of it.
> > >
> > > Please let me know whether you agree.
> > >
> > > Regards,
> > >
> > > Leif
> > >
> > > > Thanks
> > > > Liming
> > > > >-Original Message-
> > > > >From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > > > >Sent: Tuesday, December 05, 2017 7:06 PM
> > > > >To: Udit Kumar 
> > > > >Cc: Gao, Liming ; Kinney, Michael D
> > > > >; Meenakshi Aggarwal
> > > > >; ard.biesheu...@linaro.org; edk2-
> > > > >de...@lists.01.org; Varun Sethi 
> > > > >Subject: Re: [PATCH edk2-platforms] [PATCH v3 2/9] Platform/NXP : Add
> > > > >support for Watchdog driver
> > > > >
> > > > >On Tue, Dec 05, 2017 at 05:07:00AM +, Udit Kumar wrote:
> > > > >> >   I suggest return EFI_UNSUPPORTED for this case. The protocol
> > > > >implementation
> > > > >> > could return its status besides spec defined status.
> > > > >>
> > > > >> Thanks to help me , how core will treat this error
> > > > >> 1/  Wdt not available
> > > > >> 2/ ignoring this error
> > > > >> 3/ core is not registering handler
> > > > >> I guess 3 is valid,
> > > > >
> > > > >Looking at Core/Dxe/Misc/SetWatchdogTimer.c:
> > > > >  //
> > > > >  // Attempt to set the timeout
> > > > >  //
> > > > >  Status = gWatchdogTimer->SetTimerPeriod (gWatchdogTimer,
> > > > >  MultU64x32 (Timeout, WATCHDOG_TIMER_CALIBRATE_PER_SECOND));
> > > > >
> > > > >  //
> 

Re: [edk2] Ubuntu Build EDK2 Issue

[Gao, Liming]

Build run command is for NT32 platform only. NT32 is Windows emulator. It only 
supports VS tool chain on Windows. 

So, on Linux, you don't need to specify Build run command. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Anonymous
> Sent: Friday, December 8, 2017 7:49 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Ubuntu Build EDK2 Issue
> 
> Hello,I had some problems when i compiled EDK2,It was compiled successfully 
> when I executed command "build",but I executed "build
> run",feedback the command SecMain not found,My compilation environment is 
> Ubuntu 16.04 LTS,and the built dependency package
> has been installed.
> ___
> 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] MdePkg: correct and clarify documentation of VA_LIST in Base.h

[Zenith432]

This is to resolve bug 457.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zenith432 
---
 MdePkg/Include/Base.h | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 02140a5a..19f36872 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -560,13 +560,14 @@ struct _LIST_ENTRY {
 //  VA_LIST  - typedef for argument list.
 //  VA_START (VA_LIST Marker, argument before the ...) - Init Marker for use.
 //  VA_END (VA_LIST Marker) - Clear Marker
-//  VA_ARG (VA_LIST Marker, var arg size) - Use Marker to get an argument from
-//the ... list. You must know the size and pass it in this macro.
+//  VA_ARG (VA_LIST Marker, var arg type) - Use Marker to get an argument from
+//the ... list. You must know the type and pass it in this macro.
 //  VA_COPY (VA_LIST Dest, VA_LIST Start) - Initialize Dest as a copy of Start.
 //
-//  example:
+//  Example:
 //
 //  UINTN
+//  EFIAPI
 //  ExampleVarArg (
 //IN UINTN  NumberOfArgs,
 //...
@@ -582,7 +583,7 @@ struct _LIST_ENTRY {
 //VA_START (Marker, NumberOfArgs);
 //for (Index = 0, Result = 0; Index < NumberOfArgs; Index++) {
 //  //
-//  // The ... list is a series of UINTN values, so average them up.
+//  // The ... list is a series of UINTN values, so sum them up.
 //  //
 //  Result += VA_ARG (Marker, UINTN);
 //}
@@ -591,6 +592,21 @@ struct _LIST_ENTRY {
 //return Result
 //  }
 //
+//  Notes:
+//
+//  This set of macros is intended to support variadic functions that
+//  use the EFIAPI calling convention.  Variadic functions that use a
+//  native calling convention should use stdarg.h.
+//  In particular:
+//  -- VA_START may only be used in a variadic EFIAPI function.
+//  -- va_start may only be used in a variadic native function.
+//  -- VA_START, VA_END, VA_ARG and VA_COPY may only be used on a VA_LIST.
+//  -- va_start, va_end, va_arg and va_copy may only be used on a va_list.
+//  -- Both VA_LIST or va_list may be passed as arguments to functions
+// of either EFIAPI or native calling conventions.
+//  -- VA_END, VA_ARG, VA_COPY, va_end, va_arg, and va_copy may be used
+// in functions of either calling conventions.
+//
 
 /**
   Return the size of argument that has been aligned to sizeof (UINTN).
-- 
2.14.3
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel