Re: [edk2] [PATCH v3 08/12] BaseTools/AARCH64: move to unified GCC linker script

2015-07-30 Thread Leif Lindholm
On Wed, Jul 29, 2015 at 05:11:58PM +0200, Ard Biesheuvel wrote:
 Drop the GCC AARCH64 specific linker script and use the new
 unified one instead.
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org

Reviewed-by: Leif Lindholm leif.lindh...@linaro.org

Also tested on ARM and AARCH64 hardware.

 ---
  BaseTools/Conf/tools_def.template   |  9 -
  BaseTools/Scripts/gcc-aarch64-ld-script | 39 
  2 files changed, 8 insertions(+), 40 deletions(-)
 
 diff --git a/BaseTools/Conf/tools_def.template 
 b/BaseTools/Conf/tools_def.template
 index 8e5750e2f300..d3dfdc41ada1 100644
 --- a/BaseTools/Conf/tools_def.template
 +++ b/BaseTools/Conf/tools_def.template
 @@ -3820,10 +3820,11 @@ DEFINE GCC_IPF_CC_FLAGS= 
 DEF(GCC_ALL_CC_FLAGS) -minline-int-divide-m
  DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
 -mword-relocations -mlittle-endian -mabi=aapcs -mapcs -fno-short-enums 
 -save-temps -fsigned-char -ffunction-sections -fdata-sections 
 -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
  DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mcmodel=large 
 -mlittle-endian -fno-short-enums -save-temps -fverbose-asm -fsigned-char  
 -ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin 
 -Wno-address -fno-asynchronous-unwind-tables
  DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 +DEFINE GCC_DLINK2_FLAGS_COMMON = 
 --script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
  DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) 
 --gc-sections
  DEFINE GCC_ARM_AARCH64_DLINK_COMMON= --emit-relocs -nostdlib --gc-sections 
 -u $(IMAGE_ENTRY_POINT) -e $(IMAGE_ENTRY_POINT) -Map 
 $(DEST_DIR_DEBUG)/$(BASE_NAME).map
  DEFINE GCC_ARM_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) 
 -Ttext=0x0
 -DEFINE GCC_AARCH64_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) 
 --script=$(EDK_TOOLS_PATH)/Scripts/gcc-aarch64-ld-script
 +DEFINE GCC_AARCH64_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) -z 
 common-page-size=0x20
  DEFINE GCC_IA32_X64_ASLDLINK_FLAGS = DEF(GCC_IA32_X64_DLINK_COMMON) --entry 
 _ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
  DEFINE GCC_ARM_ASLDLINK_FLAGS  = DEF(GCC_ARM_DLINK_FLAGS) --entry 
 ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
  DEFINE GCC_AARCH64_ASLDLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) --entry 
 ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
 @@ -3885,6 +3886,7 @@ DEFINE GCC47_ARM_CC_FLAGS= 
 DEF(GCC46_ARM_CC_FLAGS) -mno-unaligned-ac
  DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
 DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)
  DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
  DEFINE GCC47_AARCH64_DLINK_FLAGS = DEF(GCC_AARCH64_DLINK_FLAGS)
 +DEFINE GCC47_AARCH64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
 --defsym=PECOFF_HEADER_SIZE=0x228
  DEFINE GCC47_ARM_ASLDLINK_FLAGS  = DEF(GCC46_ARM_ASLDLINK_FLAGS)
  DEFINE GCC47_AARCH64_ASLDLINK_FLAGS  = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
  
 @@ -3901,6 +3903,7 @@ DEFINE GCC48_ARM_CC_FLAGS= 
 DEF(GCC47_ARM_CC_FLAGS)
  DEFINE GCC48_AARCH64_CC_FLAGS= DEF(GCC47_AARCH64_CC_FLAGS)
  DEFINE GCC48_ARM_DLINK_FLAGS = DEF(GCC47_ARM_DLINK_FLAGS)
  DEFINE GCC48_AARCH64_DLINK_FLAGS = DEF(GCC47_AARCH64_DLINK_FLAGS)
 +DEFINE GCC48_AARCH64_DLINK2_FLAGS= DEF(GCC47_AARCH64_DLINK2_FLAGS)
  DEFINE GCC48_ARM_ASLDLINK_FLAGS  = DEF(GCC47_ARM_ASLDLINK_FLAGS)
  DEFINE GCC48_AARCH64_ASLDLINK_FLAGS  = DEF(GCC47_AARCH64_ASLDLINK_FLAGS)
  
 @@ -3917,6 +3920,7 @@ DEFINE GCC49_ARM_CC_FLAGS= 
 DEF(GCC48_ARM_CC_FLAGS)
  DEFINE GCC49_AARCH64_CC_FLAGS= DEF(GCC48_AARCH64_CC_FLAGS)
  DEFINE GCC49_ARM_DLINK_FLAGS = DEF(GCC48_ARM_DLINK_FLAGS)
  DEFINE GCC49_AARCH64_DLINK_FLAGS = DEF(GCC48_AARCH64_DLINK_FLAGS)
 +DEFINE GCC49_AARCH64_DLINK2_FLAGS= DEF(GCC48_AARCH64_DLINK2_FLAGS)
  DEFINE GCC49_ARM_ASLDLINK_FLAGS  = DEF(GCC48_ARM_ASLDLINK_FLAGS)
  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)
  
 @@ -4357,6 +4361,7 @@ RELEASE_GCC47_ARM_CC_FLAGS   = 
 DEF(GCC47_ARM_CC_FLAGS) -Wno-unused-but-set-v
  *_GCC47_AARCH64_ASLDLINK_FLAGS   = DEF(GCC47_AARCH64_ASLDLINK_FLAGS)
  *_GCC47_AARCH64_ASM_FLAGS= DEF(GCC47_AARCH64_ASM_FLAGS)
  *_GCC47_AARCH64_DLINK_FLAGS  = DEF(GCC47_AARCH64_DLINK_FLAGS)
 +*_GCC47_AARCH64_DLINK2_FLAGS = DEF(GCC47_AARCH64_DLINK2_FLAGS)
  *_GCC47_AARCH64_PLATFORM_FLAGS   =
  *_GCC47_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
 DEF(GCC_PP_FLAGS)
  *_GCC47_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
 @@ -4480,6 +4485,7 @@ RELEASE_GCC48_ARM_CC_FLAGS   = 
 DEF(GCC48_ARM_CC_FLAGS) -Wno-unused-but-set-v
  *_GCC48_AARCH64_ASLDLINK_FLAGS   = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)
  *_GCC48_AARCH64_ASM_FLAGS= DEF(GCC48_AARCH64_ASM_FLAGS)
  *_GCC48_AARCH64_DLINK_FLAGS  = 

Re: [edk2] [PATCH v3 11/12] BaseTools/AARCH64: remove incremental linker script for 64K alignment

2015-07-30 Thread Leif Lindholm
On Wed, Jul 29, 2015 at 05:12:01PM +0200, Ard Biesheuvel wrote:
 Now that we moved all users to the unified GCC linker script, remove
 the old 64 KB incremental linker script for AARCH64 since it is now
 unused.
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org

Reviewed-by: Leif Lindholm leif.lindh...@linaro.org

Also tested on ARM and AARCH64 hardware.

Thanks for this cleanup.

 ---
  BaseTools/Scripts/gcc-aarch64-64K-align-ld-script | 4 
  1 file changed, 4 deletions(-)
 
 diff --git a/BaseTools/Scripts/gcc-aarch64-64K-align-ld-script 
 b/BaseTools/Scripts/gcc-aarch64-64K-align-ld-script
 deleted file mode 100644
 index 8aa4c5f08c0b..
 --- a/BaseTools/Scripts/gcc-aarch64-64K-align-ld-script
 +++ /dev/null
 @@ -1,4 +0,0 @@
 -SECTIONS {
 -  .text : ALIGN(0x1) { }
 -  .data : ALIGN(0x1) { }
 -}
 -- 
 1.9.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 v4 03/13] BaseTools IA32/X64: drop redundant alignment from linker script

2015-07-30 Thread Ard Biesheuvel
There is no need to pad out the end of a section of the start of
the following section is aligned to the same value. So drop the
redundant ALIGN() statements.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Tested-by: Liming Gao liming@intel.com
---
 BaseTools/Scripts/gcc-4K-align-ld-script | 3 ---
 BaseTools/Scripts/gcc4.4-ld-script   | 3 ---
 BaseTools/Scripts/gcc4.9-ld-script   | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/BaseTools/Scripts/gcc-4K-align-ld-script 
b/BaseTools/Scripts/gcc-4K-align-ld-script
index 1f0f1afb27f4..3ed1c12fb8aa 100644
--- a/BaseTools/Scripts/gcc-4K-align-ld-script
+++ b/BaseTools/Scripts/gcc-4K-align-ld-script
@@ -7,7 +7,6 @@ SECTIONS
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
 *(.rodata .rodata.* .gnu.linkonce.r.*)
-. = ALIGN(0x20);
   }
   .data : ALIGN(0x1000)
   {
@@ -16,7 +15,6 @@ SECTIONS
   .bss .bss.*
   *COM*
 )
-. = ALIGN(0x20);
   }
   .eh_frame : ALIGN(0x1000)
   {
@@ -25,7 +23,6 @@ SECTIONS
   .got : ALIGN(0x1000)
   {
 *(.got .got.*)
-. = ALIGN(0x20);
   }
   .rela : ALIGN(0x1000)
   {
diff --git a/BaseTools/Scripts/gcc4.4-ld-script 
b/BaseTools/Scripts/gcc4.4-ld-script
index 22b3220816c9..0d86908d0b30 100644
--- a/BaseTools/Scripts/gcc4.4-ld-script
+++ b/BaseTools/Scripts/gcc4.4-ld-script
@@ -7,7 +7,6 @@ SECTIONS
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
 *(.rodata .rodata.* .gnu.linkonce.r.*)
-. = ALIGN(0x20);
   }
   .data ALIGN(0x20) :
   {
@@ -16,7 +15,6 @@ SECTIONS
   .bss .bss.*
   *COM*
 )
-. = ALIGN(0x20);
   }
   .eh_frame ALIGN(0x20) :
   {
@@ -25,7 +23,6 @@ SECTIONS
   .got ALIGN(0x20) :
   {
 *(.got .got.*)
-. = ALIGN(0x20);
   }
   .rela ALIGN(0x20) :
   {
diff --git a/BaseTools/Scripts/gcc4.9-ld-script 
b/BaseTools/Scripts/gcc4.9-ld-script
index 2ac86e38fac7..207b9e1dc7f0 100644
--- a/BaseTools/Scripts/gcc4.9-ld-script
+++ b/BaseTools/Scripts/gcc4.9-ld-script
@@ -7,7 +7,6 @@ SECTIONS
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
 *(.rodata .rodata.* .gnu.linkonce.r.*)
-. = ALIGN(0x20);
   }
   .data ALIGN(0x40) :
   {
@@ -16,7 +15,6 @@ SECTIONS
   .bss .bss.*
   *COM*
 )
-. = ALIGN(0x20);
   }
   .eh_frame ALIGN(0x20) :
   {
@@ -25,7 +23,6 @@ SECTIONS
   .got ALIGN(0x20) :
   {
 *(.got .got.*)
-. = ALIGN(0x20);
   }
   .rela ALIGN(0x20) :
   {
-- 
1.9.1

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


[edk2] [PATCH v4 00/13] BaseTools: unify all GCC linker scripts

2015-07-30 Thread Ard Biesheuvel
Fourth attempt at unifying the various GCC linker scripts for different
architectures, GCC versions and minimum alignments.

Changes since v3:
- added patch #5 which updates the various IA32/X86 linker scripts to take their
  PE/COFF header size and section alignment from the command line, before
  switching to the unified version which does the same
- added Jordan's Reviewed-by, which he gave on the condition that patch #5
  be added
- added Liming's Tested-by to the patches that apply to IA32 and X86
- added Leif's Tested-by to the patches that apply to AARCH64 (except the
  ArmVirtPkg which he didn't test, but this was my testbed during development)
  
Changes since v2:
- for easier bisection, factor out the differences between the original
  and the unified linker scripts for X86 before making the switch
  (patches #1 - #4 and #6)
- add Intel copyright notice to unified version (patch #5)
- avoid defining *_*_*_DLINK2_FLAGS so that we don't pollute the variable
  definition space of non-GCC toolchains (patches #8 and #12)
- added Laszlo's ack to patch #10

 v2 blurb -
This time, I have added only a single unified GCC linker script that
can be parametrised by ld command line options:
- --defsym=PECOFF_HEADER_SIZE sets the size of the PE/COFF header
- -z common-page-size sets the minimum alignment

This use of common-page-size is entirely legal: it sets an internal LD
constant which can be referred to as CONSTANT(COMMONPAGESIZE) in linker
scripts, and is otherwise unused internally by the linker.

Tested with ArmVirtQemu/AARCH64 and Ovmf/X64

Branch is here
https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/unify-gcc-ld-scripts-v4
(now correctly based on the GitHub repo)

Ard Biesheuvel (13):
  BaseTools IA32/X64: remove NOP padding from X86/IA32 GCC linker
scripts
  BaseTools IA32/X64: move .rodata to PE/COFF .text section
  BaseTools IA32/X64: drop redundant alignment from linker script
  BaseTools IA32/X64: move .got contents to the PE/COFF .text section
  BaseTools IA32/X64: get header size and alignment from ld commandline
  BaseTools GCC: add unified GCC linker script for all archs and
versions
  BaseTools GCC: align start of .data to .text alignment
  BaseTools GCC: move AutoGen.obj contents to .text section
  BaseTools AARCH64: move to unified GCC linker script
  ArmPlatformPkg/ArmVExpressPkg: move to unified GCC linker script
  ArmVirtPkg: move to unified GCC linker script
  BaseTools AARCH64: remove incremental linker script for 64K alignment
  BaseTools IA32/X64: Use GccBase.lds instead of gcc*-ld-script

 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc |  2 +-
 ArmVirtPkg/ArmVirt.dsc.inc|  2 +-
 BaseTools/Conf/tools_def.template | 37 ++-
 BaseTools/Scripts/GccBase.lds | 70 
 BaseTools/Scripts/gcc-4K-align-ld-script  | 44 
 BaseTools/Scripts/gcc-aarch64-64K-align-ld-script |  4 --
 BaseTools/Scripts/gcc-aarch64-ld-script   | 39 ---
 BaseTools/Scripts/gcc4.4-ld-script| 44 
 BaseTools/Scripts/gcc4.9-ld-script| 44 
 9 files changed, 106 insertions(+), 180 deletions(-)
 create mode 100644 BaseTools/Scripts/GccBase.lds
 delete mode 100644 BaseTools/Scripts/gcc-4K-align-ld-script
 delete mode 100644 BaseTools/Scripts/gcc-aarch64-64K-align-ld-script
 delete mode 100644 BaseTools/Scripts/gcc-aarch64-ld-script
 delete mode 100644 BaseTools/Scripts/gcc4.4-ld-script
 delete mode 100644 BaseTools/Scripts/gcc4.9-ld-script

-- 
1.9.1

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


[edk2] [PATCH v4 07/13] BaseTools GCC: align start of .data to .text alignment

2015-07-30 Thread Ard Biesheuvel
Now that GenFw honors the ELF section alignment when placing the
PE/COFF sections in the output, the start of the PE/COFF version of
.data will be aligned to the alignment of .text if its alignment is
higher than the default. So duplicate this behavior in the ELF output,
this will make the memory layout of the PE/COFF binary match the
layout of the ELF version more closely.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Tested-by: Liming Gao liming@intel.com
Tested-by: Leif Lindholm leif.lindh...@linaro.org
---
 BaseTools/Scripts/GccBase.lds | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 0f0210e40703..3d99f01db21f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -37,7 +37,7 @@ SECTIONS {
* between these sections is the same in the ELF and the PE/COFF versions of
* this binary.
*/
-  .data : ALIGN(CONSTANT(COMMONPAGESIZE)) {
+  .data ALIGN(ALIGNOF(.text)) : ALIGN(CONSTANT(COMMONPAGESIZE)) {
 *(.data .data.* .gnu.linkonce.d.*)
 *(.bss .bss.* *COM*)
   }
-- 
1.9.1

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


[edk2] [PATCH v4 10/13] ArmPlatformPkg/ArmVExpressPkg: move to unified GCC linker script

2015-07-30 Thread Ard Biesheuvel
Move to the parametrised generic GCC linker script and set 64 KB
alignment, instead of using the AARCH64 specific incremental linker
script for 64 KB alignment which is about to be removed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Reviewed-by: Leif Lindholm leif.lindh...@linaro.org
Tested-by: Leif Lindholm leif.lindh...@linaro.org
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
index 7e0d8ff4b6e6..d2f8f5aa6d41 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
@@ -13,7 +13,7 @@
 #
 
 [BuildOptions.AARCH64.EDKII.DXE_RUNTIME_DRIVER]
-  GCC:*_*_AARCH64_DLINK_FLAGS = 
--script=$(EDK_TOOLS_PATH)/Scripts/gcc-aarch64-64K-align-ld-script
+  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1
 
 [LibraryClasses.common]
 !if $(TARGET) == RELEASE
-- 
1.9.1

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


Re: [edk2] [PATCH v3 12/12] BaseTools/X86|IA32: move to unified GCC linker script

2015-07-30 Thread Ard Biesheuvel
On 30 July 2015 at 02:59, Gao, Liming liming@intel.com wrote:
 Jordan:
   I have verified 4K aligned image build.

 Test-by: Liming Gao liming.gao@intel


Thanks Liming

Just to be clear, I assume you added something like this


diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a86a7f57143b..90dc29a5157d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -48,6 +48,9 @@ [BuildOptions]
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
 !endif

+[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
 

 #
 # SKU Identification section - list of all SKU IDs supported by this Platform.


when you did the test?

Thanks,
Ard.

 -Original Message-
 From: Justen, Jordan L
 Sent: Thursday, July 30, 2015 5:16 AM
 To: Ard Biesheuvel; edk2-devel@lists.01.org; Liu, Yingke D; Gao, Liming
 Cc: ler...@redhat.com; leif.lindh...@linaro.org; Ard Biesheuvel
 Subject: Re: [PATCH v3 12/12] BaseTools/X86|IA32: move to unified GCC linker 
 script

 Subject prefix: BaseTools/X86|IA32 = BaseTools IA32/X64

 What about 1 more step? :)

 This change starts to make use of the -z common-page-size and 
 --defsym=PECOFF_HEADER_SIZE params, but the description says 'move to unified 
 script'.

 So, how about first modifying the gcc*-ld-script files to use -z 
 common-page-size and --defsym=PECOFF_HEADER_SIZE and then the last patch is 
 trivial:

   BaseTools IA32/X64: Use GccBase.lds instead of gcc*-ld-script

   These scripts all now have the same contents, so we only need to use
   GccBase.lds. Therefore we can delete gcc-4K-align-ld-script,
   gcc4.4-ld-script and gcc4.9-ld-script.

 With that change, the series is

   Reviewed-by: Jordan Justen jordan.l.jus...@intel.com

 although, I would like someone to test the changes on a '4k' aligned image 
 build. Liming, do you know who might be able to do that?

 -Jordan

 On 2015-07-29 08:12:02, Ard Biesheuvel wrote:
 Drop the GCC 4.4/X86 and 4.9/X86 specific linker scripts and use the
 new unified one instead.

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  BaseTools/Conf/tools_def.template| 28 +--
  BaseTools/Scripts/gcc-4K-align-ld-script | 38 
  BaseTools/Scripts/gcc4.4-ld-script   | 38 
  BaseTools/Scripts/gcc4.9-ld-script   | 38 
  4 files changed, 26 insertions(+), 116 deletions(-)

 diff --git a/BaseTools/Conf/tools_def.template
 b/BaseTools/Conf/tools_def.template
 index d3dfdc41ada1..eeb488fb3597 100644
 --- a/BaseTools/Conf/tools_def.template
 +++ b/BaseTools/Conf/tools_def.template
 @@ -3847,10 +3847,12 @@ DEFINE GCC_AARCH64_RC_FLAGS= -I binary -O 
 elf64-littleaarch64 -B aarch64
  DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar 
 -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -ffunction-sections 
 -fdata-sections -c -include AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
  DEFINE GCC44_IA32_CC_FLAGS   = DEF(GCC44_ALL_CC_FLAGS) -m32 
 -malign-double -fno-stack-protector -D EFI32
  DEFINE GCC44_X64_CC_FLAGS= DEF(GCC44_ALL_CC_FLAGS) -m64 
 -fno-stack-protector -DEFIAPI=__attribute__((ms_abi)) 
 -DNO_BUILTIN_VA_FUNCS -mno-red-zone -Wno-address -mcmodel=large
 -DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections 
 --script=$(EDK_TOOLS_PATH)/Scripts/gcc4.4-ld-script
 +DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
 common-page-size=0x20
  DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) 
 --entry ReferenceAcpiTable -u ReferenceAcpiTable
  DEFINE GCC44_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_COMMON) 
 --entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map 
 $(DEST_DIR_DEBUG)/$(BASE_NAME).map
 +DEFINE GCC44_IA32_DLINK2_FLAGS   = DEF(GCC_DLINK2_FLAGS_COMMON) 
 --defsym=PECOFF_HEADER_SIZE=0x220
  DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)  
 -melf_x86_64 --oformat=elf64-x86-64
 +DEFINE GCC44_X64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
 --defsym=PECOFF_HEADER_SIZE=0x228
  DEFINE GCC44_ASM_FLAGS   = DEF(GCC_ASM_FLAGS)

  DEFINE GCC45_IA32_CC_FLAGS   = DEF(GCC44_IA32_CC_FLAGS)
 @@ -3858,7 +3860,9 @@ DEFINE GCC45_X64_CC_FLAGS= 
 DEF(GCC44_X64_CC_FLAGS)
  DEFINE GCC45_IA32_X64_DLINK_COMMON   = DEF(GCC44_IA32_X64_DLINK_COMMON)
  DEFINE GCC45_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_ASLDLINK_FLAGS)
  DEFINE GCC45_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_FLAGS)
 +DEFINE GCC45_IA32_DLINK2_FLAGS   = DEF(GCC44_IA32_DLINK2_FLAGS)
  DEFINE GCC45_X64_DLINK_FLAGS = DEF(GCC44_X64_DLINK_FLAGS)
 +DEFINE GCC45_X64_DLINK2_FLAGS= DEF(GCC44_X64_DLINK2_FLAGS)
  DEFINE GCC45_ASM_FLAGS   = DEF(GCC44_ASM_FLAGS)

  DEFINE 

Re: [edk2] [PATCH 2/2] CryptoPkg/OpensslLib: Undefine NO_BUILTIN_VA_FUNCS to fix varargs breakage

2015-07-30 Thread Laszlo Ersek
On 07/30/15 13:37, David Woodhouse wrote:
 On Tue, 2015-07-28 at 20:26 +0200, Laszlo Ersek wrote:

 series (up to and including 3/2)
 Tested-by: Laszlo Ersek ler...@redhat.com
 
 If you did this by pulling my tree, rather than manually applying
 patches — which I'm fairly sure you did — then you also tested the
 previous series of 6 API cleanups. I'll include your Tested-By on those
 too, unless you object.

I indeed pulled your tree, so please do add my T-b.

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


Re: [edk2] efi acpidump port

2015-07-30 Thread Laszlo Ersek
On 07/30/15 01:27, Andrew Fish wrote:
 
 On Jul 29, 2015, at 4:11 PM, Jordan Justen jordan.l.jus...@intel.com wrote:

 On 2015-07-29 13:43:43, Smith, Jonathan D wrote:
 Attaching a zip of the files

 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org 
 mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Smith, Jonathan D
 Sent: Wednesday, July 29, 2015 1:37 PM
 To: 'Andrew Fish'; 'edk2-devel@lists.01.org 
 mailto:edk2-devel@lists.01.org'
 Subject: Re: [edk2] efi acpidump port

 Hi all,

 Let me know if the attachments to this message do not show up.

 I’ve elaborated the build metadata, reduced the ACPICA changes
 (moving alterations into additional files as needed), and verified
 that the compiled AcpicaPkg provides a correct implementation of
 acpidump that works for Minnow.

 To set up the EDK2 AcpicaPkg from the ACPICA sources:


 1.) Go to https://acpica.org/downloads https://acpica.org/downloads and 
 download
 aspia-unix-20150619.tarhttps://acpica.org/downloads%20and%20download%20aspia-unix-20150619.tar
  https://acpica.org/downloads%20and%20download%20aspia-unix-20150619.tar

 2.) Extract the tar and put it in the root of edk2 as AcpicaPkg

 3.) Apply acenv.patch to
 edk2\AcpicaPkg\source\include\platform\acenv.h or just add the two
 lines manually.

 4.) Add the files AcpicaPkg.dec, AcpicaPkg.dsc, acpidump.inf, and
 acpi_main.c to the dir edk2\AcpicaPkg

 5.) Add the file acedk2efi.h to the dir
 edk2\AcpicaPkg\source\include\platform

 This seems a little silly.

 
 Like the Intel email server removing the INF from the zip file? 
 
 Seems silly to make a branch to add a single package. How do you add a  
 subproject? Like the FatPkg? 

Forking off a branch in git is the cheapest thing one can do, for
whatever development. The branch exists only in one's local git repo at
that point, of course. Then, for sharing it with others, the branch can
be pushed to another (public) repo, assuming the permissions are in
place, eg. to one owned by the developer on github. Then others can
fetch that branch if they'd like to play with it.

One way to think about a git repo is commit store. It just holds
commits, where each commit is cryptographically linked to (usually) one
parent commit (more parents if the commit is a merge). Some commits are
referenced by branch names. Pushing and pulling a branch is
(conceptually) just (a) tossing around the exact commit that the branch
points at, along with the branch name itself, and (b) making sure that
all ancestors of the commit, recursively, exist on the recipient side too.

Point (b) usually involves uploading a number of intermediary commits,
in case of push, and downloading the same, in case of fetch. Commits
that both sender and recipient repositories already share are not
transferred. This suffices for the commit that is identified by the
branch name being pushed or pulled to be *connected* in the recipient
repository as well.

Thanks
Laszlo

 Thanks,
 
 Andrew Fish
 
 Maybe you can just fork edk2, make the required changes and provide a
 link to your branch?

 https://github.com/tianocore/edk2 https://github.com/tianocore/edk2

 Also, what about 'Contributed-under'? (MdePkg/Contributions.txt)

 By the way, I think Prince (Cc'd) did a port of acpidump in the past,
 but never tried to release it externally.

 -Jordan
 
 ___
 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 v4 09/13] BaseTools AARCH64: move to unified GCC linker script

2015-07-30 Thread Ard Biesheuvel
Drop the GCC AARCH64 specific linker script and use the new
unified one instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Reviewed-by: Leif Lindholm leif.lindh...@linaro.org
Tested-by: Leif Lindholm leif.lindh...@linaro.org
---
 BaseTools/Conf/tools_def.template   |  9 -
 BaseTools/Scripts/gcc-aarch64-ld-script | 39 
 2 files changed, 8 insertions(+), 40 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 8f84b55236fd..f5e27cfc347f 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3820,10 +3820,11 @@ DEFINE GCC_IPF_CC_FLAGS= 
DEF(GCC_ALL_CC_FLAGS) -minline-int-divide-m
 DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mword-relocations 
-mlittle-endian -mabi=aapcs -mapcs -fno-short-enums -save-temps -fsigned-char 
-ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb 
-mfloat-abi=soft
 DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mcmodel=large 
-mlittle-endian -fno-short-enums -save-temps -fverbose-asm -fsigned-char  
-ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin 
-Wno-address -fno-asynchronous-unwind-tables
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
+DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
 DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) --gc-sections
 DEFINE GCC_ARM_AARCH64_DLINK_COMMON= --emit-relocs -nostdlib --gc-sections -u 
$(IMAGE_ENTRY_POINT) -e $(IMAGE_ENTRY_POINT) -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map
 DEFINE GCC_ARM_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) 
-Ttext=0x0
-DEFINE GCC_AARCH64_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) 
--script=$(EDK_TOOLS_PATH)/Scripts/gcc-aarch64-ld-script
+DEFINE GCC_AARCH64_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) -z 
common-page-size=0x20
 DEFINE GCC_IA32_X64_ASLDLINK_FLAGS = DEF(GCC_IA32_X64_DLINK_COMMON) --entry 
_ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
 DEFINE GCC_ARM_ASLDLINK_FLAGS  = DEF(GCC_ARM_DLINK_FLAGS) --entry 
ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
 DEFINE GCC_AARCH64_ASLDLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) --entry 
ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
@@ -3893,6 +3894,7 @@ DEFINE GCC47_ARM_CC_FLAGS= 
DEF(GCC46_ARM_CC_FLAGS) -mno-unaligned-ac
 DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)
 DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
 DEFINE GCC47_AARCH64_DLINK_FLAGS = DEF(GCC_AARCH64_DLINK_FLAGS)
+DEFINE GCC47_AARCH64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x228
 DEFINE GCC47_ARM_ASLDLINK_FLAGS  = DEF(GCC46_ARM_ASLDLINK_FLAGS)
 DEFINE GCC47_AARCH64_ASLDLINK_FLAGS  = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
 
@@ -3911,6 +3913,7 @@ DEFINE GCC48_ARM_CC_FLAGS= 
DEF(GCC47_ARM_CC_FLAGS)
 DEFINE GCC48_AARCH64_CC_FLAGS= DEF(GCC47_AARCH64_CC_FLAGS)
 DEFINE GCC48_ARM_DLINK_FLAGS = DEF(GCC47_ARM_DLINK_FLAGS)
 DEFINE GCC48_AARCH64_DLINK_FLAGS = DEF(GCC47_AARCH64_DLINK_FLAGS)
+DEFINE GCC48_AARCH64_DLINK2_FLAGS= DEF(GCC47_AARCH64_DLINK2_FLAGS)
 DEFINE GCC48_ARM_ASLDLINK_FLAGS  = DEF(GCC47_ARM_ASLDLINK_FLAGS)
 DEFINE GCC48_AARCH64_ASLDLINK_FLAGS  = DEF(GCC47_AARCH64_ASLDLINK_FLAGS)
 
@@ -3929,6 +3932,7 @@ DEFINE GCC49_ARM_CC_FLAGS= 
DEF(GCC48_ARM_CC_FLAGS)
 DEFINE GCC49_AARCH64_CC_FLAGS= DEF(GCC48_AARCH64_CC_FLAGS)
 DEFINE GCC49_ARM_DLINK_FLAGS = DEF(GCC48_ARM_DLINK_FLAGS)
 DEFINE GCC49_AARCH64_DLINK_FLAGS = DEF(GCC48_AARCH64_DLINK_FLAGS)
+DEFINE GCC49_AARCH64_DLINK2_FLAGS= DEF(GCC48_AARCH64_DLINK2_FLAGS)
 DEFINE GCC49_ARM_ASLDLINK_FLAGS  = DEF(GCC48_ARM_ASLDLINK_FLAGS)
 DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)
 
@@ -4377,6 +4381,7 @@ RELEASE_GCC47_ARM_CC_FLAGS   = 
DEF(GCC47_ARM_CC_FLAGS) -Wno-unused-but-set-v
 *_GCC47_AARCH64_ASLDLINK_FLAGS   = DEF(GCC47_AARCH64_ASLDLINK_FLAGS)
 *_GCC47_AARCH64_ASM_FLAGS= DEF(GCC47_AARCH64_ASM_FLAGS)
 *_GCC47_AARCH64_DLINK_FLAGS  = DEF(GCC47_AARCH64_DLINK_FLAGS)
+*_GCC47_AARCH64_DLINK2_FLAGS = DEF(GCC47_AARCH64_DLINK2_FLAGS)
 *_GCC47_AARCH64_PLATFORM_FLAGS   =
 *_GCC47_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_PP_FLAGS)
 *_GCC47_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
@@ -4502,6 +4507,7 @@ RELEASE_GCC48_ARM_CC_FLAGS   = 
DEF(GCC48_ARM_CC_FLAGS) -Wno-unused-but-set-v
 *_GCC48_AARCH64_ASLDLINK_FLAGS   = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)
 *_GCC48_AARCH64_ASM_FLAGS= DEF(GCC48_AARCH64_ASM_FLAGS)
 *_GCC48_AARCH64_DLINK_FLAGS  = DEF(GCC48_AARCH64_DLINK_FLAGS)
+*_GCC48_AARCH64_DLINK2_FLAGS = DEF(GCC48_AARCH64_DLINK2_FLAGS)
 

[edk2] [PATCH v4 08/13] BaseTools GCC: move AutoGen.obj contents to .text section

2015-07-30 Thread Ard Biesheuvel
All AutoGen.obj files consist of global GUID definitions, fixed
and patchable PCDs and other data that is essentially read-only at
runtime but has not been declared as such for various reasons.

By moving these contents to .text we achieve two things:
- global GUIDs and other data items which must be constant for correct
  program operation can no longer be modified, for instance, when
  running a DXE_RUNTIME_MODULE binary under the OS with the Properties
  Table feature for memory protection enabled;
- the .data section becomes smaller, and may be dropped completely for
  many XIP modules, which reduces wasted FV space if the PE/COFF section
  alignment is large.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Tested-by: Liming Gao liming@intel.com
Tested-by: Leif Lindholm leif.lindh...@linaro.org
---
 BaseTools/Scripts/GccBase.lds | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 3d99f01db21f..4ee6d998532c 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -29,6 +29,13 @@ SECTIONS {
 *(.text .text.* .stub .gnu.linkonce.t.*)
 *(.rodata .rodata.* .gnu.linkonce.r.*)
 *(.got .got.*)
+
+/*
+ * The contents of AutoGen.c files are constant from the POV of the 
program,
+ * but most of its contents end up in .data or .bss by default since few of
+ * the variable definitions that get emitted are declared as CONST.
+ */
+*:AutoGen.obj(.data .data.* .bss .bss.*)
   }
 
   /*
-- 
1.9.1

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


[edk2] [PATCH v4 12/13] BaseTools AARCH64: remove incremental linker script for 64K alignment

2015-07-30 Thread Ard Biesheuvel
Now that we moved all users to the unified GCC linker script, remove
the old 64 KB incremental linker script for AARCH64 since it is now
unused.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Reviewed-by: Leif Lindholm leif.lindh...@linaro.org
Tested-by: Leif Lindholm leif.lindh...@linaro.org
---
 BaseTools/Scripts/gcc-aarch64-64K-align-ld-script | 4 
 1 file changed, 4 deletions(-)

diff --git a/BaseTools/Scripts/gcc-aarch64-64K-align-ld-script 
b/BaseTools/Scripts/gcc-aarch64-64K-align-ld-script
deleted file mode 100644
index 8aa4c5f08c0b..
--- a/BaseTools/Scripts/gcc-aarch64-64K-align-ld-script
+++ /dev/null
@@ -1,4 +0,0 @@
-SECTIONS {
-  .text : ALIGN(0x1) { }
-  .data : ALIGN(0x1) { }
-}
-- 
1.9.1

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


[edk2] [PATCH v4 01/13] BaseTools IA32/X64: remove NOP padding from X86/IA32 GCC linker scripts

2015-07-30 Thread Ard Biesheuvel
The NOP padding in the GCC linker scripts ensures that all empty
regions in the ELF binary are filled with x86 NOP instructions.

There is no upside to doing this: if the CPU ends up executing these
instructions, we have little hope of resuming normal execution of the
program anyway. And having NOP slides in memory only makes it easier
for attackers to launch exploits. So remove them.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Tested-by: Liming Gao liming@intel.com
---
 BaseTools/Scripts/gcc4.4-ld-script | 2 +-
 BaseTools/Scripts/gcc4.9-ld-script | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Scripts/gcc4.4-ld-script 
b/BaseTools/Scripts/gcc4.4-ld-script
index 68b2767590ac..c0aa62180440 100644
--- a/BaseTools/Scripts/gcc4.4-ld-script
+++ b/BaseTools/Scripts/gcc4.4-ld-script
@@ -7,7 +7,7 @@ SECTIONS
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
 . = ALIGN(0x20);
-  } =0x90909090
+  }
   .data ALIGN(0x20) :
   {
 *(
diff --git a/BaseTools/Scripts/gcc4.9-ld-script 
b/BaseTools/Scripts/gcc4.9-ld-script
index b69232853617..37a93cd85e94 100644
--- a/BaseTools/Scripts/gcc4.9-ld-script
+++ b/BaseTools/Scripts/gcc4.9-ld-script
@@ -7,7 +7,7 @@ SECTIONS
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
 . = ALIGN(0x20);
-  } =0x90909090
+  }
   .data ALIGN(0x40) :
   {
 *(
-- 
1.9.1

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


[edk2] [PATCH v4 02/13] BaseTools IA32/X64: move .rodata to PE/COFF .text section

2015-07-30 Thread Ard Biesheuvel
The .rodata ELF section contains constant non-executable data that
should never be modified by the program itself. Since the risk of
inadvertent modification is typically higher than the risk of
inadvertent execution, it makes sense to put this data in the
R-X .text section rather than in the RW- .data section.
So move it there.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Tested-by: Liming Gao liming@intel.com
---
 BaseTools/Scripts/gcc-4K-align-ld-script | 2 +-
 BaseTools/Scripts/gcc4.4-ld-script   | 2 +-
 BaseTools/Scripts/gcc4.9-ld-script   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Scripts/gcc-4K-align-ld-script 
b/BaseTools/Scripts/gcc-4K-align-ld-script
index 16cf623a3362..1f0f1afb27f4 100644
--- a/BaseTools/Scripts/gcc-4K-align-ld-script
+++ b/BaseTools/Scripts/gcc-4K-align-ld-script
@@ -6,12 +6,12 @@ SECTIONS
   .text : ALIGN(0x1000)
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
+*(.rodata .rodata.* .gnu.linkonce.r.*)
 . = ALIGN(0x20);
   }
   .data : ALIGN(0x1000)
   {
 *(
-  .rodata .rodata.* .gnu.linkonce.r.*
   .data .data.* .gnu.linkonce.d.*
   .bss .bss.*
   *COM*
diff --git a/BaseTools/Scripts/gcc4.4-ld-script 
b/BaseTools/Scripts/gcc4.4-ld-script
index c0aa62180440..22b3220816c9 100644
--- a/BaseTools/Scripts/gcc4.4-ld-script
+++ b/BaseTools/Scripts/gcc4.4-ld-script
@@ -6,12 +6,12 @@ SECTIONS
   .text ALIGN(0x20) :
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
+*(.rodata .rodata.* .gnu.linkonce.r.*)
 . = ALIGN(0x20);
   }
   .data ALIGN(0x20) :
   {
 *(
-  .rodata .rodata.* .gnu.linkonce.r.*
   .data .data.* .gnu.linkonce.d.*
   .bss .bss.*
   *COM*
diff --git a/BaseTools/Scripts/gcc4.9-ld-script 
b/BaseTools/Scripts/gcc4.9-ld-script
index 37a93cd85e94..2ac86e38fac7 100644
--- a/BaseTools/Scripts/gcc4.9-ld-script
+++ b/BaseTools/Scripts/gcc4.9-ld-script
@@ -6,12 +6,12 @@ SECTIONS
   .text ALIGN(0x20) :
   {
 *(.text .stub .text.* .gnu.linkonce.t.*)
+*(.rodata .rodata.* .gnu.linkonce.r.*)
 . = ALIGN(0x20);
   }
   .data ALIGN(0x40) :
   {
 *(
-  .rodata .rodata.* .gnu.linkonce.r.*
   .data .data.* .gnu.linkonce.d.*
   .bss .bss.*
   *COM*
-- 
1.9.1

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


Re: [edk2] [PATCH v3 12/12] BaseTools/X86|IA32: move to unified GCC linker script

2015-07-30 Thread Gao, Liming
Yes. 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, July 30, 2015 10:26 PM
To: Gao, Liming
Cc: Justen, Jordan L; edk2-devel@lists.01.org; ler...@redhat.com; Liu, Yingke 
D; leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH v3 12/12] BaseTools/X86|IA32: move to unified GCC 
linker script

On 30 July 2015 at 02:59, Gao, Liming liming@intel.com wrote:
 Jordan:
   I have verified 4K aligned image build.

 Test-by: Liming Gao liming.gao@intel


Thanks Liming

Just to be clear, I assume you added something like this


diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 
a86a7f57143b..90dc29a5157d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -48,6 +48,9 @@ [BuildOptions]
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable  !endif

+[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
 

 #
 # SKU Identification section - list of all SKU IDs supported by this Platform.


when you did the test?

Thanks,
Ard.

 -Original Message-
 From: Justen, Jordan L
 Sent: Thursday, July 30, 2015 5:16 AM
 To: Ard Biesheuvel; edk2-devel@lists.01.org; Liu, Yingke D; Gao, 
 Liming
 Cc: ler...@redhat.com; leif.lindh...@linaro.org; Ard Biesheuvel
 Subject: Re: [PATCH v3 12/12] BaseTools/X86|IA32: move to unified GCC 
 linker script

 Subject prefix: BaseTools/X86|IA32 = BaseTools IA32/X64

 What about 1 more step? :)

 This change starts to make use of the -z common-page-size and 
 --defsym=PECOFF_HEADER_SIZE params, but the description says 'move to unified 
 script'.

 So, how about first modifying the gcc*-ld-script files to use -z 
 common-page-size and --defsym=PECOFF_HEADER_SIZE and then the last patch is 
 trivial:

   BaseTools IA32/X64: Use GccBase.lds instead of gcc*-ld-script

   These scripts all now have the same contents, so we only need to use
   GccBase.lds. Therefore we can delete gcc-4K-align-ld-script,
   gcc4.4-ld-script and gcc4.9-ld-script.

 With that change, the series is

   Reviewed-by: Jordan Justen jordan.l.jus...@intel.com

 although, I would like someone to test the changes on a '4k' aligned image 
 build. Liming, do you know who might be able to do that?

 -Jordan

 On 2015-07-29 08:12:02, Ard Biesheuvel wrote:
 Drop the GCC 4.4/X86 and 4.9/X86 specific linker scripts and use the 
 new unified one instead.

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  BaseTools/Conf/tools_def.template| 28 +--
  BaseTools/Scripts/gcc-4K-align-ld-script | 38 
  BaseTools/Scripts/gcc4.4-ld-script   | 38 
  BaseTools/Scripts/gcc4.9-ld-script   | 38 
  4 files changed, 26 insertions(+), 116 deletions(-)

 diff --git a/BaseTools/Conf/tools_def.template
 b/BaseTools/Conf/tools_def.template
 index d3dfdc41ada1..eeb488fb3597 100644
 --- a/BaseTools/Conf/tools_def.template
 +++ b/BaseTools/Conf/tools_def.template
 @@ -3847,10 +3847,12 @@ DEFINE GCC_AARCH64_RC_FLAGS= -I binary -O 
 elf64-littleaarch64 -B aarch64
  DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar 
 -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -ffunction-sections 
 -fdata-sections -c -include AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
  DEFINE GCC44_IA32_CC_FLAGS   = DEF(GCC44_ALL_CC_FLAGS) -m32 
 -malign-double -fno-stack-protector -D EFI32
  DEFINE GCC44_X64_CC_FLAGS= DEF(GCC44_ALL_CC_FLAGS) -m64 
 -fno-stack-protector -DEFIAPI=__attribute__((ms_abi)) 
 -DNO_BUILTIN_VA_FUNCS -mno-red-zone -Wno-address -mcmodel=large
 -DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections 
 --script=$(EDK_TOOLS_PATH)/Scripts/gcc4.4-ld-script
 +DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
 common-page-size=0x20
  DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) 
 --entry ReferenceAcpiTable -u ReferenceAcpiTable
  DEFINE GCC44_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_COMMON) 
 --entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map 
 $(DEST_DIR_DEBUG)/$(BASE_NAME).map
 +DEFINE GCC44_IA32_DLINK2_FLAGS   = DEF(GCC_DLINK2_FLAGS_COMMON) 
 --defsym=PECOFF_HEADER_SIZE=0x220
  DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)  
 -melf_x86_64 --oformat=elf64-x86-64
 +DEFINE GCC44_X64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
 --defsym=PECOFF_HEADER_SIZE=0x228
  DEFINE GCC44_ASM_FLAGS   = DEF(GCC_ASM_FLAGS)

  DEFINE GCC45_IA32_CC_FLAGS   = DEF(GCC44_IA32_CC_FLAGS)
 @@ -3858,7 +3860,9 @@ DEFINE GCC45_X64_CC_FLAGS= 
 DEF(GCC44_X64_CC_FLAGS)
  DEFINE GCC45_IA32_X64_DLINK_COMMON   = DEF(GCC44_IA32_X64_DLINK_COMMON)
  DEFINE GCC45_IA32_X64_ASLDLINK_FLAGS = 

Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support

2015-07-30 Thread Leif Lindholm
Hi Jordan,

On Wed, Jul 29, 2015 at 02:59:04PM -0700, Jordan Justen wrote:
   But, the name 'open platform' also sounds strange, assuming this a
   plain PCI bus driver. Couldn't it live in a 'pci drivers' package?
   
   Personally, I think we should rename OptionRomPkg to DriversPkg, or
   split it into UsbDriversPkg and PciDriversPkg.
 
 This seems like another thing we've been talking about for years. :)
 
 What do you think about adding:
 
 DriversPkg
 DriversPkg/Pci
 DriversPkg/Usb
 
  or
 
 PciDriversPkg
 UsbDriversPkg

bikeshed
We may well need support for drivers (read: I already have) that are
neither USB nor PCI. Should connecting interface still be the primary
filter?
/bikeshed

 One possible concern is who will own/maintain such a package. In this
 case, it might make sense to have a separate Maintainers.txt file
 under the package.
 
 Or, what I would suggest is that we just start out by saying that
 edk2-devel collectively owns it, and that any other package maintainer
 can commit contributions to the package.

I think that is a sensible thing to do until there is sufficient
amount of code in there to require a dedicated (set of) maintainer(s).

What about platform code?

/
Leif

  So, OpenPlatformPkg is my current way of dealing with the lack of a
  unified handling of platform/driver code in edk2. It is not something
  I mind giving up if this situation resolves itself another way. Or
  renaming to something more palatable :)
  
  I gave a presentation (more like lightning talk) at the spring
  plugfest in Seattle on this:
  http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Platforms_Tree_May_2015.pdf
  
  If we simply rename OptionRomPkg to DriversPkg (and restructure a bit
  in there), then that solves the drivers part of my problem. But I
  still need something for opensource platform support.
  
  /
  Leif
  ___
  edk2-devel mailing list
  edk2-devel@lists.01.org
  https://lists.01.org/mailman/listinfo/edk2-devel
 ___
 edk2-devel mailing list
 edk2-devel@lists.01.org
 https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v4 06/13] BaseTools GCC: add unified GCC linker script for all archs and versions

2015-07-30 Thread Ard Biesheuvel
This unifies all GCC linker scripts into a single parametrised GCC
linker script that can be used for all GCC versions and architectures.

The two parameters that can be set on the linker command line are:
- PECOFF_HEADER_SIZE, this is a build time property of GenFw, but
  its value is different between 32-bit and 64-bit;
- common-page-size, this can be set using -z on the ld command line,
  and controls the value of the COMMONPAGESIZE constant when used in
  a linker script. This value is used for the minimum section alignment.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
Tested-by: Liming Gao liming@intel.com
Tested-by: Leif Lindholm leif.lindh...@linaro.org
---
 BaseTools/Scripts/GccBase.lds | 63 
 1 file changed, 63 insertions(+)

diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
new file mode 100644
index ..0f0210e40703
--- /dev/null
+++ b/BaseTools/Scripts/GccBase.lds
@@ -0,0 +1,63 @@
+/** @file
+
+  Unified linker script for GCC based builds
+
+  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.BR
+  Copyright (c) 2015, Linaro Ltd. All rights reserved.BR
+
+  This program and the accompanying materials are licensed and made available 
under
+  the terms and conditions of the BSD License that accompanies this 
distribution.
+  The full text of the license may be found at
+  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.
+
+**/
+
+SECTIONS {
+
+  /*
+   * The PE/COFF binary consists of DOS and PE/COFF headers, and a sequence of
+   * section headers adding up to PECOFF_HEADER_SIZE bytes (which differs
+   * between 32-bit and 64-bit builds). The actual start of the .text section
+   * will be rounded up based on its actual alignment.
+   */
+  . = PECOFF_HEADER_SIZE;
+
+  .text : ALIGN(CONSTANT(COMMONPAGESIZE)) {
+*(.text .text.* .stub .gnu.linkonce.t.*)
+*(.rodata .rodata.* .gnu.linkonce.r.*)
+*(.got .got.*)
+  }
+
+  /*
+   * The alignment of the .data section should be less than or equal to the
+   * alignment of the .text section. This ensures that the relative offset
+   * between these sections is the same in the ELF and the PE/COFF versions of
+   * this binary.
+   */
+  .data : ALIGN(CONSTANT(COMMONPAGESIZE)) {
+*(.data .data.* .gnu.linkonce.d.*)
+*(.bss .bss.* *COM*)
+  }
+
+  .eh_frame ALIGN(CONSTANT(COMMONPAGESIZE)) : {
+KEEP (*(.eh_frame))
+  }
+
+  .rela ALIGN(CONSTANT(COMMONPAGESIZE)) : {
+*(.rela .rela.*)
+  }
+
+  /DISCARD/ : {
+*(.note.GNU-stack)
+*(.gnu_debuglink)
+*(.interp)
+*(.dynsym)
+*(.dynstr)
+*(.dynamic)
+*(.hash)
+*(.comment)
+  }
+}
-- 
1.9.1

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


[edk2] [PATCH v4 11/13] ArmVirtPkg: move to unified GCC linker script

2015-07-30 Thread Ard Biesheuvel
Move to the parametrised generic GCC linker script and set 64 KB
alignment, instead of using the AARCH64 specific incremental linker
script for 64 KB alignment which is about to be removed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
Acked-by: Laszlo Ersek ler...@redhat.com
Reviewed-by: Jordan Justen jordan.l.jus...@intel.com
---
 ArmVirtPkg/ArmVirt.dsc.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index b2003a58be04..8c54242b597b 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -18,7 +18,7 @@ [Defines]
   DEFINE TTY_TERMINAL= FALSE
 
 [BuildOptions.AARCH64.EDKII.DXE_RUNTIME_DRIVER]
-  GCC:*_*_AARCH64_DLINK_FLAGS = 
--script=$(EDK_TOOLS_PATH)/Scripts/gcc-aarch64-64K-align-ld-script
+  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1
 
 [LibraryClasses.common]
 !if $(TARGET) == RELEASE
-- 
1.9.1

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


Re: [edk2] today's US CERT UEFI advisory

2015-07-30 Thread Laszlo Ersek
On 07/30/15 21:02, Andrew Fish wrote:

 On Jul 30, 2015, at 11:43 AM, Laszlo Ersek ler...@redhat.com
 mailto:ler...@redhat.com wrote:

 On 07/30/15 20:09, Andrew Fish wrote:

 I’m not sure that the commit messages is the best way to communicate the
 info, and something could get fixed prior CVE going out.

 Therefore fixes for such issues *must not* be pushed to a public repo,
 nor publicly discussed, before the respective embargo is over. Once the
 embargo is over, the updates and the advisories are pushed / released as
 quickly as possible.

 Sorry I was thinking more about routine maintenance (like the string n
 cleanup recently), or refactoring of the code disabling the exploit
 mechanism with out knowledge that the exploit exists.  I guess an older
 branch can get patched, but the commit history in master is not going to
 record the CVE. 

Ah, right. That makes sense, absolutely. A fresh release may not contain
the bug any longer, as random luck, without any clues for the user.

 Regarding the commit messages: for many system administrators the only
 (or the safest) way to ensure that an update contains a fix for a CVE is
 to scan the changelog / commit log, and search for the CVE identifier.
 The black hats absolutely don't need this help to pinpoint holes from
 commit histories, but sysadmins (and developers outside the privileged,
 coordinated circle) certainly need the help for identifying fixes. In
 other words, obscurity only hurts the good guy.

 
 Which kind of points out it should be easy to get the info in the commit
 message. 

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


Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support

2015-07-30 Thread Jordan Justen
On 2015-07-30 11:51:56, Kinney, Michael D wrote:
 Jordan,
 
 I agree with the concept of having package(s) for device driver content.
 
 OptionRomPkg is really only intended to provide good examples for
 writing drivers, and we want to restrict what goes in there to a
 good example for each driver type.

Thus, if we have good example drivers in DriversPkg, then OptionRomPkg
is not needed? :)

 So I do not think OptionRomPkg is a good landing zone for this type
 of content.
 
 I think the hardest part is deciding how to organizer the driver
 content in package(s)/directories. Some device vendors may prefer to
 have vendor specific package/directory names to store families of
 drivers. Other vendors may be fine with putting their driver in a
 common package.
 
 The other part that may cause some confusion is that we already have
 drivers for some controllers in existing packages so finding the
 drivers already requires looking in multiple packages. Those drivers
 tend to be based on industry standard register sets (i.e. UHCI,
 EHCI, SATA, USB, etc).
 
 If we want to follow dir structure that matches what we have done in
 MdeModulePkg for industry standard host controllers, buses, and
 devices and support vendor specific areas, maybe we can do something
 like the following:
 
 DriversPkg
 DriversPkg/Vendor/VendorName 
 DriversPkg/Bus/BusType

Would we expect this too?

DriversPkg/Vendor/VendorName/Bus/BusType

I'm not sure how necessary the 'Vendor' directory level is. Seems fine
though.

-Jordan

 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
 Lindholm
 Sent: Thursday, July 30, 2015 8:20 AM
 To: Justen, Jordan L
 Cc: Kinney, Michael D; edk2-devel@lists.01.org
 Subject: Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support
 
 Hi Jordan,
 
 On Wed, Jul 29, 2015 at 02:59:04PM -0700, Jordan Justen wrote:
But, the name 'open platform' also sounds strange, assuming this a
plain PCI bus driver. Couldn't it live in a 'pci drivers' package?

Personally, I think we should rename OptionRomPkg to DriversPkg, or
split it into UsbDriversPkg and PciDriversPkg.
  
  This seems like another thing we've been talking about for years. :)
  
  What do you think about adding:
  
  DriversPkg
  DriversPkg/Pci
  DriversPkg/Usb
  
   or
  
  PciDriversPkg
  UsbDriversPkg
 
 bikeshed
 We may well need support for drivers (read: I already have) that are
 neither USB nor PCI. Should connecting interface still be the primary
 filter?
 /bikeshed
 
  One possible concern is who will own/maintain such a package. In this
  case, it might make sense to have a separate Maintainers.txt file
  under the package.
  
  Or, what I would suggest is that we just start out by saying that
  edk2-devel collectively owns it, and that any other package maintainer
  can commit contributions to the package.
 
 I think that is a sensible thing to do until there is sufficient
 amount of code in there to require a dedicated (set of) maintainer(s).
 
 What about platform code?
 
 /
 Leif
 
   So, OpenPlatformPkg is my current way of dealing with the lack of a
   unified handling of platform/driver code in edk2. It is not something
   I mind giving up if this situation resolves itself another way. Or
   renaming to something more palatable :)
   
   I gave a presentation (more like lightning talk) at the spring
   plugfest in Seattle on this:
   http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Platforms_Tree_May_2015.pdf
   
   If we simply rename OptionRomPkg to DriversPkg (and restructure a bit
   in there), then that solves the drivers part of my problem. But I
   still need something for opensource platform support.
   
   /
   Leif
   ___
   edk2-devel mailing list
   edk2-devel@lists.01.org
   https://lists.01.org/mailman/listinfo/edk2-devel
  ___
  edk2-devel mailing list
  edk2-devel@lists.01.org
  https://lists.01.org/mailman/listinfo/edk2-devel
 ___
 edk2-devel mailing list
 edk2-devel@lists.01.org
 https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] today's US CERT UEFI advisory

2015-07-30 Thread Blibbet
[...]
 Sorry I was thinking more about routine maintenance (like the string n
 cleanup recently), or refactoring of the code disabling the exploit
 mechanism with out knowledge that the exploit exists.  I guess an older
 branch can get patched, but the commit history in master is not going to
 record the CVE.

 Ah, right. That makes sense, absolutely. A fresh release may not contain
 the bug any longer, as random luck, without any clues for the user.
[...]

Sorry, I'm dim, this makes no sense to me, unless both of you forgot
smileys.

Why strip off useful metadata in the public release, if you have it? Why
not add useful metadata to the public release?

Or is it that the CVE data is only available internally to UEFI Forum
members?

UEFI Forum's antis numbers are already in the UEFI spec, in the revision
section. And a CVE isn't supposed to be a private/internal identifier,
it's supposed to be public, for people to search for, not something an
industry trade group should be obfuscating from the public.

Why  keep people in the dark about vulnerabilities in the public code? I
can see how some vendors want to keep their customers ignorant, but this
is code that's shared but multiple vendors, exploitable open source is
not useful to anyone.

Thanks for any clarification.

PS: CHIPSEC is working on a new security test for this. I hope QA teams
at all OEMS will start using it when it's out:
https://github.com/chipsec/chipsec

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


Re: [edk2] [PATCH v2 0/4] FFS/FV aligment optimization (was: [RFC] small C model and LLVM/clang support for AARCH64)

2015-07-30 Thread Andrew Fish

 On Jul 30, 2015, at 4:17 PM, Laszlo Ersek ler...@redhat.com wrote:
 
 On 07/27/15 15:52, Ard Biesheuvel wrote:
 On 27 July 2015 at 15:34, Liu, Yingke D yingke.d@intel.com wrote:
 Reviewed-by: Yingke Liu yingke.d@intel.com
 
 
 Thank you
 
 Committed as SVN r18077 ... r18080
 
 I do have another question related to the use of FIXED in a [Rule] section:
 since the increased alignment on AARCH64 will only be necessary for
 some toolchains, is it possible to have separate [Rule] sections?
 I did notice the 'DEBUG_MYTOOLS_IA32' in [Rule] section (for instance,
 EmbeddedPkg/EmbeddedPkg.fdf:102) but I don't quite understand how it
 works.
 
 Looking at the __GetFileOpts() method in
 BaseTools/Source/Python/GenFds/FdfParser.py, I think that pattern of
 tool chain keywords serves exactly the purpose you have in mind. I
 checked a couple occurrences of DEBUG_MYTOOLS_IA32 in various FDF files,
 and it always seems to restrict rules for the file type
 [Rule.Common.PEIM.TIANOCOMPRESSED]. So, in practice, it doesn matter:
 for ARM you usually (universally?) don't have LZMA compressed PEIMs,
 because PEIMs run from flash.

I don’t think PEIMs run from FLASH is a safe assumption to make. The PI 
architecture supports PEIMs that only run from memory, and thus compressed 
PEIMs. For modules that are specific to a boot mode, like recovery, it makes 
sense to compress them. 

Thanks,

Andrew Fish

 The DEBUG_MYTOOLS_IA32 restriction doesn't
 matter because you don't have that module type anyway.
 
 Anyway, I think if you are going to add (or try) this kind of
 restriction, then I should postpone reviewing
 
  [edk2] [PATCH 2/4] ArmVirtPkg: use 'auto' alignment and FIXED
 placement for XIP modules
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/547/focus=545
 
 because you might want to post a new version of it. Isn't that so?
 
 In any case, my most important question follows: the FDF spec says that
 FIXED means the file cannot be moved, it is not relocateable. But, how
 does that enable new optimizations in GenFfs, as you write in the
 linked patch? I found this in SVN r18079:
 
 + //
 + // Only add a special reducible padding section if
 + // - this FFS has the FFS_ATTRIB_FIXED attribute,
 + // - none of the preceding sections have alignment requirements,
 + // - the size of the padding is sufficient for the
 + // EFI_SECTION_FREEFORM_SUBTYPE_GUID header.
 + //
 + if ((FfsAttrib  FFS_ATTRIB_FIXED) != 0 
 + MaxEncounteredAlignment = 1 
 + Offset = sizeof (EFI_FREEFORM_SUBTYPE_GUID_SECTION)) {
 + SectHeader-CommonHeader.Type = EFI_SECTION_FREEFORM_SUBTYPE_GUID;
 + SectHeader-SubTypeGuid = mEfiFfsSectionAlignmentPaddingGuid;
 + } else {
 + SectHeader-CommonHeader.Type = EFI_SECTION_RAW;
 + }
 
 I'm quite sure you've explained why FIXED is important for this, but I
 can't recall the reason. Care to refresh my memory? Is it inherently
 related to mcmodel=small?
 
 Thanks
 Laszlo
 
 
 
 Thanks,
 Ard.
 
 
 -Original Message-
 From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
 Sent: Monday, July 27, 2015 8:32 PM
 To: Gao, Liming; Liu, Yingke D; edk2-devel@lists.01.org
 Cc: leif.lindh...@linaro.org; eugene.co...@hp.com; Ard Biesheuvel
 Subject: [PATCH v2 0/4] FFS/FV aligment optimization (was: [RFC] small C 
 model and LLVM/clang support for AARCH64)
 
 This series deals with the basetools optimizations to get rid of excessive 
 XIP alignment padding when using 2 KB or 4 KB alignment, which is common on 
 AARCH64, since the exception vector table requires 2 KB alignment, and the 
 small code model (which is the only one supported by the commercial LLVM 
 based compiler supplied by ARM) requires 4 KB alignment.
 
 
 
 v2 changelog
 - patches #1 and #2 are unchanged
 - patch #3 and #4 have been updated to only emit or adjust the special 
 padding
  section if the FDF [Rule] defines the FFS_ATTRIB_FIXED attributed for the 
 file
 - the adjustment logic in patch #4 has been reordered and the comments 
 updated
  to reflect more clearly that the misalignment and adjustment are not 
 specific
  to a single FFS section, but to the alignment of the FFS file as a whole
 - patch #4 now clears the alignment bits in the FFS header since they have 
 no
  meaning in FFS files that have been modified in place
 - replaced pointer arithmetic using VOID* pointers
 
 
 v1 changelog compared to 'RFC: small C model and LLVM/clang support for 
 AARCH64'
 
 Patches #1 and #2 (formerly #2 and #3) are unchanged.
 
 Patch #3 (formerly #4) has been updated to ensure that a special reducible 
 padding section is only emitted right before the first section in a FFS 
 that has alignment requirements. Reducing the size of such a section will 
 shift all subsequent sections into alignment, provided that the size of the 
 padding is sufficient. In some cases, 

Re: [edk2] [PATCH 2/4] ArmVirtPkg: use 'auto' alignment and FIXED placement for XIP modules

2015-07-30 Thread Laszlo Ersek
On 07/28/15 18:42, Ard Biesheuvel wrote:
 Now that GenFw correctly propagates the minimum alignment of the ELF
 input sections to the PE/COFF binary, we can simply select 'auto'
 alignment in the FDF Rule section instead of tweaking it by hand.
 
 Also add the FIXED FFS attribute to the module types that may execute
 in place. This enables a newly added optimization in GenFfs that strips
 redundant padding, preventing excessive waste of FV space if the section
 alignment is considerable (i.e., 2 KB or 4 KB)
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org
 ---
  ArmVirtPkg/ArmVirtQemu.fdf | 12 ++--
  ArmVirtPkg/ArmVirtXen.fdf  | 12 ++--
  2 files changed, 12 insertions(+), 12 deletions(-)
 
 diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
 index 4ef0f8bb6aa4..3c0487cd95b6 100644
 --- a/ArmVirtPkg/ArmVirtQemu.fdf
 +++ b/ArmVirtPkg/ArmVirtQemu.fdf
 @@ -309,20 +309,20 @@ [FV.FVMAIN_COMPACT]
  
  
  [Rule.Common.SEC]
 -  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
 -TE  TE Align = 128  $(INF_OUTPUT)/$(MODULE_NAME).efi
 +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
 +TE  TE Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi
}
  
  [Rule.Common.PEI_CORE]
 -  FILE PEI_CORE = $(NAMED_GUID) {
 -TE TE Align = 8 $(INF_OUTPUT)/$(MODULE_NAME).efi
 +  FILE PEI_CORE = $(NAMED_GUID) FIXED {
 +TE TE Align = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
  UI STRING =$(MODULE_NAME) Optional
}
  
  [Rule.Common.PEIM]
 -  FILE PEIM = $(NAMED_GUID) {
 +  FILE PEIM = $(NAMED_GUID) FIXED {
   PEI_DEPEX PEI_DEPEX Optional   $(INF_OUTPUT)/$(MODULE_NAME).depex
 - TE   TE Align = 8  $(INF_OUTPUT)/$(MODULE_NAME).efi
 + TE   TE Align = Auto   $(INF_OUTPUT)/$(MODULE_NAME).efi
   UI   STRING=$(MODULE_NAME) Optional
}
  
 diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
 index f98772b7191d..97cab4b058f2 100644
 --- a/ArmVirtPkg/ArmVirtXen.fdf
 +++ b/ArmVirtPkg/ArmVirtXen.fdf
 @@ -220,20 +220,20 @@ [FV.FVMAIN_COMPACT]
  
  
  [Rule.Common.SEC]
 -  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
 -TE  TE Align = 4K   $(INF_OUTPUT)/$(MODULE_NAME).efi
 +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
 +TE  TE Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi
}
  
  [Rule.Common.PEI_CORE]
 -  FILE PEI_CORE = $(NAMED_GUID) {
 -TE TE Align = 8 $(INF_OUTPUT)/$(MODULE_NAME).efi
 +  FILE PEI_CORE = $(NAMED_GUID) FIXED {
 +TE TE Align = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
  UI STRING =$(MODULE_NAME) Optional
}
  
  [Rule.Common.PEIM]
 -  FILE PEIM = $(NAMED_GUID) {
 +  FILE PEIM = $(NAMED_GUID) FIXED {
   PEI_DEPEX PEI_DEPEX Optional   $(INF_OUTPUT)/$(MODULE_NAME).depex
 - TE   TE Align = 8  $(INF_OUTPUT)/$(MODULE_NAME).efi
 + TE   TE Align = Auto   $(INF_OUTPUT)/$(MODULE_NAME).efi
   UI   STRING=$(MODULE_NAME) Optional
}
  
 

So, if I understand correctly, FIXED has no drawback even when building
with gcc, and when used in a clang build, it enables an optimization
that is *then* significant. And, because there is no penalty when using
gcc, it's simpler to add FIXED in a fixed way (pun intended) than to
create clang-specific rules (eg. by duplicating the current rules and
restricting them with *_XCODE5_* and *_GCC4x_* respectively).

... I hope the above mishmash can be called review. :)

Reviewed-by: Laszlo Ersek ler...@redhat.com
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support

2015-07-30 Thread Jordan Justen
On 2015-07-30 15:35:58, Yao, Jiewen wrote:
 Hello
 Do we consider below 2 options?
 -- DriversPkg/Vendor/VendorName/Bus/BusType ?
 or
 -- DriversPkg/Bus/BusType/Vendor/VendorName ?
 
 Another option is that we add DriverCategory, like
 GOP/UNDI/RAID/SIO. Then we can put UNDI driver together, no matter
 it is PCI based or USB based.

The Linux drivers directory seems to be organized like that:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers

drivers/net
drivers/gpu

...

-Jordan

 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jordan 
 Justen
 Sent: Friday, July 31, 2015 6:17 AM
 To: Kinney, Michael D; Leif Lindholm; Kinney, Michael D
 Cc: edk2-devel@lists.01.org
 Subject: Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support
 
 On 2015-07-30 11:51:56, Kinney, Michael D wrote:
  Jordan,
  
  I agree with the concept of having package(s) for device driver content.
  
  OptionRomPkg is really only intended to provide good examples for 
  writing drivers, and we want to restrict what goes in there to a good 
  example for each driver type.
 
 Thus, if we have good example drivers in DriversPkg, then OptionRomPkg is not 
 needed? :)
 
  So I do not think OptionRomPkg is a good landing zone for this type of 
  content.
  
  I think the hardest part is deciding how to organizer the driver 
  content in package(s)/directories. Some device vendors may prefer to 
  have vendor specific package/directory names to store families of 
  drivers. Other vendors may be fine with putting their driver in a 
  common package.
  
  The other part that may cause some confusion is that we already have 
  drivers for some controllers in existing packages so finding the 
  drivers already requires looking in multiple packages. Those drivers 
  tend to be based on industry standard register sets (i.e. UHCI, EHCI, 
  SATA, USB, etc).
  
  If we want to follow dir structure that matches what we have done in 
  MdeModulePkg for industry standard host controllers, buses, and 
  devices and support vendor specific areas, maybe we can do something 
  like the following:
  
  DriversPkg
  DriversPkg/Vendor/VendorName
  DriversPkg/Bus/BusType
 
 Would we expect this too?
 
 DriversPkg/Vendor/VendorName/Bus/BusType
 
 I'm not sure how necessary the 'Vendor' directory level is. Seems fine though.
 
 -Jordan
 
  -Original Message-
  From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
  Leif Lindholm
  Sent: Thursday, July 30, 2015 8:20 AM
  To: Justen, Jordan L
  Cc: Kinney, Michael D; edk2-devel@lists.01.org
  Subject: Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet 
  support
  
  Hi Jordan,
  
  On Wed, Jul 29, 2015 at 02:59:04PM -0700, Jordan Justen wrote:
 But, the name 'open platform' also sounds strange, assuming this 
 a plain PCI bus driver. Couldn't it live in a 'pci drivers' package?
 
 Personally, I think we should rename OptionRomPkg to DriversPkg, 
 or split it into UsbDriversPkg and PciDriversPkg.
   
   This seems like another thing we've been talking about for years. :)
   
   What do you think about adding:
   
   DriversPkg
   DriversPkg/Pci
   DriversPkg/Usb
   
or
   
   PciDriversPkg
   UsbDriversPkg
  
  bikeshed
  We may well need support for drivers (read: I already have) that are 
  neither USB nor PCI. Should connecting interface still be the primary 
  filter?
  /bikeshed
  
   One possible concern is who will own/maintain such a package. In 
   this case, it might make sense to have a separate Maintainers.txt 
   file under the package.
   
   Or, what I would suggest is that we just start out by saying that 
   edk2-devel collectively owns it, and that any other package 
   maintainer can commit contributions to the package.
  
  I think that is a sensible thing to do until there is sufficient 
  amount of code in there to require a dedicated (set of) maintainer(s).
  
  What about platform code?
  
  /
  Leif
  
So, OpenPlatformPkg is my current way of dealing with the lack of 
a unified handling of platform/driver code in edk2. It is not 
something I mind giving up if this situation resolves itself 
another way. Or renaming to something more palatable :)

I gave a presentation (more like lightning talk) at the spring 
plugfest in Seattle on this:
http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Pl
atforms_Tree_May_2015.pdf

If we simply rename OptionRomPkg to DriversPkg (and restructure a 
bit in there), then that solves the drivers part of my problem. 
But I still need something for opensource platform support.

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

Re: [edk2] [PATCH 38/58] UefiCpuPkg: CpuDxe: optionally save MTRR settings to AcpiNVS memory block

2015-07-30 Thread Yao, Jiewen
Thanks for the info. That makes sense.

It is pity that there is no X64 version and SMP not work. We can make 
improvement step by step.

Thank you
Yao Jiewen

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Wednesday, July 29, 2015 9:25 PM
To: Yao, Jiewen; Paolo Bonzini; Fan, Jeff; edk2-de...@ml01.01.org
Cc: Chen Fan; Justen, Jordan L
Subject: Re: [edk2] [PATCH 38/58] UefiCpuPkg: CpuDxe: optionally save MTRR 
settings to AcpiNVS memory block

On 07/28/15 23:41, Yao, Jiewen wrote:
 HI Laszlo
 I like the diagram in GMANE. Good work!

Thanks!

 Will you consider the option to merge CpuS3DataDxe into CpuMpDxe?
 Then we can reduce the driver number.

Well, that's a hard question. The code that I'm gradually creating / porting 
under

  OvmfPkg/QuarkPort/CpuS3DataDxe/

*originates* from the

  Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuMpDxe

driver in the Quark distribution. This is documented in the commit message of:

  [edk2] [PATCH 33/58] OvmfPkg: add skeleton QuarkPort/CpuS3DataDxe
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/329/focus=357

It says,

 The PiSmmCpuDxeSmm driver depends on an ACPI_CPU_DATA structure from 
 the Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuMpDxe/ driver. [...] 
 [...] Since CpuMpDxe is very complex and covers much more 
 functionality, we're going to gradually import only the above feature. 
 This patch pulls in the basic skeleton of the driver.

I justified this (ie. the creation of a separate, slimmed-down driver) with the 
following facts:
- edk2 does not have a separate CpuMpDxe driver at all,
- between the functionalities of Quark's CpuMpDxe, and edk2's
  UefiCpuPkg/CpuDxe, there's a partial overlap.

So importing CpuMpDxe whole-sale from Quark would have been overkill (it would 
have duplicated some functionality already present in UefiCpuPkg, and also 
included superfluous, complex stuff I didn't need at all). I thought that 
gathering / saving the ACPI_CPU_DATA structure was a task that was big enough 
and well defined enough to warrant a separate driver. Also, I wanted to 
minimize the impact on UefiCpuPkg.

 Or can we put CpuS3DataDxe to UefiCpuPkg?

Perhaps. In order to simplify things as much as possible, I analyzed the 
ACPI_CPU_DATA structure itself, and cut away some fields that might have made 
sense for a physical platform, but were not useful on QEMU. Please refer to the 
following patches:

  [edk2] [PATCH 32/58] OvmfPkg: QuarkPort: drop ACPI_CPU_DATA.APState
  [edk2] [PATCH 43/58] OvmfPkg: QuarkPort: drop
   ACPI_CPU_DATA.PreSmmInitRegisterTable
  [edk2] [PATCH 44/58] OvmfPkg: QuarkPort: drop
   ACPI_CPU_DATA.RegisterTable

Also, there were fields that I preserved, but filled in differently.
Please see for example:

  [edk2] [PATCH 34/58] OvmfPkg: QuarkPort/CpuS3DataDxe: fill in
   ACPI_CPU_DATA.StartupVector

So, if what I have posted as OvmfPkg/QuarkPort/CpuS3DataDxe is acceptable for 
UefiCpuPkg/CpuS3DataDxe, according to reviewers, then I don't have anything 
against it. (The relevant patches can be easily
identified: their subjects all start with OvmfPkg:
QuarkPort/CpuS3DataDxe.)

 Same question for PiSmmCpuDxeSmm. Can we put it to UefiCpuPkg, if it is 
 generic enough?

Same answer :) I think PiSmmCpuDxeSmm was already somewhat specific to Quark, 
to begin with, and then I modified it heavily. See the patches:

- OvmfPkg: PiSmmCpuDxeSmm: eliminate SmmLib dependency
- OvmfPkg: PiSmmCpuDxeSmm: eliminate CpuConfigLib linkage dependency
- OvmfPkg: QuarkPort/CpuS3DataDxe: fill in ACPI_CPU_DATA.StartupVector
  (this one affects both PiSmmCpuDxeSmm and CpuS3DataDxe)
- OvmfPkg: QuarkPort: drop ACPI_CPU_DATA.PreSmmInitRegisterTable
  (ditto)
- OvmfPkg: QuarkPort: drop ACPI_CPU_DATA.RegisterTable
  (ditto)
- OvmfPkg: QuarkPort/PiSmmCpuDxeSmm: hard-code CPU class identification
  (this one is definitely incorrect for physical hardware)

I trimmed stuff as aggressively as possible for two reasons:
- Analyzing and porting each single field in ACPI_CPU_DATA was a big
  effort. A single field (or a small group of fields belonging
  together) could easily take a day or more.
- Testability. I can test on QEMU (TCG and KVM), but not on real
  hardware.

So, *if* there had been a sufficiently generic driver in edk2 that had worked 
on physical platforms already, I might have included just that (and then some 
extras that are not important for, or not emulated by, a virtual machine, 
would have just decayed to no-ops at runtime.) In the other direction, it 
doesn't work; originally VM-targeted code cannot be expected to run on a 
physical platform. For that, the original driver in the Quark distribution 
should be generalized (to other processor types), and then imported to 
UefiCpuPkg.

Other limitations of this PiSmmCpuDxeSmm port:

- No support for an X64 SMM entry vector. (Intel could help a lot with
  this, by open sourcing a 

Re: [edk2] more code sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS

2015-07-30 Thread Laszlo Ersek
On 07/30/15 20:54, Jordan Justen wrote:
 On 2015-07-30 10:09:34, Laszlo Ersek wrote:
 (Sigh, I left off the list address. This should be discussed publicly.
 Resending.)

 Clearly, the SMBIOS patches I posted and got committed last time are not
 good enough. That's because the SMBIOS 3.0 entry point is structurally
 different from the prior versions (because why not). Therefore, now that
 Wei has QEMU generate SMBIOS 3.0 payload for the firmware, the checks we
 currently have in place, fail.

 (First and foremost, the *size* of the 3.0 entry point is different from
 that of the 2.8 one. We correctly catch this, but we incorrectly don't
 recognize 3.0.)

 I can write the code for telling apart 2.8 from 3.0; that's not the
 problem. The problem is that this code would have to be triplicated, as
 things currently stand:

 (1) ArmVirtPkg/QemuFwCfgToPcdDxe/QemuFwCfgToPcd.c sets
 PcdSmbiosVersion for MdeModulePkg/Universal/SmbiosDxe in the
 ArmVirtPkg build, early in the DXE phase,

 (2) OvmfPkg/PlatformPei/Platform.c sets the same PCD for the same
 generic driver in the OvmfPkg build, in the PEI phase,

 (3) OvmfPkg/SmbiosPlatformDxe/Qemu.c verifies the entry point again,
 and fetches the actual tables. (Note that
 (TablesSize != QemuAnchor.TableLength) is checked too, so there is a
 cross-dependency between the two blobs, the entry point and the
 actual tables.)

 Now, the current triplication we have is simplistic, so it shouldn't be
 a large problem. But, if I need to add smarts for detecting v2 versus
 v3, I would hate to triplicate that logic too. This is what code sites
 (1) and (2) both should do, for determining PcdSmbiosVersion:

 - Find etc/smbios/smbios-anchor. If it is missing, bail.
 - Compare the size of that fw-cfg blob against SMBIOS_TABLE_ENTRY_POINT.
 - If it is a match, perform the current _SM_ and _DMI_ checks.
 - If they fail, bail.
 - If they match, check that QemuAnchor.MajorVersion is actually 2.
 - If that fails, bail.
 - If it matches, set PcdSmbiosVersion to 0x02__.
 - Otherwise, compare the size of the anchor blob against
   SMBIOS_TABLE_3_0_ENTRY_POINT.
 - If it is a match, perform the (new) _SM3_ check.
 - If it fails, bail.
 - Check that MajorVersion (which now lives at a different offset) is
   3.
 - If that fails, bail.
 - If it matches, set PcdSmbiosVersion to 0x03__.
 - Otherwise, bail.

 In (3):
 - repeat all of the above, except do not *set* the PCD -- enforce it
   instead. ASSERT() that the PCD already has the right value (because
   at this point MdeModulePkg/Universal/SmbiosDxe is expected to have
   been dispatched, and consumed the PCD).
 - If the version is 2, then compare TableLength == tables blob size.
   If it fails, return NULL; otherwise return the tables.
 - If the version is 3, then compare TableMaximumSize = tables blob
   size. If it fails, return NULL; otherwise return the tables.
 - If the version is something different, return NULL.

 As I said, this can be coded as-written, but I strongly dislike the code
 triplication, and I expect you guys will too. So what do you propose? I
 could extract a tiny BASE library (a single function) that:
 - depends on QemuFwCfgLib
 - takes a BOOLEAN parameter that tells it whether to set the PCD, or
   check and enforce the PCD
 - return the detected SMBIOS version (2, 3, or missing/unknown)

 Then I could use this function in sites (1) and (2) for setting the PCD,
 and then use it in (3) for enforcing the PCD, and based on the returned
 SMBIOS version, perform the version-specific tables blob size check too.
 
 Would it help if you used a NULL library with a constructor to set
 gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion?
 
   MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
 LibraryClasses
   NULL|OvmfPkg/Library/SmbiosVersionLib/DetectSmbiosVersionLib.inf
   }

Wow, that's a fantastic idea. I had used LD_PRELOAD / RTLD_NEXT tricks
a few years back, with __attribute__ ((init)) functions even, in order
to hook into binaries, but honestly, I never thought of the above.

It's awesome:
- for ArmVirtPkg, it will make the DXE ordering trickery go away. I can
  drop the driver I introduced just for that, and I can remove it from
  the APRIORI DXE file too.
- For OvmfPkg, I can remove the PCD setting from PlatformPei, which
  never really felt right anyway.
- I can hook the PCD initialization *exactly* to the point where it
  matters. SmbiosDxe is what consumes the PCD, so there's no need to
  set it earlier. And, since it is a DXE driverRe: [edk2] more code
sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS, fw-cfg will be
  available even on ARM by then.
- I can use a library instance without giving it a class (header file,
  dec file hunk, etc) first.

My only worry is some kind of unforeseen constructor cycle in SmbiosDxe
(or silent breakage if the constructors are not spelled out correctly),
but I'll just have to test that.

 I'm not 

Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support

2015-07-30 Thread Yao, Jiewen
Hello
Do we consider below 2 options?
-- DriversPkg/Vendor/VendorName/Bus/BusType ?
or
-- DriversPkg/Bus/BusType/Vendor/VendorName ?

Another option is that we add DriverCategory, like GOP/UNDI/RAID/SIO. Then we 
can put UNDI driver together, no matter it is PCI based or USB based.

Thank you
Yao Jiewen

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jordan 
Justen
Sent: Friday, July 31, 2015 6:17 AM
To: Kinney, Michael D; Leif Lindholm; Kinney, Michael D
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet support

On 2015-07-30 11:51:56, Kinney, Michael D wrote:
 Jordan,
 
 I agree with the concept of having package(s) for device driver content.
 
 OptionRomPkg is really only intended to provide good examples for 
 writing drivers, and we want to restrict what goes in there to a good 
 example for each driver type.

Thus, if we have good example drivers in DriversPkg, then OptionRomPkg is not 
needed? :)

 So I do not think OptionRomPkg is a good landing zone for this type of 
 content.
 
 I think the hardest part is deciding how to organizer the driver 
 content in package(s)/directories. Some device vendors may prefer to 
 have vendor specific package/directory names to store families of 
 drivers. Other vendors may be fine with putting their driver in a 
 common package.
 
 The other part that may cause some confusion is that we already have 
 drivers for some controllers in existing packages so finding the 
 drivers already requires looking in multiple packages. Those drivers 
 tend to be based on industry standard register sets (i.e. UHCI, EHCI, 
 SATA, USB, etc).
 
 If we want to follow dir structure that matches what we have done in 
 MdeModulePkg for industry standard host controllers, buses, and 
 devices and support vendor specific areas, maybe we can do something 
 like the following:
 
 DriversPkg
 DriversPkg/Vendor/VendorName
 DriversPkg/Bus/BusType

Would we expect this too?

DriversPkg/Vendor/VendorName/Bus/BusType

I'm not sure how necessary the 'Vendor' directory level is. Seems fine though.

-Jordan

 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
 Leif Lindholm
 Sent: Thursday, July 30, 2015 8:20 AM
 To: Justen, Jordan L
 Cc: Kinney, Michael D; edk2-devel@lists.01.org
 Subject: Re: [edk2] [PATCH] EmbeddedPkg: Added Marvell Yukon Ethernet 
 support
 
 Hi Jordan,
 
 On Wed, Jul 29, 2015 at 02:59:04PM -0700, Jordan Justen wrote:
But, the name 'open platform' also sounds strange, assuming this 
a plain PCI bus driver. Couldn't it live in a 'pci drivers' package?

Personally, I think we should rename OptionRomPkg to DriversPkg, 
or split it into UsbDriversPkg and PciDriversPkg.
  
  This seems like another thing we've been talking about for years. :)
  
  What do you think about adding:
  
  DriversPkg
  DriversPkg/Pci
  DriversPkg/Usb
  
   or
  
  PciDriversPkg
  UsbDriversPkg
 
 bikeshed
 We may well need support for drivers (read: I already have) that are 
 neither USB nor PCI. Should connecting interface still be the primary 
 filter?
 /bikeshed
 
  One possible concern is who will own/maintain such a package. In 
  this case, it might make sense to have a separate Maintainers.txt 
  file under the package.
  
  Or, what I would suggest is that we just start out by saying that 
  edk2-devel collectively owns it, and that any other package 
  maintainer can commit contributions to the package.
 
 I think that is a sensible thing to do until there is sufficient 
 amount of code in there to require a dedicated (set of) maintainer(s).
 
 What about platform code?
 
 /
 Leif
 
   So, OpenPlatformPkg is my current way of dealing with the lack of 
   a unified handling of platform/driver code in edk2. It is not 
   something I mind giving up if this situation resolves itself 
   another way. Or renaming to something more palatable :)
   
   I gave a presentation (more like lightning talk) at the spring 
   plugfest in Seattle on this:
   http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Pl
   atforms_Tree_May_2015.pdf
   
   If we simply rename OptionRomPkg to DriversPkg (and restructure a 
   bit in there), then that solves the drivers part of my problem. 
   But I still need something for opensource platform support.
   
   /
   Leif
   ___
   edk2-devel mailing list
   edk2-devel@lists.01.org
   https://lists.01.org/mailman/listinfo/edk2-devel
  ___
  edk2-devel mailing list
  edk2-devel@lists.01.org
  https://lists.01.org/mailman/listinfo/edk2-devel
 ___
 edk2-devel mailing list
 edk2-devel@lists.01.org
 https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org

Re: [edk2] today's US CERT UEFI advisory

2015-07-30 Thread Andrew Fish

 On Jul 30, 2015, at 3:42 PM, Blibbet blib...@gmail.com wrote:
 
 [...]
 Sorry I was thinking more about routine maintenance (like the string n
 cleanup recently), or refactoring of the code disabling the exploit
 mechanism with out knowledge that the exploit exists.  I guess an older
 branch can get patched, but the commit history in master is not going to
 record the CVE.
 
 Ah, right. That makes sense, absolutely. A fresh release may not contain
 the bug any longer, as random luck, without any clues for the user.
 [...]
 
 Sorry, I'm dim, this makes no sense to me, unless both of you forgot
 smileys.
 
 Why strip off useful metadata in the public release, if you have it? Why
 not add useful metadata to the public release?
 

The point was about commit messages. My example cases where the code base had 
the bug, the code got removed for some random reason not related to the CVE, 
and then the CVE comes along. In this lifecycle there is no commit message that 
the CVE has been fixed. 

 Or is it that the CVE data is only available internally to UEFI Forum
 members?
 

The public data should be public :). 

Thanks,

Andrew Fish

 UEFI Forum's antis numbers are already in the UEFI spec, in the revision
 section. And a CVE isn't supposed to be a private/internal identifier,
 it's supposed to be public, for people to search for, not something an
 industry trade group should be obfuscating from the public.
 
 Why  keep people in the dark about vulnerabilities in the public code? I
 can see how some vendors want to keep their customers ignorant, but this
 is code that's shared but multiple vendors, exploitable open source is
 not useful to anyone.
 
 Thanks for any clarification.
 
 PS: CHIPSEC is working on a new security test for this. I hope QA teams
 at all OEMS will start using it when it's out:
 https://github.com/chipsec/chipsec
 
 
 ___
 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 0/4] FFS/FV aligment optimization (was: [RFC] small C model and LLVM/clang support for AARCH64)

2015-07-30 Thread Ard Biesheuvel
On 31 July 2015 at 01:17, Laszlo Ersek ler...@redhat.com wrote:
 On 07/27/15 15:52, Ard Biesheuvel wrote:
 On 27 July 2015 at 15:34, Liu, Yingke D yingke.d@intel.com wrote:
 Reviewed-by: Yingke Liu yingke.d@intel.com


 Thank you

 Committed as SVN r18077 ... r18080

 I do have another question related to the use of FIXED in a [Rule] section:
 since the increased alignment on AARCH64 will only be necessary for
 some toolchains, is it possible to have separate [Rule] sections?
 I did notice the 'DEBUG_MYTOOLS_IA32' in [Rule] section (for instance,
 EmbeddedPkg/EmbeddedPkg.fdf:102) but I don't quite understand how it
 works.

 Looking at the __GetFileOpts() method in
 BaseTools/Source/Python/GenFds/FdfParser.py, I think that pattern of
 tool chain keywords serves exactly the purpose you have in mind. I
 checked a couple occurrences of DEBUG_MYTOOLS_IA32 in various FDF files,
 and it always seems to restrict rules for the file type
 [Rule.Common.PEIM.TIANOCOMPRESSED]. So, in practice, it doesn matter:
 for ARM you usually (universally?) don't have LZMA compressed PEIMs,
 because PEIMs run from flash. The DEBUG_MYTOOLS_IA32 restriction doesn't
 matter because you don't have that module type anyway.


OK, so the restriction means that this particular rule will only be
applied if you are building DEBUG for IA32 with the MYTOOLS
toolchains?

 Anyway, I think if you are going to add (or try) this kind of
 restriction, then I should postpone reviewing

   [edk2] [PATCH 2/4] ArmVirtPkg: use 'auto' alignment and FIXED
  placement for XIP modules
   http://thread.gmane.org/gmane.comp.bios.edk2.devel/547/focus=545

 because you might want to post a new version of it. Isn't that so?


No, not really. The reason I was asking is for the CLANG patches I
posted as well. As explained before, CLANG requires 4 kB alignment so
that its relative symbol references resolve correctly.

 In any case, my most important question follows: the FDF spec says that
 FIXED means the file cannot be moved, it is not relocateable. But, how
 does that enable new optimizations in GenFfs, as you write in the
 linked patch? I found this in SVN r18079:

 + //
 + // Only add a special reducible padding section if
 + // - this FFS has the FFS_ATTRIB_FIXED attribute,
 + // - none of the preceding sections have alignment requirements,
 + // - the size of the padding is sufficient for the
 + // EFI_SECTION_FREEFORM_SUBTYPE_GUID header.
 + //
 + if ((FfsAttrib  FFS_ATTRIB_FIXED) != 0 
 + MaxEncounteredAlignment = 1 
 + Offset = sizeof (EFI_FREEFORM_SUBTYPE_GUID_SECTION)) {
 + SectHeader-CommonHeader.Type = EFI_SECTION_FREEFORM_SUBTYPE_GUID;
 + SectHeader-SubTypeGuid = mEfiFfsSectionAlignmentPaddingGuid;
 + } else {
 + SectHeader-CommonHeader.Type = EFI_SECTION_RAW;
 + }

 I'm quite sure you've explained why FIXED is important for this, but I
 can't recall the reason. Care to refresh my memory? Is it inherently
 related to mcmodel=small?


No, it has nothing to do with that.

The optimization I implemented results in the FFS to be unmovable,
i.e., you can no longer infer from the metadata at which alignment you
would need to load it to get the various sections to line up
correctly. So initially, I suggested that GenFfs would set the
FFS_ATTRIB_FIXED flag when applying the optimization.

However, as Liming pointed out, this may affect use cases where some
FFS is loaded into memory programmatically: such images would lose
their ability to be loaded anywhere all of a sudden due to this
optimization being applied. So instead, we set the attribute upfront
for files that don't need to be moved around, and only apply the
optimization if it has the flag set already.

Note that this does not affect the ability to shadow the PEIMs, those
are loaded from the TE images, not from the FFS containers.

-- 
Ard.


 -Original Message-
 From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
 Sent: Monday, July 27, 2015 8:32 PM
 To: Gao, Liming; Liu, Yingke D; edk2-devel@lists.01.org
 Cc: leif.lindh...@linaro.org; eugene.co...@hp.com; Ard Biesheuvel
 Subject: [PATCH v2 0/4] FFS/FV aligment optimization (was: [RFC] small C 
 model and LLVM/clang support for AARCH64)

 This series deals with the basetools optimizations to get rid of excessive 
 XIP alignment padding when using 2 KB or 4 KB alignment, which is common on 
 AARCH64, since the exception vector table requires 2 KB alignment, and the 
 small code model (which is the only one supported by the commercial LLVM 
 based compiler supplied by ARM) requires 4 KB alignment.


 
 v2 changelog
 - patches #1 and #2 are unchanged
 - patch #3 and #4 have been updated to only emit or adjust the special 
 padding
   section if the FDF [Rule] defines the FFS_ATTRIB_FIXED attributed for the 
 file
 - the adjustment logic in patch #4 has been reordered and the comments 
 updated
   to reflect more clearly that the 

[edk2] [Patch] Remove the useless code to fix build failure caused by error depend on IntelFrameworkModulePkg.

2015-07-30 Thread Eric Dong
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong eric.d...@intel.com
---
 .../Application/UiApp/BootMaint/BootOption.c   | 58 --
 .../Application/UiApp/BootMaint/UpdatePage.c   | 29 ---
 MdeModulePkg/Application/UiApp/Ui.h|  2 -
 MdeModulePkg/Application/UiApp/UiApp.inf   |  4 --
 4 files changed, 93 deletions(-)

diff --git a/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c 
b/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c
index a5bd796..1ad93bf 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c
+++ b/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c
@@ -236,17 +236,12 @@ BOpt_FindFileSystem (
   BM_MENU_ENTRY *MenuEntry;
   BM_FILE_CONTEXT   *FileContext;
   UINT16*TempStr;
   UINTN OptionNumber;
   VOID  *Buffer;
-  EFI_LEGACY_BIOS_PROTOCOL  *LegacyBios;
-  UINT16DeviceType;
-  BBS_BBS_DEVICE_PATH   BbsDevicePathNode;
-  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
   BOOLEAN   RemovableMedia;
 
-
   NoSimpleFsHandles = 0;
   NoLoadFileHandles = 0;
   OptionNumber  = 0;
   InitializeListHead (FsOptionMenu.Head);
 
@@ -442,63 +437,10 @@ BOpt_FindFileSystem (
   if (NoLoadFileHandles != 0) {
 FreePool (LoadFileHandle);
   }
 
   //
-  // Add Legacy Boot Option Support Here
-  //
-  Status = gBS-LocateProtocol (
-  gEfiLegacyBiosProtocolGuid,
-  NULL,
-  (VOID **) LegacyBios
-  );
-  if (!EFI_ERROR (Status)) {
-
-for (Index = BBS_TYPE_FLOPPY; Index = BBS_TYPE_EMBEDDED_NETWORK; Index++) 
{
-  MenuEntry = BOpt_CreateMenuEntry (BM_FILE_CONTEXT_SELECT);
-  if (NULL == MenuEntry) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  FileContext   = (BM_FILE_CONTEXT *) 
MenuEntry-VariableContext;
-
-  FileContext-IsRemovableMedia = FALSE;
-  FileContext-IsLoadFile   = TRUE;
-  FileContext-IsBootLegacy = TRUE;
-  DeviceType= (UINT16) Index;
-  BbsDevicePathNode.Header.Type = BBS_DEVICE_PATH;
-  BbsDevicePathNode.Header.SubType  = BBS_BBS_DP;
-  SetDevicePathNodeLength (
-BbsDevicePathNode.Header,
-sizeof (BBS_BBS_DEVICE_PATH)
-);
-  BbsDevicePathNode.DeviceType  = DeviceType;
-  BbsDevicePathNode.StatusFlag  = 0;
-  BbsDevicePathNode.String[0]   = 0;
-  DevicePath = AppendDevicePathNode (
-EndDevicePath,
-(EFI_DEVICE_PATH_PROTOCOL *) BbsDevicePathNode
-);
-
-  FileContext-DevicePath   = DevicePath;
-  MenuEntry-HelpString = UiDevicePathToStr (FileContext-DevicePath);
-
-  TempStr   = MenuEntry-HelpString;
-  MenuEntry-DisplayString  = AllocateZeroPool (MAX_CHAR);
-  ASSERT (MenuEntry-DisplayString != NULL);
-  UnicodeSPrint (
-MenuEntry-DisplayString,
-MAX_CHAR,
-LBoot Legacy [%s],
-TempStr
-);
-  MenuEntry-OptionNumber = OptionNumber;
-  OptionNumber++;
-  InsertTailList (FsOptionMenu.Head, MenuEntry-Link);
-}
-  }
-  //
   // Remember how many file system options are here
   //
   FsOptionMenu.MenuNumber = OptionNumber;
   return EFI_SUCCESS;
 }
diff --git a/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c 
b/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c
index ea96d13..938912b 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c
+++ b/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c
@@ -230,39 +230,10 @@ UpdateConCOMPage (
   }
 
   UpdatePageEnd (CallbackData);
 }
 
-/**
-
-  IsShellNodeDevicePath checks for the Shell device path. 
-  If it's the shell device path then return TRUE otherwise 
-  return FALSE.
-
-  @param DevicePathThe DevicePath to check
-
-  @retval  TRUEDevicePath is Shell
-  @retval  FALSE   DevicePath is not Shell
-
-**/
-BOOLEAN
-IsShellNodeDevicePath(
-  IN  EFI_DEVICE_PATH_PROTOCOL  *FilePath
-  )
-{
-
-  EFI_DEVICE_PATH_PROTOCOL  *Node;
-
-  for (Node = FilePath; !IsDevicePathEnd(Node); Node = 
NextDevicePathNode(Node)) 
-  {
-if ((DevicePathType (Node) == MEDIA_DEVICE_PATH)  (DevicePathSubType 
(Node) == MEDIA_PIWG_FW_FILE_DP)) {
-  if (!CompareMem(PcdGetPtr(PcdShellFile), 
(((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)Node)-FvFileName), sizeof(EFI_GUID)))
- return TRUE;
-}
-  }
-  return FALSE;
-}
 
 /**
   Create a list of boot option from global BootOptionMenu. It
   allow user to delete the boot option.
 
diff --git a/MdeModulePkg/Application/UiApp/Ui.h 
b/MdeModulePkg/Application/UiApp/Ui.h
index 6075b64..7ce7d47 100644
--- a/MdeModulePkg/Application/UiApp/Ui.h
+++ b/MdeModulePkg/Application/UiApp/Ui.h
@@ -25,11 +25,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR 

Re: [edk2] more code sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS

2015-07-30 Thread Andrew Fish

 On Jul 30, 2015, at 3:35 PM, Laszlo Ersek ler...@redhat.com wrote:
 
 On 07/30/15 20:54, Jordan Justen wrote:
 On 2015-07-30 10:09:34, Laszlo Ersek wrote:
 (Sigh, I left off the list address. This should be discussed publicly.
 Resending.)
 
 Clearly, the SMBIOS patches I posted and got committed last time are not
 good enough. That's because the SMBIOS 3.0 entry point is structurally
 different from the prior versions (because why not). Therefore, now that
 Wei has QEMU generate SMBIOS 3.0 payload for the firmware, the checks we
 currently have in place, fail.
 
 (First and foremost, the *size* of the 3.0 entry point is different from
 that of the 2.8 one. We correctly catch this, but we incorrectly don't
 recognize 3.0.)
 
 I can write the code for telling apart 2.8 from 3.0; that's not the
 problem. The problem is that this code would have to be triplicated, as
 things currently stand:
 
 (1) ArmVirtPkg/QemuFwCfgToPcdDxe/QemuFwCfgToPcd.c sets
PcdSmbiosVersion for MdeModulePkg/Universal/SmbiosDxe in the
ArmVirtPkg build, early in the DXE phase,
 
 (2) OvmfPkg/PlatformPei/Platform.c sets the same PCD for the same
generic driver in the OvmfPkg build, in the PEI phase,
 
 (3) OvmfPkg/SmbiosPlatformDxe/Qemu.c verifies the entry point again,
and fetches the actual tables. (Note that
(TablesSize != QemuAnchor.TableLength) is checked too, so there is a
cross-dependency between the two blobs, the entry point and the
actual tables.)
 
 Now, the current triplication we have is simplistic, so it shouldn't be
 a large problem. But, if I need to add smarts for detecting v2 versus
 v3, I would hate to triplicate that logic too. This is what code sites
 (1) and (2) both should do, for determining PcdSmbiosVersion:
 
 - Find etc/smbios/smbios-anchor. If it is missing, bail.
 - Compare the size of that fw-cfg blob against SMBIOS_TABLE_ENTRY_POINT.
- If it is a match, perform the current _SM_ and _DMI_ checks.
- If they fail, bail.
- If they match, check that QemuAnchor.MajorVersion is actually 2.
- If that fails, bail.
- If it matches, set PcdSmbiosVersion to 0x02__.
 - Otherwise, compare the size of the anchor blob against
  SMBIOS_TABLE_3_0_ENTRY_POINT.
- If it is a match, perform the (new) _SM3_ check.
- If it fails, bail.
- Check that MajorVersion (which now lives at a different offset) is
  3.
- If that fails, bail.
- If it matches, set PcdSmbiosVersion to 0x03__.
 - Otherwise, bail.
 
 In (3):
 - repeat all of the above, except do not *set* the PCD -- enforce it
  instead. ASSERT() that the PCD already has the right value (because
  at this point MdeModulePkg/Universal/SmbiosDxe is expected to have
  been dispatched, and consumed the PCD).
 - If the version is 2, then compare TableLength == tables blob size.
  If it fails, return NULL; otherwise return the tables.
 - If the version is 3, then compare TableMaximumSize = tables blob
  size. If it fails, return NULL; otherwise return the tables.
 - If the version is something different, return NULL.
 
 As I said, this can be coded as-written, but I strongly dislike the code
 triplication, and I expect you guys will too. So what do you propose? I
 could extract a tiny BASE library (a single function) that:
 - depends on QemuFwCfgLib
 - takes a BOOLEAN parameter that tells it whether to set the PCD, or
  check and enforce the PCD
 - return the detected SMBIOS version (2, 3, or missing/unknown)
 
 Then I could use this function in sites (1) and (2) for setting the PCD,
 and then use it in (3) for enforcing the PCD, and based on the returned
 SMBIOS version, perform the version-specific tables blob size check too.
 
 Would it help if you used a NULL library with a constructor to set
 gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion?
 
  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
LibraryClasses
  NULL|OvmfPkg/Library/SmbiosVersionLib/DetectSmbiosVersionLib.inf
  }
 
 Wow, that's a fantastic idea. I had used LD_PRELOAD / RTLD_NEXT tricks
 a few years back, with __attribute__ ((init)) functions even, in order
 to hook into binaries, but honestly, I never thought of the above.
 
 It's awesome:
 - for ArmVirtPkg, it will make the DXE ordering trickery go away. I can
  drop the driver I introduced just for that, and I can remove it from
  the APRIORI DXE file too.
 - For OvmfPkg, I can remove the PCD setting from PlatformPei, which
  never really felt right anyway.
 - I can hook the PCD initialization *exactly* to the point where it
  matters. SmbiosDxe is what consumes the PCD, so there's no need to
  set it earlier. And, since it is a DXE driverRe: [edk2] more code
 sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS, fw-cfg will be
  available even on ARM by then.
 - I can use a library instance without giving it a class (header file,
  dec file hunk, etc) first.
 
 My only worry is some kind of unforeseen constructor cycle in SmbiosDxe
 (or silent breakage if the constructors are not 

Re: [edk2] [PATCH v2 0/4] FFS/FV aligment optimization

2015-07-30 Thread Laszlo Ersek
On 07/31/15 01:31, Andrew Fish wrote:
 
 On Jul 30, 2015, at 4:17 PM, Laszlo Ersek ler...@redhat.com wrote:

 On 07/27/15 15:52, Ard Biesheuvel wrote:
 On 27 July 2015 at 15:34, Liu, Yingke D yingke.d@intel.com wrote:
 Reviewed-by: Yingke Liu yingke.d@intel.com


 Thank you

 Committed as SVN r18077 ... r18080

 I do have another question related to the use of FIXED in a [Rule] section:
 since the increased alignment on AARCH64 will only be necessary for
 some toolchains, is it possible to have separate [Rule] sections?
 I did notice the 'DEBUG_MYTOOLS_IA32' in [Rule] section (for instance,
 EmbeddedPkg/EmbeddedPkg.fdf:102) but I don't quite understand how it
 works.

 Looking at the __GetFileOpts() method in
 BaseTools/Source/Python/GenFds/FdfParser.py, I think that pattern of
 tool chain keywords serves exactly the purpose you have in mind. I
 checked a couple occurrences of DEBUG_MYTOOLS_IA32 in various FDF files,
 and it always seems to restrict rules for the file type
 [Rule.Common.PEIM.TIANOCOMPRESSED]. So, in practice, it doesn matter:
 for ARM you usually (universally?) don't have LZMA compressed PEIMs,
 because PEIMs run from flash.
 
 I don’t think PEIMs run from FLASH is a safe assumption to make. The
 PI architecture supports PEIMs that only run from memory, and thus
 compressed PEIMs. For modules that are specific to a boot mode, like
 recovery, it makes sense to compress them.

For example, PEIMs in OVMF run from RAM :)

I wasn't trying to justify the assumption that PEIMs run from flash
only; I just tried to explain why Ard had never seen DEBUG_MYTOOLS_IA32
matter, in those particular FDF files. Because, DEBUG_MYTOOLS_IA32 is
only used in connection with [Rule.Common.PEIM.TIANOCOMPRESSED], and
that type seems generally unused (in those FDF files). Filtering an
already empty set is not observable.

Thanks
Laszlo

 
 Thanks,
 
 Andrew Fish
 
 The DEBUG_MYTOOLS_IA32 restriction doesn't
 matter because you don't have that module type anyway.

 Anyway, I think if you are going to add (or try) this kind of
 restriction, then I should postpone reviewing

  [edk2] [PATCH 2/4] ArmVirtPkg: use 'auto' alignment and FIXED
 placement for XIP modules
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/547/focus=545

 because you might want to post a new version of it. Isn't that so?

 In any case, my most important question follows: the FDF spec says that
 FIXED means the file cannot be moved, it is not relocateable. But, how
 does that enable new optimizations in GenFfs, as you write in the
 linked patch? I found this in SVN r18079:

 + //
 + // Only add a special reducible padding section if
 + // - this FFS has the FFS_ATTRIB_FIXED attribute,
 + // - none of the preceding sections have alignment requirements,
 + // - the size of the padding is sufficient for the
 + // EFI_SECTION_FREEFORM_SUBTYPE_GUID header.
 + //
 + if ((FfsAttrib  FFS_ATTRIB_FIXED) != 0 
 + MaxEncounteredAlignment = 1 
 + Offset = sizeof (EFI_FREEFORM_SUBTYPE_GUID_SECTION)) {
 + SectHeader-CommonHeader.Type = EFI_SECTION_FREEFORM_SUBTYPE_GUID;
 + SectHeader-SubTypeGuid = mEfiFfsSectionAlignmentPaddingGuid;
 + } else {
 + SectHeader-CommonHeader.Type = EFI_SECTION_RAW;
 + }

 I'm quite sure you've explained why FIXED is important for this, but I
 can't recall the reason. Care to refresh my memory? Is it inherently
 related to mcmodel=small?

 Thanks
 Laszlo



 Thanks,
 Ard.


 -Original Message-
 From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
 Sent: Monday, July 27, 2015 8:32 PM
 To: Gao, Liming; Liu, Yingke D; edk2-devel@lists.01.org
 Cc: leif.lindh...@linaro.org; eugene.co...@hp.com; Ard Biesheuvel
 Subject: [PATCH v2 0/4] FFS/FV aligment optimization (was: [RFC] small C 
 model and LLVM/clang support for AARCH64)

 This series deals with the basetools optimizations to get rid of excessive 
 XIP alignment padding when using 2 KB or 4 KB alignment, which is common 
 on AARCH64, since the exception vector table requires 2 KB alignment, and 
 the small code model (which is the only one supported by the commercial 
 LLVM based compiler supplied by ARM) requires 4 KB alignment.


 
 v2 changelog
 - patches #1 and #2 are unchanged
 - patch #3 and #4 have been updated to only emit or adjust the special 
 padding
  section if the FDF [Rule] defines the FFS_ATTRIB_FIXED attributed for the 
 file
 - the adjustment logic in patch #4 has been reordered and the comments 
 updated
  to reflect more clearly that the misalignment and adjustment are not 
 specific
  to a single FFS section, but to the alignment of the FFS file as a whole
 - patch #4 now clears the alignment bits in the FFS header since they have 
 no
  meaning in FFS files that have been modified in place
 - replaced pointer arithmetic using VOID* pointers

 
 v1 changelog 

Re: [edk2] [Patch 2/2] Update copyright info, use BDS license.

2015-07-30 Thread Dong, Eric
Daryl and Ard,

Sorry for the incorrect description. This file is new created and  changed from 
an old existed file, so the old copyright is error kept. 
I have update the copyright info. Because this is new added file, so I only 
kept the 2015 copyright.

Thanks,
Eric

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daryl 
McDaniel
Sent: Friday, July 31, 2015 2:32 AM
To: 'Ard Biesheuvel'; Dong, Eric
Cc: Ni, Ruiyu; edk2-devel@lists.01.org; Gao, Liming
Subject: Re: [edk2] [Patch 2/2] Update copyright info, use BDS license.

The copyright information was incorrectly updated.
The correct copyright is:

Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.BR

Previous copyrights must be maintained.

This patch should be rejected and a new one submitted with the corrected 
copyright notice and subject line.

Daryl McDaniel


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, July 30, 2015 12:27 AM
To: Eric Dong eric.d...@intel.com
Cc: Ruiyu Ni ruiyu...@intel.com; edk2-devel@lists.01.org; Gao, Liming 
liming@intel.com
Subject: Re: [edk2] [Patch 2/2] Update copyright info, use BDS license.

On 29 July 2015 at 10:59, Eric Dong eric.d...@intel.com wrote:
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Eric Dong eric.d...@intel.com

The license is called 'BSD' not 'BDS'

Could you fix up the commit titles please?

--
Ard.

 ---
  .../Include/Guid/HiiBootMaintenanceFormset.h   | 28
++
  1 file changed, 12 insertions(+), 16 deletions(-)

 diff --git a/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 b/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 index 7c809f4..393cc95 100644
 --- a/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 +++ b/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 @@ -1,25 +1,21 @@
  //
 -// This file contains 'Framework Code' and is licensed as such -// 
 under the terms of your license agreement with Intel or your -// 
 vendor.  This file may not be modified, except as allowed by -// 
 additional terms of your license agreement.
 -//
 -/**@file
 -Constants and declarations that are common accross PEI and DXE.
 -
 -Copyright (c) 2011, Intel Corporation. All rights reserved.BR -This 
 software and associated documentation (if any) is furnished -under a 
 license and may only be used or copied in accordance -with the terms 
 of the license. Except as permitted by such -license, no part of this 
 software or documentation may be -reproduced, stored in a retrieval 
 system, or transmitted in any -form or by any means without the 
 express written consent of -Intel Corporation.
 +/** @file
 +  Guid definition for Boot Maintainence Formset.
 +
 +Copyright (c) 2015, Intel Corporation. All rights reserved.BR 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.

  **/

 +
  #ifndef __HII_BOOT_MAINTENANCE_FORMSET_H__
  #define __HII_BOOT_MAINTENANCE_FORMSET_H__

  ///
  /// Guid define to group the item show on the Boot Menaintenance 
 Manager
Menu.
 --
 1.9.5.msysgit.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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
//
/** @file
  Guid definition for Boot Maintainence Formset.

Copyright (c) 2015, Intel Corporation. All rights reserved.BR
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.

**/


#ifndef __HII_BOOT_MAINTENANCE_FORMSET_H__
#define __HII_BOOT_MAINTENANCE_FORMSET_H__

///
/// Guid define to group the item show on the Boot Menaintenance Manager Menu.
///
#define EFI_IFR_BOOT_MAINTENANCE_GUID \
  { 0xb2dedc91, 0xd59f, 0x48d2, { 0x89, 0x8a, 0x12, 0x49, 0xc, 0x74, 0xa4, 0xe0 
} }


extern EFI_GUID gEfiIfrBootMaintenanceGuid;

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

Re: [edk2] [PATCH v2 0/4] FFS/FV aligment optimization

2015-07-30 Thread Laszlo Ersek
On 07/31/15 01:49, Ard Biesheuvel wrote:
 On 31 July 2015 at 01:17, Laszlo Ersek ler...@redhat.com wrote:
 On 07/27/15 15:52, Ard Biesheuvel wrote:
 On 27 July 2015 at 15:34, Liu, Yingke D yingke.d@intel.com wrote:
 Reviewed-by: Yingke Liu yingke.d@intel.com


 Thank you

 Committed as SVN r18077 ... r18080

 I do have another question related to the use of FIXED in a [Rule] section:
 since the increased alignment on AARCH64 will only be necessary for
 some toolchains, is it possible to have separate [Rule] sections?
 I did notice the 'DEBUG_MYTOOLS_IA32' in [Rule] section (for instance,
 EmbeddedPkg/EmbeddedPkg.fdf:102) but I don't quite understand how it
 works.

 Looking at the __GetFileOpts() method in
 BaseTools/Source/Python/GenFds/FdfParser.py, I think that pattern of
 tool chain keywords serves exactly the purpose you have in mind. I
 checked a couple occurrences of DEBUG_MYTOOLS_IA32 in various FDF files,
 and it always seems to restrict rules for the file type
 [Rule.Common.PEIM.TIANOCOMPRESSED]. So, in practice, it doesn matter:
 for ARM you usually (universally?) don't have LZMA compressed PEIMs,
 because PEIMs run from flash. The DEBUG_MYTOOLS_IA32 restriction doesn't
 matter because you don't have that module type anyway.

 
 OK, so the restriction means that this particular rule will only be
 applied if you are building DEBUG for IA32 with the MYTOOLS
 toolchains?

I believe so, yes.

 
 Anyway, I think if you are going to add (or try) this kind of
 restriction, then I should postpone reviewing

   [edk2] [PATCH 2/4] ArmVirtPkg: use 'auto' alignment and FIXED
  placement for XIP modules
   http://thread.gmane.org/gmane.comp.bios.edk2.devel/547/focus=545

 because you might want to post a new version of it. Isn't that so?

 
 No, not really. The reason I was asking is for the CLANG patches I
 posted as well. As explained before, CLANG requires 4 kB alignment so
 that its relative symbol references resolve correctly.

Okay. I thought that you wanted to add *_CLANG_* keywords or some such
to the rules, after the FIXED keywod.

 
 In any case, my most important question follows: the FDF spec says that
 FIXED means the file cannot be moved, it is not relocateable. But, how
 does that enable new optimizations in GenFfs, as you write in the
 linked patch? I found this in SVN r18079:

 + //
 + // Only add a special reducible padding section if
 + // - this FFS has the FFS_ATTRIB_FIXED attribute,
 + // - none of the preceding sections have alignment requirements,
 + // - the size of the padding is sufficient for the
 + // EFI_SECTION_FREEFORM_SUBTYPE_GUID header.
 + //
 + if ((FfsAttrib  FFS_ATTRIB_FIXED) != 0 
 + MaxEncounteredAlignment = 1 
 + Offset = sizeof (EFI_FREEFORM_SUBTYPE_GUID_SECTION)) {
 + SectHeader-CommonHeader.Type = EFI_SECTION_FREEFORM_SUBTYPE_GUID;
 + SectHeader-SubTypeGuid = mEfiFfsSectionAlignmentPaddingGuid;
 + } else {
 + SectHeader-CommonHeader.Type = EFI_SECTION_RAW;
 + }

 I'm quite sure you've explained why FIXED is important for this, but I
 can't recall the reason. Care to refresh my memory? Is it inherently
 related to mcmodel=small?

 
 No, it has nothing to do with that.
 
 The optimization I implemented results in the FFS to be unmovable,
 i.e., you can no longer infer from the metadata at which alignment you
 would need to load it to get the various sections to line up
 correctly. So initially, I suggested that GenFfs would set the
 FFS_ATTRIB_FIXED flag when applying the optimization.
 
 However, as Liming pointed out, this may affect use cases where some
 FFS is loaded into memory programmatically: such images would lose
 their ability to be loaded anywhere all of a sudden due to this
 optimization being applied. So instead, we set the attribute upfront
 for files that don't need to be moved around, and only apply the
 optimization if it has the flag set already.

Okay. More or less understood. Thanks.
Laszlo


 Note that this does not affect the ability to shadow the PEIMs, those
 are loaded from the TE images, not from the FFS containers.
 

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


[edk2] [patch] MdeModulePkg:Fix the issue FindQuestionFromProgress in SetupBrowserDxe is broken

2015-07-30 Thread Dandan Bi
If the storage of the question is 
EFI_HII_VARSTORE_BUFFER/EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER,
in SetupBrowserDxe the configuration stings contain uppercase,but 
HiiDataBaseDxe generates the
ConfigResp string in lowercase,they mismatch,so FindQuestionFromProgress 
function is broken.
Now convert the configuration string in SetupBrowserDxe to lowercase to fix 
this issue.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi dandan...@intel.com
---
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c |  1 +
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.h| 13 +
 2 files changed, 14 insertions(+)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 953e3a5..8e8607d 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -713,10 +713,11 @@ InitializeRequestElement (
30 * sizeof (CHAR16),
LOFFSET=%04xWIDTH=%04x,
Question-VarStoreInfo.VarOffset,
Question-StorageWidth
);
+HiiToLower(RequestElement);
 Question-BlockName = AllocateCopyPool ((StrLen + 1) * sizeof (CHAR16), 
RequestElement);
   } else {
 StrLen = UnicodeSPrint (RequestElement, 30 * sizeof (CHAR16), L%s, 
Question-VariableName);
   }
 
diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h 
b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h
index cfd8133..1238197 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h
@@ -1847,6 +1847,19 @@ GetFstStgFromBrsStg (
 BOOLEAN
 ReconnectController (
   IN EFI_HANDLE   DriverHandle
   );
 
+/**
+  Converts the unicode character of the string from uppercase to lowercase.
+  This is a internal function.
+
+  @param ConfigString  String to be converted
+
+**/
+VOID
+EFIAPI
+HiiToLower (
+  IN EFI_STRING  ConfigString
+  );
+
 #endif
-- 
1.9.5.msysgit.1

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


[edk2] more code sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS

2015-07-30 Thread Laszlo Ersek
(Sigh, I left off the list address. This should be discussed publicly.
Resending.)

Clearly, the SMBIOS patches I posted and got committed last time are not
good enough. That's because the SMBIOS 3.0 entry point is structurally
different from the prior versions (because why not). Therefore, now that
Wei has QEMU generate SMBIOS 3.0 payload for the firmware, the checks we
currently have in place, fail.

(First and foremost, the *size* of the 3.0 entry point is different from
that of the 2.8 one. We correctly catch this, but we incorrectly don't
recognize 3.0.)

I can write the code for telling apart 2.8 from 3.0; that's not the
problem. The problem is that this code would have to be triplicated, as
things currently stand:

(1) ArmVirtPkg/QemuFwCfgToPcdDxe/QemuFwCfgToPcd.c sets
PcdSmbiosVersion for MdeModulePkg/Universal/SmbiosDxe in the
ArmVirtPkg build, early in the DXE phase,

(2) OvmfPkg/PlatformPei/Platform.c sets the same PCD for the same
generic driver in the OvmfPkg build, in the PEI phase,

(3) OvmfPkg/SmbiosPlatformDxe/Qemu.c verifies the entry point again,
and fetches the actual tables. (Note that
(TablesSize != QemuAnchor.TableLength) is checked too, so there is a
cross-dependency between the two blobs, the entry point and the
actual tables.)

Now, the current triplication we have is simplistic, so it shouldn't be
a large problem. But, if I need to add smarts for detecting v2 versus
v3, I would hate to triplicate that logic too. This is what code sites
(1) and (2) both should do, for determining PcdSmbiosVersion:

- Find etc/smbios/smbios-anchor. If it is missing, bail.
- Compare the size of that fw-cfg blob against SMBIOS_TABLE_ENTRY_POINT.
- If it is a match, perform the current _SM_ and _DMI_ checks.
- If they fail, bail.
- If they match, check that QemuAnchor.MajorVersion is actually 2.
- If that fails, bail.
- If it matches, set PcdSmbiosVersion to 0x02__.
- Otherwise, compare the size of the anchor blob against
  SMBIOS_TABLE_3_0_ENTRY_POINT.
- If it is a match, perform the (new) _SM3_ check.
- If it fails, bail.
- Check that MajorVersion (which now lives at a different offset) is
  3.
- If that fails, bail.
- If it matches, set PcdSmbiosVersion to 0x03__.
- Otherwise, bail.

In (3):
- repeat all of the above, except do not *set* the PCD -- enforce it
  instead. ASSERT() that the PCD already has the right value (because
  at this point MdeModulePkg/Universal/SmbiosDxe is expected to have
  been dispatched, and consumed the PCD).
- If the version is 2, then compare TableLength == tables blob size.
  If it fails, return NULL; otherwise return the tables.
- If the version is 3, then compare TableMaximumSize = tables blob
  size. If it fails, return NULL; otherwise return the tables.
- If the version is something different, return NULL.

As I said, this can be coded as-written, but I strongly dislike the code
triplication, and I expect you guys will too. So what do you propose? I
could extract a tiny BASE library (a single function) that:
- depends on QemuFwCfgLib
- takes a BOOLEAN parameter that tells it whether to set the PCD, or
  check and enforce the PCD
- return the detected SMBIOS version (2, 3, or missing/unknown)

Then I could use this function in sites (1) and (2) for setting the PCD,
and then use it in (3) for enforcing the PCD, and based on the returned
SMBIOS version, perform the version-specific tables blob size check too.

Small problem: you are going to hate me for introducing a
single-function library *class*.

So how do I share *one function* between different *phase* modules of
different top-level packages? Should we introduce BaseCrapLib ^W
BaseUtilityLib, and just keep throwing such functions in there?

If so, what names (pathnames, basenames, library class and instance
names etc) do you recommend?

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


Re: [edk2] today's US CERT UEFI advisory

2015-07-30 Thread Bill Jacobs (billjac)
Bravo!

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Thursday, July 30, 2015 9:58 AM
To: Blibbet; edk2-de...@ml01.01.org
Cc: Peter Jones
Subject: Re: [edk2] today's US CERT UEFI advisory

On 07/30/15 17:49, Blibbet wrote:
 FYI, in case any OEM's missed today's US-CERT UEFI vulnerability notice:
 
 http://firmwaresecurity.com/2015/07/30/us-cert-bios-vulnerability-note
 -vu577140/
 
 Remember that any TianoCore-based bugs may be in your platorm:
 
 https://twitter.com/XenoKovah/status/623483244890189824
 
 Can anyone clarify if the code in this vulnerability is in TianoCore 
 or external vendor-specific?

I think both sides, the firmware researcher side, and the firmware vendor side, 
have ample room for improvement in their behavior.

The researcher side should tone down their repulsive sensationalism, selling 
each security bug to the public as the end of the world, and showing off 
themselves as the most leet security startup ever, seeking to score hefty $$$ 
gigs right after. Responsible disclosure exists.

The vendor side should get their act together, and react to, and *address*, 
responsible disclosures in a *timely* manner. Among other things, this requires:

- spelling out CVE numbers in the subject lines of edk2 commit
  messages, when edk2 is affected and some patches address those issues,

- publicly release *plain text* advisories, rather than privately
  circulated PDF files that contain the problem description as embedded
  image files, with (probable) watermarks embedded as well.

Edk2's track record has been absolutely deplorable in this regard. But, as I 
said, both sides have a lot of room for improvement; the hype generated by some 
white hats around each single discovery is hugely childish and unprofessional 
too.

(Yes, I have discovered, reported, and fixed vulnerabilities too, in various 
projects. I have never tried to exploit them though, so maybe that's why I'm so 
unexcited.)

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


Re: [edk2] [Patch 2/2] Update copyright info, use BDS license.

2015-07-30 Thread Daryl McDaniel
The copyright information was incorrectly updated.
The correct copyright is:

Copyright (c) 2011 - 2015, Intel Corporation. All rights
reserved.BR

Previous copyrights must be maintained.

This patch should be rejected and a new one submitted with the corrected
copyright notice and subject line.

Daryl McDaniel


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
Biesheuvel
Sent: Thursday, July 30, 2015 12:27 AM
To: Eric Dong eric.d...@intel.com
Cc: Ruiyu Ni ruiyu...@intel.com; edk2-devel@lists.01.org; Gao, Liming
liming@intel.com
Subject: Re: [edk2] [Patch 2/2] Update copyright info, use BDS license.

On 29 July 2015 at 10:59, Eric Dong eric.d...@intel.com wrote:
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Eric Dong eric.d...@intel.com

The license is called 'BSD' not 'BDS'

Could you fix up the commit titles please?

--
Ard.

 ---
  .../Include/Guid/HiiBootMaintenanceFormset.h   | 28
++
  1 file changed, 12 insertions(+), 16 deletions(-)

 diff --git a/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h 
 b/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 index 7c809f4..393cc95 100644
 --- a/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 +++ b/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 @@ -1,25 +1,21 @@
  //
 -// This file contains 'Framework Code' and is licensed as such -// 
 under the terms of your license agreement with Intel or your -// 
 vendor.  This file may not be modified, except as allowed by -// 
 additional terms of your license agreement.
 -//
 -/**@file
 -Constants and declarations that are common accross PEI and DXE.
 -
 -Copyright (c) 2011, Intel Corporation. All rights reserved.BR -This 
 software and associated documentation (if any) is furnished -under a 
 license and may only be used or copied in accordance -with the terms 
 of the license. Except as permitted by such -license, no part of this 
 software or documentation may be -reproduced, stored in a retrieval 
 system, or transmitted in any -form or by any means without the 
 express written consent of -Intel Corporation.
 +/** @file
 +  Guid definition for Boot Maintainence Formset.
 +
 +Copyright (c) 2015, Intel Corporation. All rights reserved.BR 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.

  **/

 +
  #ifndef __HII_BOOT_MAINTENANCE_FORMSET_H__
  #define __HII_BOOT_MAINTENANCE_FORMSET_H__

  ///
  /// Guid define to group the item show on the Boot Menaintenance Manager
Menu.
 --
 1.9.5.msysgit.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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch 2/2] Update copyright info, use BDS license.

2015-07-30 Thread Ard Biesheuvel
On 29 July 2015 at 10:59, Eric Dong eric.d...@intel.com wrote:
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Eric Dong eric.d...@intel.com

The license is called 'BSD' not 'BDS'

Could you fix up the commit titles please?

-- 
Ard.

 ---
  .../Include/Guid/HiiBootMaintenanceFormset.h   | 28 
 ++
  1 file changed, 12 insertions(+), 16 deletions(-)

 diff --git a/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h 
 b/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 index 7c809f4..393cc95 100644
 --- a/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 +++ b/MdeModulePkg/Include/Guid/HiiBootMaintenanceFormset.h
 @@ -1,25 +1,21 @@
  //
 -// This file contains 'Framework Code' and is licensed as such
 -// under the terms of your license agreement with Intel or your
 -// vendor.  This file may not be modified, except as allowed by
 -// additional terms of your license agreement.
 -//
 -/**@file
 -Constants and declarations that are common accross PEI and DXE.
 -
 -Copyright (c) 2011, Intel Corporation. All rights reserved.BR
 -This software and associated documentation (if any) is furnished
 -under a license and may only be used or copied in accordance
 -with the terms of the license. Except as permitted by such
 -license, no part of this software or documentation may be
 -reproduced, stored in a retrieval system, or transmitted in any
 -form or by any means without the express written consent of
 -Intel Corporation.
 +/** @file
 +  Guid definition for Boot Maintainence Formset.
 +
 +Copyright (c) 2015, Intel Corporation. All rights reserved.BR
 +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.

  **/

 +
  #ifndef __HII_BOOT_MAINTENANCE_FORMSET_H__
  #define __HII_BOOT_MAINTENANCE_FORMSET_H__

  ///
  /// Guid define to group the item show on the Boot Menaintenance Manager 
 Menu.
 --
 1.9.5.msysgit.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] today's US CERT UEFI advisory

2015-07-30 Thread Blibbet
FYI, in case any OEM's missed today's US-CERT UEFI vulnerability notice:

http://firmwaresecurity.com/2015/07/30/us-cert-bios-vulnerability-note-vu577140/

Remember that any TianoCore-based bugs may be in your platorm:

https://twitter.com/XenoKovah/status/623483244890189824

Can anyone clarify if the code in this vulnerability is in TianoCore or
external vendor-specific?

Thanks,
Lee
RSS: http://firmwaresecurity.com/feed
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] Vlv2DeviceRefCodePkg: don't redefine struct typedefs

2015-07-30 Thread Bruce Cran

On 7/27/2015 11:39 PM, He, Tim wrote:

Yes, these 2 typedefs is being defined twice, we need to remove the second 
definition, I will check in the code patch.


Thanks. Just to check, how long is there normally between accepting the 
patch and committing it? I don't see any of my recent changesets in 
https://edk2.bluestop.org/diffusion/EDK/history/branches/UDK2014.SP1/ or 
https://github.com/tianocore/edk2/commits/svn/branches/UDK2014.SP1 .


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


Re: [edk2] more code sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS

2015-07-30 Thread Laszlo Ersek
(resending this one too)

On 07/30/15 19:09, Laszlo Ersek wrote:
 (Sigh, I left off the list address. This should be discussed publicly.
 Resending.)
 
 Clearly, the SMBIOS patches I posted and got committed last time are not
 good enough. That's because the SMBIOS 3.0 entry point is structurally
 different from the prior versions (because why not). Therefore, now that
 Wei has QEMU generate SMBIOS 3.0 payload for the firmware, the checks we
 currently have in place, fail.
 
 (First and foremost, the *size* of the 3.0 entry point is different from
 that of the 2.8 one. We correctly catch this, but we incorrectly don't
 recognize 3.0.)
 
 I can write the code for telling apart 2.8 from 3.0; that's not the
 problem. The problem is that this code would have to be triplicated, as
 things currently stand:
 
 (1) ArmVirtPkg/QemuFwCfgToPcdDxe/QemuFwCfgToPcd.c sets
 PcdSmbiosVersion for MdeModulePkg/Universal/SmbiosDxe in the
 ArmVirtPkg build, early in the DXE phase,
 
 (2) OvmfPkg/PlatformPei/Platform.c sets the same PCD for the same
 generic driver in the OvmfPkg build, in the PEI phase,
 
 (3) OvmfPkg/SmbiosPlatformDxe/Qemu.c verifies the entry point again,
 and fetches the actual tables. (Note that
 (TablesSize != QemuAnchor.TableLength) is checked too, so there is a
 cross-dependency between the two blobs, the entry point and the
 actual tables.)
 
 Now, the current triplication we have is simplistic, so it shouldn't be
 a large problem. But, if I need to add smarts for detecting v2 versus
 v3, I would hate to triplicate that logic too. This is what code sites
 (1) and (2) both should do, for determining PcdSmbiosVersion:
 
 - Find etc/smbios/smbios-anchor. If it is missing, bail.
 - Compare the size of that fw-cfg blob against SMBIOS_TABLE_ENTRY_POINT.
 - If it is a match, perform the current _SM_ and _DMI_ checks.
 - If they fail, bail.
 - If they match, check that QemuAnchor.MajorVersion is actually 2.
 - If that fails, bail.
 - If it matches, set PcdSmbiosVersion to 0x02__.
 - Otherwise, compare the size of the anchor blob against
   SMBIOS_TABLE_3_0_ENTRY_POINT.
 - If it is a match, perform the (new) _SM3_ check.
 - If it fails, bail.
 - Check that MajorVersion (which now lives at a different offset) is
   3.
 - If that fails, bail.
 - If it matches, set PcdSmbiosVersion to 0x03__.
 - Otherwise, bail.
 
 In (3):
 - repeat all of the above, except do not *set* the PCD -- enforce it
   instead. ASSERT() that the PCD already has the right value (because
   at this point MdeModulePkg/Universal/SmbiosDxe is expected to have
   been dispatched, and consumed the PCD).
 - If the version is 2, then compare TableLength == tables blob size.
   If it fails, return NULL; otherwise return the tables.
 - If the version is 3, then compare TableMaximumSize = tables blob
   size. If it fails, return NULL; otherwise return the tables.
 - If the version is something different, return NULL.
 
 As I said, this can be coded as-written, but I strongly dislike the code
 triplication, and I expect you guys will too. So what do you propose? I
 could extract a tiny BASE library (a single function) that:
 - depends on QemuFwCfgLib
 - takes a BOOLEAN parameter that tells it whether to set the PCD, or
   check and enforce the PCD
 - return the detected SMBIOS version (2, 3, or missing/unknown)
 
 Then I could use this function in sites (1) and (2) for setting the PCD,
 and then use it in (3) for enforcing the PCD, and based on the returned
 SMBIOS version, perform the version-specific tables blob size check too.
 
 Small problem: you are going to hate me for introducing a
 single-function library *class*.
 
 So how do I share *one function* between different *phase* modules of
 different top-level packages? Should we introduce BaseCrapLib ^W
 BaseUtilityLib, and just keep throwing such functions in there?
 
 If so, what names (pathnames, basenames, library class and instance
 names etc) do you recommend?

... Alternatively: I could unify ArmVirtPkg/Library/QemuFwCfgLib and
OvmfPkg/Library/QemuFwCfgLib (under the latter location). There would
be X86* files, Arm* files, three INF files:
- X86QemuFwCfgSecLib.inf,
- X86QemuFwCfgLib.inf,
- ArmQemuFwCfgLib.inf,

and the common code between ARM and X86 would be extracted into a shared
file. (We already have some minimal code duplication between them.) The
one function discussed above could be then added to this common code file.

What do you guys think?

Thanks
Laszlo

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


Re: [edk2] [PATCH 11/58] OvmfPkg: implement EFI_SMM_CONTROL2_PROTOCOL with a DXE_RUNTIME_DRIVER

2015-07-30 Thread Laszlo Ersek
On 07/30/15 20:50, Paolo Bonzini wrote:
 
 
 On 28/07/2015 20:44, Laszlo Ersek wrote:
 I have a significant update for this patch. On S3 resume, the APMC_EN
 bit (and other bits) are cleared in SMI_EN (which is necessary, see qemu
 commit be66680e). For the trigger method to work right after S3 resume,
 the APMC_EN bit must be set again (otherwise a working-as-expected OS
 will not be able to use the variable services after S3 resume).

 I decided to put the S3 boot script to actual use this time, and I am
 now writing the necessary opcodes to the bootscript in the entry point
 (more precisely, in a protocol notify callback). This way chipset state
 is restored by the time the OS gets the control back.
 
 Ah, so the lockbox is accessed straight from TSEG to retrieve the S3
 bootscript, without going through the SMM handler?

Exactly. Please see the EFI_NOT_STARTED occurrences in

  [edk2] [PATCH 09/58] OvmfPkg: add PEIM for providing TSEG-as-SMRAM
   during PEI

both code and commit message. See also the last paragraph of the
Variable store and LockBox in SMRAM section in the OVMF whitepaper,
and the Intel whitepaper referenced in there.

The PEIM that orchestrates the S3 resume boot path, S3Resume2Pei, has
direct access to SMRAM (it can open it and close it as it pleases)
because SMRAM has not been locked down yet. So it doesn't need to
trigger an SMI, in order to enter SMM, in order to access SMRAM.

It looks for EFI_PEI_SMM_COMMUNICATION_PPI, yes, and we provide it with
one, but we just say, hey, EFI_NOT_STARTED yet. Then it goes to
PEI_SMM_ACCESS_PPI (which we do implement in full), to open / close
SMRAM as appropriate, and it restores the lockbox contents with direct
access. (Including the boot script stuff saved in lockbox.)

This is not very easy to see, because it is split between S3Resume2Pei
and SmmLockBoxPeiLib (which the former links against).

*After* the lockboxes are restored, the PEIM locks down SMRAM (again
using our PEI_SMM_ACCESS_PPI), and then executes the boot script. Since
the OS is soon going to reacquire control, this runtime DXE driver that
writes to APM_CNT to raise an SMI must be able to work again. (Ie.
APM_CNT should actually trigger an SMI, otherwise the OS will just stare
cluelessly at us, when the dependent variable services return error codes.)

I could have updated the SmmControl2DxeTrigger() function itself to set
APMC_EN in SMI_EN on every call -- SmmControl2Dxe is unaware of an
intervening S3 suspend/resume cycle --, but for this use case the boot
script felt really appropriate. It now restores a piece of chipset state
that relates strongly to a runtime DXE driver, which is practically what
the boot script concept was invented for.

... Sorry about over-explaining it; I just got real enthused over your
insight! :)

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


Re: [edk2] [PATCH 11/58] OvmfPkg: implement EFI_SMM_CONTROL2_PROTOCOL with a DXE_RUNTIME_DRIVER

2015-07-30 Thread Paolo Bonzini


On 28/07/2015 20:44, Laszlo Ersek wrote:
 I have a significant update for this patch. On S3 resume, the APMC_EN
 bit (and other bits) are cleared in SMI_EN (which is necessary, see qemu
 commit be66680e). For the trigger method to work right after S3 resume,
 the APMC_EN bit must be set again (otherwise a working-as-expected OS
 will not be able to use the variable services after S3 resume).
 
 I decided to put the S3 boot script to actual use this time, and I am
 now writing the necessary opcodes to the bootscript in the entry point
 (more precisely, in a protocol notify callback). This way chipset state
 is restored by the time the OS gets the control back.

Ah, so the lockbox is accessed straight from TSEG to retrieve the S3
bootscript, without going through the SMM handler?

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


Re: [edk2] today's US CERT UEFI advisory

2015-07-30 Thread Andrew Fish

 On Jul 30, 2015, at 9:58 AM, Laszlo Ersek ler...@redhat.com wrote:
 
 On 07/30/15 17:49, Blibbet wrote:
 FYI, in case any OEM's missed today's US-CERT UEFI vulnerability notice:
 
 http://firmwaresecurity.com/2015/07/30/us-cert-bios-vulnerability-note-vu577140/
 
 Remember that any TianoCore-based bugs may be in your platorm:
 
 https://twitter.com/XenoKovah/status/623483244890189824
 
 Can anyone clarify if the code in this vulnerability is in TianoCore or
 external vendor-specific?

I don’t think that any platform on TianoCore are “secure” in that the ROMs are 
not signed, and only signed ROM images can be placed in the ROM. That is a 
“platform feature”. 

Locking the ROM is chipset/platform code in general. 

Italian cyber-security firm Hacking Team, allegedly, sells a UEFI root kit. 
This root kit reads the ROM, patches in extra drivers (NTFS driver, malware, 
etc.), and flash updates the ROM. It actually uses an open source tool that 
exists to let folks “mod” their BIOS. It requires physical access to the 
machine. 


 I think both sides, the firmware researcher side, and the firmware
 vendor side, have ample room for improvement in their behavior.
 
 The researcher side should tone down their repulsive sensationalism,
 selling each security bug to the public as the end of the world, and
 showing off themselves as the most leet security startup ever, seeking
 to score hefty $$$ gigs right after. Responsible disclosure exists.
 

+1

 The vendor side should get their act together, and react to, and
 *address*, responsible disclosures in a *timely* manner. Among other
 things, this requires:
 
 - spelling out CVE numbers in the subject lines of edk2 commit
  messages, when edk2 is affected and some patches address those issues,
 
 - publicly release *plain text* advisories, rather than privately
  circulated PDF files that contain the problem description as embedded
  image files, with (probable) watermarks embedded as well.
 
 Edk2's track record has been absolutely deplorable in this regard. But,
 as I said, both sides have a lot of room for improvement; the hype
 generated by some white hats around each single discovery is hugely
 childish and unprofessional too.
 

I’m not sure that the commit messages is the best way to communicate the info, 
and something could get fixed prior CVE going out. I agree it would be nice to 
have an easier way to cross reference this info. I’m going to ping some UEFI 
Forum folks about this.

Thanks,

Andrew Fish

PS. If folks want to brush up on edk2/UEFI security related topics, there has 
been a lot of security related presentations at UEFI Plugfests that are 
archived here: http://www.uefi.org/learning_center/presentationsandvideos 
http://www.uefi.org/learning_center/presentationsandvideos



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


Re: [edk2] [Patch] Remove the useless code to fix build failure caused by error depend on IntelFrameworkModulePkg.

2015-07-30 Thread Gao, Liming
Reviewed-by: Liming Gao liming@intel.com

-Original Message-
From: Dong, Eric 
Sent: Friday, July 31, 2015 9:32 AM
To: Ni, Ruiyu; Gao, Liming; edk2-devel@lists.01.org
Subject: [Patch] Remove the useless code to fix build failure caused by error 
depend on IntelFrameworkModulePkg.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong eric.d...@intel.com
---
 .../Application/UiApp/BootMaint/BootOption.c   | 58 --
 .../Application/UiApp/BootMaint/UpdatePage.c   | 29 ---
 MdeModulePkg/Application/UiApp/Ui.h|  2 -
 MdeModulePkg/Application/UiApp/UiApp.inf   |  4 --
 4 files changed, 93 deletions(-)

diff --git a/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c 
b/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c
index a5bd796..1ad93bf 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c
+++ b/MdeModulePkg/Application/UiApp/BootMaint/BootOption.c
@@ -236,17 +236,12 @@ BOpt_FindFileSystem (
   BM_MENU_ENTRY *MenuEntry;
   BM_FILE_CONTEXT   *FileContext;
   UINT16*TempStr;
   UINTN OptionNumber;
   VOID  *Buffer;
-  EFI_LEGACY_BIOS_PROTOCOL  *LegacyBios;
-  UINT16DeviceType;
-  BBS_BBS_DEVICE_PATH   BbsDevicePathNode;
-  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
   BOOLEAN   RemovableMedia;
 
-
   NoSimpleFsHandles = 0;
   NoLoadFileHandles = 0;
   OptionNumber  = 0;
   InitializeListHead (FsOptionMenu.Head);
 
@@ -442,63 +437,10 @@ BOpt_FindFileSystem (
   if (NoLoadFileHandles != 0) {
 FreePool (LoadFileHandle);
   }
 
   //
-  // Add Legacy Boot Option Support Here
-  //
-  Status = gBS-LocateProtocol (
-  gEfiLegacyBiosProtocolGuid,
-  NULL,
-  (VOID **) LegacyBios
-  );
-  if (!EFI_ERROR (Status)) {
-
-for (Index = BBS_TYPE_FLOPPY; Index = BBS_TYPE_EMBEDDED_NETWORK; Index++) 
{
-  MenuEntry = BOpt_CreateMenuEntry (BM_FILE_CONTEXT_SELECT);
-  if (NULL == MenuEntry) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  FileContext   = (BM_FILE_CONTEXT *) 
MenuEntry-VariableContext;
-
-  FileContext-IsRemovableMedia = FALSE;
-  FileContext-IsLoadFile   = TRUE;
-  FileContext-IsBootLegacy = TRUE;
-  DeviceType= (UINT16) Index;
-  BbsDevicePathNode.Header.Type = BBS_DEVICE_PATH;
-  BbsDevicePathNode.Header.SubType  = BBS_BBS_DP;
-  SetDevicePathNodeLength (
-BbsDevicePathNode.Header,
-sizeof (BBS_BBS_DEVICE_PATH)
-);
-  BbsDevicePathNode.DeviceType  = DeviceType;
-  BbsDevicePathNode.StatusFlag  = 0;
-  BbsDevicePathNode.String[0]   = 0;
-  DevicePath = AppendDevicePathNode (
-EndDevicePath,
-(EFI_DEVICE_PATH_PROTOCOL *) BbsDevicePathNode
-);
-
-  FileContext-DevicePath   = DevicePath;
-  MenuEntry-HelpString = UiDevicePathToStr (FileContext-DevicePath);
-
-  TempStr   = MenuEntry-HelpString;
-  MenuEntry-DisplayString  = AllocateZeroPool (MAX_CHAR);
-  ASSERT (MenuEntry-DisplayString != NULL);
-  UnicodeSPrint (
-MenuEntry-DisplayString,
-MAX_CHAR,
-LBoot Legacy [%s],
-TempStr
-);
-  MenuEntry-OptionNumber = OptionNumber;
-  OptionNumber++;
-  InsertTailList (FsOptionMenu.Head, MenuEntry-Link);
-}
-  }
-  //
   // Remember how many file system options are here
   //
   FsOptionMenu.MenuNumber = OptionNumber;
   return EFI_SUCCESS;
 }
diff --git a/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c 
b/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c
index ea96d13..938912b 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c
+++ b/MdeModulePkg/Application/UiApp/BootMaint/UpdatePage.c
@@ -230,39 +230,10 @@ UpdateConCOMPage (
   }
 
   UpdatePageEnd (CallbackData);
 }
 
-/**
-
-  IsShellNodeDevicePath checks for the Shell device path. 
-  If it's the shell device path then return TRUE otherwise
-  return FALSE.
-
-  @param DevicePathThe DevicePath to check
-
-  @retval  TRUEDevicePath is Shell
-  @retval  FALSE   DevicePath is not Shell
-
-**/
-BOOLEAN
-IsShellNodeDevicePath(
-  IN  EFI_DEVICE_PATH_PROTOCOL  *FilePath
-  )
-{
-
-  EFI_DEVICE_PATH_PROTOCOL  *Node;
-
-  for (Node = FilePath; !IsDevicePathEnd(Node); Node = 
NextDevicePathNode(Node))
-  {
-if ((DevicePathType (Node) == MEDIA_DEVICE_PATH)  (DevicePathSubType 
(Node) == MEDIA_PIWG_FW_FILE_DP)) {
-  if (!CompareMem(PcdGetPtr(PcdShellFile), 
(((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)Node)-FvFileName), sizeof(EFI_GUID)))
- return TRUE;
-}
-  }
-  return FALSE;
-}
 
 /**
   Create a list of boot option from global BootOptionMenu. It
   allow user to delete the boot option.
 

Re: [edk2] today's US CERT UEFI advisory

2015-07-30 Thread Laszlo Ersek
On 07/30/15 20:09, Andrew Fish wrote:
 
 On Jul 30, 2015, at 9:58 AM, Laszlo Ersek ler...@redhat.com
 mailto:ler...@redhat.com wrote:

 On 07/30/15 17:49, Blibbet wrote:
 FYI, in case any OEM's missed today's US-CERT UEFI vulnerability notice:

 http://firmwaresecurity.com/2015/07/30/us-cert-bios-vulnerability-note-vu577140/

 Remember that any TianoCore-based bugs may be in your platorm:

 https://twitter.com/XenoKovah/status/623483244890189824

 Can anyone clarify if the code in this vulnerability is in TianoCore or
 external vendor-specific?
 
 I don’t think that any platform on TianoCore are “secure” in that the
 ROMs are not signed, and only signed ROM images can be placed in the
 ROM. That is a “platform feature”. 
 
 Locking the ROM is chipset/platform code in general. 
 
 Italian cyber-security firm Hacking Team, allegedly, sells a UEFI root
 kit. This root kit reads the ROM, patches in extra drivers (NTFS driver,
 malware, etc.), and flash updates the ROM. It actually uses an open
 source tool that exists to let folks “mod” their BIOS. It requires
 physical access to the machine. 
 
 
 I think both sides, the firmware researcher side, and the firmware
 vendor side, have ample room for improvement in their behavior.

 The researcher side should tone down their repulsive sensationalism,
 selling each security bug to the public as the end of the world, and
 showing off themselves as the most leet security startup ever, seeking
 to score hefty $$$ gigs right after. Responsible disclosure exists.

 
 +1
 
 The vendor side should get their act together, and react to, and
 *address*, responsible disclosures in a *timely* manner. Among other
 things, this requires:

 - spelling out CVE numbers in the subject lines of edk2 commit
  messages, when edk2 is affected and some patches address those issues,

 - publicly release *plain text* advisories, rather than privately
  circulated PDF files that contain the problem description as embedded
  image files, with (probable) watermarks embedded as well.

 Edk2's track record has been absolutely deplorable in this regard. But,
 as I said, both sides have a lot of room for improvement; the hype
 generated by some white hats around each single discovery is hugely
 childish and unprofessional too.

 
 I’m not sure that the commit messages is the best way to communicate the
 info, and something could get fixed prior CVE going out.

Usually when a vulnerability is disclosed responsibly, a CVE is
requested, and then participants coordinate the CVE embargo date. This
should give enough time to all participant vendors to fix their
products, but not delay the public advisory for too long (eg. more than
one or two weeks).

Some participants will get there earlier, but they should nonetheless
hold their updates / releases / upstream commits until the embargo is
lifted, on the agreed upon date. Otherwise the embargo is meaningless --
if just one party releases an update (or there is an edk2 commit fixing
the issue, with as obscure a commit message as possible), malicious
attackers that are always watching but have not been previously aware of
this specific vulnerability will immediately deduce the full details of
the issue, and exploit the other (as yet unpatched) systems.

The embargo is a tradeoff between fix the bug as early as possible and
give all participants some time to fix it in a coordinated way, even if
that means some 'unnecessary' delay for some parties in the circle.

Therefore fixes for such issues *must not* be pushed to a public repo,
nor publicly discussed, before the respective embargo is over. Once the
embargo is over, the updates and the advisories are pushed / released as
quickly as possible.

Regarding the commit messages: for many system administrators the only
(or the safest) way to ensure that an update contains a fix for a CVE is
to scan the changelog / commit log, and search for the CVE identifier.
The black hats absolutely don't need this help to pinpoint holes from
commit histories, but sysadmins (and developers outside the privileged,
coordinated circle) certainly need the help for identifying fixes. In
other words, obscurity only hurts the good guy.

 I agree it
 would be nice to have an easier way to cross reference this info. I’m
 going to ping some UEFI Forum folks about this.

Thank you!
Laszlo

 
 Thanks,
 
 Andrew Fish
 
 PS. If folks want to brush up on edk2/UEFI security related topics,
 there has been a lot of security related presentations at UEFI Plugfests
 that are archived
 here: http://www.uefi.org/learning_center/presentationsandvideos
 
 
 

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


Re: [edk2] more code sharing joy between OvmfPkg and ArmVirtPkg, re SMBIOS

2015-07-30 Thread Jordan Justen
On 2015-07-30 10:09:34, Laszlo Ersek wrote:
 (Sigh, I left off the list address. This should be discussed publicly.
 Resending.)
 
 Clearly, the SMBIOS patches I posted and got committed last time are not
 good enough. That's because the SMBIOS 3.0 entry point is structurally
 different from the prior versions (because why not). Therefore, now that
 Wei has QEMU generate SMBIOS 3.0 payload for the firmware, the checks we
 currently have in place, fail.
 
 (First and foremost, the *size* of the 3.0 entry point is different from
 that of the 2.8 one. We correctly catch this, but we incorrectly don't
 recognize 3.0.)
 
 I can write the code for telling apart 2.8 from 3.0; that's not the
 problem. The problem is that this code would have to be triplicated, as
 things currently stand:
 
 (1) ArmVirtPkg/QemuFwCfgToPcdDxe/QemuFwCfgToPcd.c sets
 PcdSmbiosVersion for MdeModulePkg/Universal/SmbiosDxe in the
 ArmVirtPkg build, early in the DXE phase,
 
 (2) OvmfPkg/PlatformPei/Platform.c sets the same PCD for the same
 generic driver in the OvmfPkg build, in the PEI phase,
 
 (3) OvmfPkg/SmbiosPlatformDxe/Qemu.c verifies the entry point again,
 and fetches the actual tables. (Note that
 (TablesSize != QemuAnchor.TableLength) is checked too, so there is a
 cross-dependency between the two blobs, the entry point and the
 actual tables.)
 
 Now, the current triplication we have is simplistic, so it shouldn't be
 a large problem. But, if I need to add smarts for detecting v2 versus
 v3, I would hate to triplicate that logic too. This is what code sites
 (1) and (2) both should do, for determining PcdSmbiosVersion:
 
 - Find etc/smbios/smbios-anchor. If it is missing, bail.
 - Compare the size of that fw-cfg blob against SMBIOS_TABLE_ENTRY_POINT.
 - If it is a match, perform the current _SM_ and _DMI_ checks.
 - If they fail, bail.
 - If they match, check that QemuAnchor.MajorVersion is actually 2.
 - If that fails, bail.
 - If it matches, set PcdSmbiosVersion to 0x02__.
 - Otherwise, compare the size of the anchor blob against
   SMBIOS_TABLE_3_0_ENTRY_POINT.
 - If it is a match, perform the (new) _SM3_ check.
 - If it fails, bail.
 - Check that MajorVersion (which now lives at a different offset) is
   3.
 - If that fails, bail.
 - If it matches, set PcdSmbiosVersion to 0x03__.
 - Otherwise, bail.
 
 In (3):
 - repeat all of the above, except do not *set* the PCD -- enforce it
   instead. ASSERT() that the PCD already has the right value (because
   at this point MdeModulePkg/Universal/SmbiosDxe is expected to have
   been dispatched, and consumed the PCD).
 - If the version is 2, then compare TableLength == tables blob size.
   If it fails, return NULL; otherwise return the tables.
 - If the version is 3, then compare TableMaximumSize = tables blob
   size. If it fails, return NULL; otherwise return the tables.
 - If the version is something different, return NULL.
 
 As I said, this can be coded as-written, but I strongly dislike the code
 triplication, and I expect you guys will too. So what do you propose? I
 could extract a tiny BASE library (a single function) that:
 - depends on QemuFwCfgLib
 - takes a BOOLEAN parameter that tells it whether to set the PCD, or
   check and enforce the PCD
 - return the detected SMBIOS version (2, 3, or missing/unknown)
 
 Then I could use this function in sites (1) and (2) for setting the PCD,
 and then use it in (3) for enforcing the PCD, and based on the returned
 SMBIOS version, perform the version-specific tables blob size check too.

Would it help if you used a NULL library with a constructor to set
gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion?

  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
LibraryClasses
  NULL|OvmfPkg/Library/SmbiosVersionLib/DetectSmbiosVersionLib.inf
  }

I'm not sure about the enforce part, but maybe this could allow you to
also share some code between detect  enforce:

  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf {
LibraryClasses
  NULL|OvmfPkg/Library/SmbiosVersionLib/ValidateSmbiosVersionLib.inf
  }

But, I'm a little skeptical of the necessity of this second one...

-Jordan

 Small problem: you are going to hate me for introducing a
 single-function library *class*.
 
 So how do I share *one function* between different *phase* modules of
 different top-level packages? Should we introduce BaseCrapLib ^W
 BaseUtilityLib, and just keep throwing such functions in there?
 
 If so, what names (pathnames, basenames, library class and instance
 names etc) do you recommend?
 
 Thanks
 Laszlo
 ___
 edk2-devel mailing list
 edk2-devel@lists.01.org
 https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 0/4] StdLib: Add ARM SoftFloat AArch64 supports

2015-07-30 Thread Laszlo Ersek
On 07/30/15 04:21, Daryl McDaniel wrote:
 Now that we have received legal approval of the LLVM / University of
 Illinois license, I can say that this set of patches looks OK to me.
 0.  [PATCH 0/4] StdLib: Add ARM SoftFloat  AArch64 supports
 1.  [PATCH 1/4] StdLib: Added BaseStackLib for ARM architectures
 2.  [PATCH 2/4] StdLib/LibC: Add software floating point library from
 NetBSD
 3.  [PATCH 3/4] StdLib/LibC: Provide missing ARM symbols
 4.  [PATCH 4/4] StdLib: Add support for AArch64
 
 Reviewed by: Daryl McDaniel edk2-li...@mc2research.org
 
 Daryl McDaniel

Committed to SVN as revs 18115..18118, inclusive.

Thanks
Laszlo

 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
 Olivier Martin
 Sent: Thursday, July 16, 2015 6:52 AM
 To: daryl.mcdan...@intel.com
 Cc: bill.fletc...@linaro.org; edk2-de...@ml01.01.org;
 edk2-de...@lists.sourceforge.net; Olivier Martin olivier.mar...@arm.com
 Subject: [edk2] [PATCH 0/4] StdLib: Add ARM SoftFloat  AArch64 supports
 
 This patchset adds support for ARM SoftFloat. ARM Hardware floating point is
 disabled when building UEFI Firmware.
 Software Floating point is required to get full StdLib support.
 
 This support also adds AArch64 support.
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Reviewed-by: Olivier Martin olivier.mar...@arm.com
 
 Brendan Jackman (1):
   StdLib: Add support for AArch64
 
 Harry Liebel (2):
   StdLib/LibC: Add software floating point library from NetBSD
   StdLib/LibC: Provide missing ARM symbols
 
 Olivier Martin (1):
   StdLib: Added BaseStackLib for ARM architectures
 
  StdLib/Include/Aarch64/arm-gcc.h   |  110 +
  StdLib/Include/Aarch64/machine/ansi.h  |  106 +
  StdLib/Include/Aarch64/machine/bswap.h |   13 +
  StdLib/Include/Aarch64/machine/byte_swap.h |   63 +
  StdLib/Include/Aarch64/machine/endian.h|3 +
  StdLib/Include/Aarch64/machine/endian_machdep.h|3 +
  StdLib/Include/Aarch64/machine/fenv.h  |   39 +
  StdLib/Include/Aarch64/machine/float.h |   59 +
  StdLib/Include/Aarch64/machine/ieee.h  |   31 +
  StdLib/Include/Aarch64/machine/ieeefp.h|   45 +
  StdLib/Include/Aarch64/machine/int_const.h |   63 +
  StdLib/Include/Aarch64/machine/int_limits.h|  127 +
  StdLib/Include/Aarch64/machine/int_mwgwtypes.h |   82 +
  StdLib/Include/Aarch64/machine/int_types.h |   61 +
  StdLib/Include/Aarch64/machine/limits.h|  100 +
  StdLib/Include/Aarch64/machine/math.h  |3 +
  StdLib/Include/Aarch64/machine/param.h |  124 +
  StdLib/Include/Aarch64/machine/signal.h|   22 +
  StdLib/Include/Aarch64/machine/types.h |   82 +
  StdLib/Include/Aarch64/milieu.h|   52 +
  StdLib/Include/Aarch64/softfloat.h |  316 ++
  StdLib/Include/Arm/arm-gcc.h   |  114 +
  StdLib/Include/Arm/machine/fenv.h  |   55 +
  StdLib/Include/Arm/machine/ieeefp.h|   58 +
  StdLib/Include/Arm/milieu.h|   38 +
  StdLib/Include/Arm/softfloat.h |  316 ++
  StdLib/Include/ieeefp.h|   46 +
  StdLib/LibC/LibC.inf   |5 +
  StdLib/LibC/Main/Arm/fixunsdfsi.c  |   74 +
  StdLib/LibC/Main/Arm/floatunsidf.c |   71 +
  StdLib/LibC/Main/Arm/fp_lib.h  |  282 +
  StdLib/LibC/Main/Arm/int_endianness.h  |   71 +
  StdLib/LibC/Main/Arm/int_lib.h |  105 +
  StdLib/LibC/Main/Arm/int_types.h   |  170 +
  StdLib/LibC/Main/Arm/int_util.h|   68 +
  StdLib/LibC/Softfloat/Arm/__aeabi_dcmpeq.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_dcmpge.c |   35 +
  StdLib/LibC/Softfloat/Arm/__aeabi_dcmpgt.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_dcmple.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_dcmplt.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_dcmpun.c |   42 +
  StdLib/LibC/Softfloat/Arm/__aeabi_fcmpeq.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_fcmpge.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_fcmpgt.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_fcmple.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_fcmplt.c |   37 +
  StdLib/LibC/Softfloat/Arm/__aeabi_fcmpun.c |   42 +
  StdLib/LibC/Softfloat/Makefile.inc |   42 +
  StdLib/LibC/Softfloat/README.NetBSD|8 +
  StdLib/LibC/Softfloat/README.txt   |   39 +
  StdLib/LibC/Softfloat/Softfloat.inf|   74 +
  StdLib/LibC/Softfloat/bits32/softfloat-macros  |  648 +++
  StdLib/LibC/Softfloat/bits32/softfloat.c   | 2355 
  StdLib/LibC/Softfloat/bits64/softfloat-macros  |  

Re: [edk2] [PATCH v2] MdeModulePkg PeiCore: Add PCD to specify PEIM Shadow

2015-07-30 Thread Zeng, Star
Reviewed-by: Star Zeng star.z...@intel.com

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Thursday, July 30, 2015 5:40 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2] MdeModulePkg PeiCore: Add PCD to specify PEIM Shadow

v2 changelog:
Check CurrentPeimHandle to check the matched PeimHandle.
Add check point to ShadowPeiCore based on PCD.

v1 changelog:
PeiCore LoadImage always shadow itself and PEIM on normal boot after the 
physical memory is installed. On the emulator platform, the shadow may be not 
necessary. To support such usage, new PCD PcdShadowPeimOnBoot is introduced to 
specify whether loads PEIM in memory by default.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao liming@intel.com
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c |  21 +---
 MdeModulePkg/Core/Pei/Image/Image.c   |  33 ++
 MdeModulePkg/Core/Pei/PeiMain.inf |   1 +
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |   6 -
 MdeModulePkg/MdeModulePkg.dec |   7 ++
 MdeModulePkg/MdeModulePkg.uni | Bin 166792 - 168226 bytes
 6 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 3a85502..46e990d 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -695,10 +695,13 @@ PeiDispatcher (
 
 for (Index1 = 0; Index1 = SaveCurrentFvCount; Index1++) {
   for (Index2 = 0; (Index2  PcdGet32 (PcdPeiCoreMaxPeimPerFv))  
(Private-Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
 if (Private-Fv[Index1].PeimState[Index2] == 
PEIM_STATE_REGISITER_FOR_SHADOW) {
   PeimFileHandle = Private-Fv[Index1].FvFileHandles[Index2];
+  Private-CurrentFileHandle   = PeimFileHandle;
+  Private-CurrentPeimFvCount  = Index1;
+  Private-CurrentPeimCount= Index2;
   Status = PeiLoadImage (
 (CONST EFI_PEI_SERVICES **) Private-Ps,
 PeimFileHandle,
 PEIM_STATE_REGISITER_FOR_SHADOW,
 EntryPoint,
@@ -707,13 +710,10 @@ PeiDispatcher (
   if (Status == EFI_SUCCESS) {
 //
 // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
 //
 Private-Fv[Index1].PeimState[Index2]++;
-Private-CurrentFileHandle   = PeimFileHandle;
-Private-CurrentPeimFvCount  = Index1;
-Private-CurrentPeimCount= Index2;
 //
 // Call the PEIM entry point
 //
 PeimEntryPoint = (EFI_PEIM_ENTRY_POINT2)(UINTN)EntryPoint;
 
@@ -1106,10 +1106,25 @@ PeiDispatcher (
   //
   // If memory is availble we shadow images by default for 
performance reasons.
   // We call the entry point a 2nd time so the module knows it's 
shadowed.
   //
   //PERF_START (PeiServices, LPEIM, PeimFileHandle, 0);
+  if ((Private-HobList.HandoffInformationTable-BootMode != 
BOOT_ON_S3_RESUME)  !PcdGetBool (PcdShadowPeimOnBoot)) {
+//
+// Load PEIM into Memory for Register for shadow PEIM.
+//
+Status = PeiLoadImage (
+   PeiServices,
+   PeimFileHandle,
+   PEIM_STATE_REGISITER_FOR_SHADOW,
+   EntryPoint,
+   AuthenticationState
+   );
+if (Status == EFI_SUCCESS) {
+  PeimEntryPoint = (EFI_PEIM_ENTRY_POINT2)(UINTN)EntryPoint;
+}
+  }
   ASSERT (PeimEntryPoint != NULL);
   PeimEntryPoint (PeimFileHandle, (const EFI_PEI_SERVICES **) 
PeiServices);
   //PERF_END (PeiServices, LPEIM, PeimFileHandle, 0);
 
   //
diff --git a/MdeModulePkg/Core/Pei/Image/Image.c 
b/MdeModulePkg/Core/Pei/Image/Image.c
index cab08fe..9c54192 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -1,9 +1,9 @@
 /** @file
   Pei Core Load Image Support
 
-Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.BR
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.BR
 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
 
@@ -115,11 +115,12 @@ GetImageReadFunction (
   PEI_CORE_INSTANCE  *Private;
   VOID*  MemoryBuffer;
 
   Private = PEI_CORE_INSTANCE_FROM_PS_THIS (GetPeiServicesTablePointer ());
   
-  if (Private-PeiMemoryInstalled   

Re: [edk2] posting to the new list without subscription?

2015-07-30 Thread Laszlo Ersek
On 07/29/15 23:38, Jordan Justen wrote:
 On 2015-07-28 12:59:46, Laszlo Ersek wrote:
 Hi Jordan,

 what are the rules for posting to the new list without being
 subscribed?
 
 On the old list, I think we dropped all such emails.
 
 Currently we are rejecting them on the new list.
 
 Paolo posted a few absolutely great comments recently, and I can only
 see them in my INBOX, and not on the list.
 
 Strangely, it seems your email did not hit the list, although you are
 subscribed.

Indeed, I noticed that. I didn't understood. :)

 I've seen plenty of emails from you on the new list, so I
 think generally your emails are making it to the list.

Generally, yes.

 Are such messages held for administrator approval? (Per message
 basis? Per poster basis?) I think the most collaborative setting
 would be to allow unsubscribed people to post. But, I guess, if the
 spam situation is bad, permanent per-poster approval should be okay
 too (I think that's what libvir-list follows too, for example.)
 Per-message approval would be bad, IMO.
 
 Hmm. I'll look into the per-poster approval.

Thanks!

 I think the only way we could consider wide-open posting is if we
 dropped all email with non-plain text attachments. That would likely
 rule out most spam emails. But, we'd have to train the community that
 only plain text emails are going to make it through to the list.
 
 I know many projects are able to get away with wide-open posting, but
 I don't know how they manage to keep spam out.

I think per-poster approval should work. If I remember correctly (I'm
not sure I do), on libvir-list (or elsewhere?) when you post your first
message as a non-subscriber, you get an automatic email about your
message being held until administrator approval. So it never feels like
a black hole. And, once the originator email address is approved
permanently, posting becomes transparent.

Thanks!
Laszlo

 
 -Jordan
 

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


Re: [edk2] posting to the new list without subscription?

2015-07-30 Thread David Woodhouse
On Wed, 2015-07-29 at 14:38 -0700, Jordan Justen wrote:
 On 2015-07-28 12:59:46, Laszlo Ersek wrote:
  Hi Jordan,
  
  what are the rules for posting to the new list without being
  subscribed?
 
 On the old list, I think we dropped all such emails.
 
 Currently we are rejecting them on the new list.

Please don't. It's important for technical mailing lists to allow
*collaboration* between projects.

For example I deliberately cross-posted to this list *and* the openssl
-dev list quite recently. Anyone who is only subscribed to the latter
list would be barred from posting to this list, and the conversation
would end up split into two threads.

(Actually, the OpenSSL list also has one of those idiotic Reply-To:
headers which tricks private replies to go back to the list,
encouraging people to press the wrong 'reply' button, so anyone
replying from that list might not have even *sent* their message here,
but that's a separate problem and *not* one that you've inflicted on
this list, thankfully.)

  Paolo posted a few absolutely great comments recently, and I can only
  see them in my INBOX, and not on the list.
 
 Strangely, it seems your email did not hit the list, although you are
 subscribed. I've seen plenty of emails from you on the new list, so I
 think generally your emails are making it to the list.

Note that Mailman has a bizarre default of *not* sending all list
traffic to subscribers. If your subscribed address appears in the To:
or Cc: headers, it *won't* send you a copy. So you end up with only the
copy in your inbox, and *not* the copy in your mailing list folder.

Could that be what you're experiencing?

At the bottom of https://lists.01.org/mailman/options/edk2-devel you
can find the 'Avoid duplicate copies of messages?' option, which
thankfully you can disable for *all* lists on the server at once. Which
almost but not quite makes up for it stupidly being enabled by default
for new subscribers.

There's also an option for whether you want to receive your own posts
to the list, but that one *does* default to sending them, I believe.

  Are such messages held for administrator approval? (Per message
  basis? Per poster basis?) I think the most collaborative setting
  would be to allow unsubscribed people to post. But, I guess, if the
  spam situation is bad, permanent per-poster approval should be okay
  too (I think that's what libvir-list follows too, for example.)
  Per-message approval would be bad, IMO.
 
 Hmm. I'll look into the per-poster approval.
 
 I think the only way we could consider wide-open posting is if we
 dropped all email with non-plain text attachments. That would likely
 rule out most spam emails. But, we'd have to train the community that
 only plain text emails are going to make it through to the list.

That works. The trick is to ensure it's bounced directly and
immediately, not just goes into a moderation black hole.

For the mailing lists I run on lists.infradead.org, you get an
immediate bounce if you attempt to send HTML or other crap to the list.
Which keeps most of the spam out, *and* lets senders know what they did
wrong. Otherwise, they might assume their message got through... or
*will* get through after moderation.

 I know many projects are able to get away with wide-open posting, but
 I don't know how they manage to keep spam out.

A per-sender approval works, if the list moderation is done quickly
enough. But mostly, with a decent spam setup and content type filters,
it isn't that much of an issue.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation


smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] UiApp: Move reset menu from frontpage to BMM page.

2015-07-30 Thread Ni, Ruiyu
Reviewed-by: Ruiyu Ni ruiyu...@intel.com

 -Original Message-
 From: Dong, Eric
 Sent: Thursday, July 30, 2015 4:56 PM
 To: Ni, Ruiyu ruiyu...@intel.com; edk2-devel@lists.01.org
 Subject: [Patch] UiApp: Move reset menu from frontpage to BMM page.
 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Eric Dong eric.d...@intel.com
 ---
  MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr  | 8 
  MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c | 4 
  MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h  | 1 +
  MdeModulePkg/Application/UiApp/FrontPage.c   | 8 
  MdeModulePkg/Application/UiApp/FrontPage.h   | 1 -
  5 files changed, 13 insertions(+), 9 deletions(-)
 
 diff --git a/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
 b/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
 index 7247bbe..9247412 100644
 --- a/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
 +++ b/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
 @@ -75,10 +75,18 @@ formset
  goto FORM_TIME_OUT_ID,
   prompt = STRING_TOKEN(STR_FORM_TIME_OUT_TITLE),
   help = STRING_TOKEN(STR_FORM_TIME_OUT_HELP),
   flags = INTERACTIVE,
   key = FORM_TIME_OUT_ID;
 +
 +subtitle text = STRING_TOKEN(STR_NULL_STRING);
 +
 +text
 + help   = STRING_TOKEN(STR_RESET),
 + text   = STRING_TOKEN(STR_RESET),
 + flags  = INTERACTIVE,
 + key= FORM_RESET;
 
  label LABEL_BMM_PLATFORM_INFORMATION;
  //
  // This is where we will dynamically add a Action type op-code to show
  // the platform information.
 diff --git a/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
 b/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
 index aec6b85..4f46ed6 100644
 --- a/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
 +++ b/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
 @@ -600,10 +600,14 @@ BootMaintCallback (
// since we have already applied or discarded.
//
*ActionRequest =
 EFI_BROWSER_ACTION_REQUEST_FORM_SUBMIT_EXIT;
 
break;
 +
 +case FORM_RESET:
 +  gRT-ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
 +  return EFI_UNSUPPORTED;
 
  default:
break;
  }
}
 diff --git a/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
 b/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
 index 4b1efb7..2126b3d 100644
 --- a/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
 +++ b/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
 @@ -67,10 +67,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
 ANY KIND, EITHER EXPRESS OR IMPLIED.
  #define KEY_VALUE_DRIVER_ADD_DESC_DATA   0x1109
  #define KEY_VALUE_DRIVER_ADD_OPT_DATA0x110A
  #define KEY_VALUE_SAVE_AND_EXIT  0x110B
  #define KEY_VALUE_NO_SAVE_AND_EXIT   0x110C
  #define KEY_VALUE_BOOT_FROM_FILE 0x110D
 +#define FORM_RESET   0x110E
 
  #define MAXIMUM_NORMAL_KEY_VALUE 0x11FF
 
  //
  // Varstore ID defined for Buffer Storage
 diff --git a/MdeModulePkg/Application/UiApp/FrontPage.c
 b/MdeModulePkg/Application/UiApp/FrontPage.c
 index 4e80d7c..7d1cf2a 100644
 --- a/MdeModulePkg/Application/UiApp/FrontPage.c
 +++ b/MdeModulePkg/Application/UiApp/FrontPage.c
 @@ -447,18 +447,10 @@ FrontPageCallback (
//Current language of platform is changed,recreate oneof options
 for language.
//
InitializeLanguage();
break;
 
 -
 -case FRONT_PAGE_KEY_RESET:
 -  //
 -  // Reset
 -  //
 -  gRT-ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
 -  return EFI_UNSUPPORTED;
 -
  default:
break;
  }
} else if (Action == EFI_BROWSER_ACTION_CHANGING) {
  if (Value == NULL) {
 diff --git a/MdeModulePkg/Application/UiApp/FrontPage.h
 b/MdeModulePkg/Application/UiApp/FrontPage.h
 index 08d1692..c5d4c42 100644
 --- a/MdeModulePkg/Application/UiApp/FrontPage.h
 +++ b/MdeModulePkg/Application/UiApp/FrontPage.h
 @@ -54,11 +54,10 @@ extern BOOLEAN  gConnectAllHappened;
  #define FRONT_PAGE_KEY_CONTINUE0x1000
  #define FRONT_PAGE_KEY_LANGUAGE0x1234
  #define FRONT_PAGE_KEY_BOOT_MANAGER0x1064
  #define FRONT_PAGE_KEY_DEVICE_MANAGER  0x8567
  #define FRONT_PAGE_KEY_BOOT_MAINTAIN   0x9876
 -#define FRONT_PAGE_KEY_RESET   0X7654
 
  #define LABEL_SELECT_LANGUAGE  0x1000
  #define LABEL_PLATFORM_INFORMATION 0x1001
  #define LABEL_END  0x
 
 --
 1.9.5.msysgit.1

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


[edk2] [PATCH v2] MdeModulePkg PeiCore: Add PCD to specify PEIM Shadow

2015-07-30 Thread Liming Gao
v2 changelog:
Check CurrentPeimHandle to check the matched PeimHandle.
Add check point to ShadowPeiCore based on PCD.

v1 changelog:
PeiCore LoadImage always shadow itself and PEIM on normal boot after
the physical memory is installed. On the emulator platform, the shadow
may be not necessary. To support such usage, new PCD PcdShadowPeimOnBoot
is introduced to specify whether loads PEIM in memory by default.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao liming@intel.com
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c |  21 +---
 MdeModulePkg/Core/Pei/Image/Image.c   |  33 ++
 MdeModulePkg/Core/Pei/PeiMain.inf |   1 +
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |   6 -
 MdeModulePkg/MdeModulePkg.dec |   7 ++
 MdeModulePkg/MdeModulePkg.uni | Bin 166792 - 168226 bytes
 6 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 3a85502..46e990d 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -695,10 +695,13 @@ PeiDispatcher (
 
 for (Index1 = 0; Index1 = SaveCurrentFvCount; Index1++) {
   for (Index2 = 0; (Index2  PcdGet32 (PcdPeiCoreMaxPeimPerFv))  
(Private-Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
 if (Private-Fv[Index1].PeimState[Index2] == 
PEIM_STATE_REGISITER_FOR_SHADOW) {
   PeimFileHandle = Private-Fv[Index1].FvFileHandles[Index2];
+  Private-CurrentFileHandle   = PeimFileHandle;
+  Private-CurrentPeimFvCount  = Index1;
+  Private-CurrentPeimCount= Index2;
   Status = PeiLoadImage (
 (CONST EFI_PEI_SERVICES **) Private-Ps,
 PeimFileHandle,
 PEIM_STATE_REGISITER_FOR_SHADOW,
 EntryPoint,
@@ -707,13 +710,10 @@ PeiDispatcher (
   if (Status == EFI_SUCCESS) {
 //
 // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
 //
 Private-Fv[Index1].PeimState[Index2]++;
-Private-CurrentFileHandle   = PeimFileHandle;
-Private-CurrentPeimFvCount  = Index1;
-Private-CurrentPeimCount= Index2;
 //
 // Call the PEIM entry point
 //
 PeimEntryPoint = (EFI_PEIM_ENTRY_POINT2)(UINTN)EntryPoint;
 
@@ -1106,10 +1106,25 @@ PeiDispatcher (
   //
   // If memory is availble we shadow images by default for 
performance reasons.
   // We call the entry point a 2nd time so the module knows it's 
shadowed.
   //
   //PERF_START (PeiServices, LPEIM, PeimFileHandle, 0);
+  if ((Private-HobList.HandoffInformationTable-BootMode != 
BOOT_ON_S3_RESUME)  !PcdGetBool (PcdShadowPeimOnBoot)) {
+//
+// Load PEIM into Memory for Register for shadow PEIM.
+//
+Status = PeiLoadImage (
+   PeiServices,
+   PeimFileHandle,
+   PEIM_STATE_REGISITER_FOR_SHADOW,
+   EntryPoint,
+   AuthenticationState
+   );
+if (Status == EFI_SUCCESS) {
+  PeimEntryPoint = (EFI_PEIM_ENTRY_POINT2)(UINTN)EntryPoint;
+}
+  }
   ASSERT (PeimEntryPoint != NULL);
   PeimEntryPoint (PeimFileHandle, (const EFI_PEI_SERVICES **) 
PeiServices);
   //PERF_END (PeiServices, LPEIM, PeimFileHandle, 0);
 
   //
diff --git a/MdeModulePkg/Core/Pei/Image/Image.c 
b/MdeModulePkg/Core/Pei/Image/Image.c
index cab08fe..9c54192 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -1,9 +1,9 @@
 /** @file
   Pei Core Load Image Support
 
-Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.BR
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.BR
 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
 
@@ -115,11 +115,12 @@ GetImageReadFunction (
   PEI_CORE_INSTANCE  *Private;
   VOID*  MemoryBuffer;
 
   Private = PEI_CORE_INSTANCE_FROM_PS_THIS (GetPeiServicesTablePointer ());
   
-  if (Private-PeiMemoryInstalled   
((Private-HobList.HandoffInformationTable-BootMode != BOOT_ON_S3_RESUME) || 
PcdGetBool (PcdShadowPeimOnS3Boot))  
+  if (Private-PeiMemoryInstalled   
(((Private-HobList.HandoffInformationTable-BootMode != BOOT_ON_S3_RESUME)  
PcdGetBool (PcdShadowPeimOnBoot)) || 
+  

[edk2] [Patch] UiApp: Move reset menu from frontpage to BMM page.

2015-07-30 Thread Eric Dong
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong eric.d...@intel.com
---
 MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr  | 8 
 MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c | 4 
 MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h  | 1 +
 MdeModulePkg/Application/UiApp/FrontPage.c   | 8 
 MdeModulePkg/Application/UiApp/FrontPage.h   | 1 -
 5 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr 
b/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
index 7247bbe..9247412 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
+++ b/MdeModulePkg/Application/UiApp/BootMaint/Bm.vfr
@@ -75,10 +75,18 @@ formset
 goto FORM_TIME_OUT_ID,
  prompt = STRING_TOKEN(STR_FORM_TIME_OUT_TITLE),
  help = STRING_TOKEN(STR_FORM_TIME_OUT_HELP),
  flags = INTERACTIVE,
  key = FORM_TIME_OUT_ID;
+
+subtitle text = STRING_TOKEN(STR_NULL_STRING);
+
+text
+ help   = STRING_TOKEN(STR_RESET),
+ text   = STRING_TOKEN(STR_RESET),
+ flags  = INTERACTIVE,
+ key= FORM_RESET;
  
 label LABEL_BMM_PLATFORM_INFORMATION;
 //
 // This is where we will dynamically add a Action type op-code to show 
 // the platform information.
diff --git a/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c 
b/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
index aec6b85..4f46ed6 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
+++ b/MdeModulePkg/Application/UiApp/BootMaint/BootMaint.c
@@ -600,10 +600,14 @@ BootMaintCallback (
   // since we have already applied or discarded.
   //
   *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_SUBMIT_EXIT;
 
   break;
+ 
+case FORM_RESET:
+  gRT-ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
+  return EFI_UNSUPPORTED;
 
 default:
   break;
 }
   }
diff --git a/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h 
b/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
index 4b1efb7..2126b3d 100644
--- a/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
+++ b/MdeModulePkg/Application/UiApp/BootMaint/FormGuid.h
@@ -67,10 +67,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define KEY_VALUE_DRIVER_ADD_DESC_DATA   0x1109
 #define KEY_VALUE_DRIVER_ADD_OPT_DATA0x110A
 #define KEY_VALUE_SAVE_AND_EXIT  0x110B
 #define KEY_VALUE_NO_SAVE_AND_EXIT   0x110C
 #define KEY_VALUE_BOOT_FROM_FILE 0x110D
+#define FORM_RESET   0x110E
 
 #define MAXIMUM_NORMAL_KEY_VALUE 0x11FF
 
 //
 // Varstore ID defined for Buffer Storage
diff --git a/MdeModulePkg/Application/UiApp/FrontPage.c 
b/MdeModulePkg/Application/UiApp/FrontPage.c
index 4e80d7c..7d1cf2a 100644
--- a/MdeModulePkg/Application/UiApp/FrontPage.c
+++ b/MdeModulePkg/Application/UiApp/FrontPage.c
@@ -447,18 +447,10 @@ FrontPageCallback (
   //Current language of platform is changed,recreate oneof options for 
language.
   //
   InitializeLanguage();
   break;
 
-
-case FRONT_PAGE_KEY_RESET:
-  //
-  // Reset
-  //
-  gRT-ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
-  return EFI_UNSUPPORTED;
-
 default:
   break;
 }
   } else if (Action == EFI_BROWSER_ACTION_CHANGING) {
 if (Value == NULL) {
diff --git a/MdeModulePkg/Application/UiApp/FrontPage.h 
b/MdeModulePkg/Application/UiApp/FrontPage.h
index 08d1692..c5d4c42 100644
--- a/MdeModulePkg/Application/UiApp/FrontPage.h
+++ b/MdeModulePkg/Application/UiApp/FrontPage.h
@@ -54,11 +54,10 @@ extern BOOLEAN  gConnectAllHappened;
 #define FRONT_PAGE_KEY_CONTINUE0x1000
 #define FRONT_PAGE_KEY_LANGUAGE0x1234
 #define FRONT_PAGE_KEY_BOOT_MANAGER0x1064
 #define FRONT_PAGE_KEY_DEVICE_MANAGER  0x8567
 #define FRONT_PAGE_KEY_BOOT_MAINTAIN   0x9876
-#define FRONT_PAGE_KEY_RESET   0X7654
 
 #define LABEL_SELECT_LANGUAGE  0x1000
 #define LABEL_PLATFORM_INFORMATION 0x1001
 #define LABEL_END  0x
 
-- 
1.9.5.msysgit.1

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