date:20150730

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] [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

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

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

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

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.

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 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] [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] [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] 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 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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[edk2] today's US CERT UEFI advisory

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

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

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

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

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

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

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

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

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

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

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

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

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

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

34 matches

Site Navigation

Mail list logo

Footer information