[edk2] [PATCH] MdePkg: Refine UefiFileHandleLib to avoid write non-ASCII char into ASCII file.

2015-09-08 Thread Qiu Shumin 
Cc: Jaben Carsey 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c 
b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
index 04a2f18..dfec5fa 100644
--- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
+++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
@@ -1079,6 +1079,7 @@ FileHandleWriteLine(
   EFI_STATUS  Status;
   CHAR16  CharBuffer;
   UINTN   Size;
+  UINTN   Index;
   UINTN   CharSize;
   UINT64  FileSize;
   UINT64  OriginalFilePosition;
@@ -1136,6 +1137,12 @@ FileHandleWriteLine(
   return EFI_OUT_OF_RESOURCES;
 }
 UnicodeStrToAsciiStr (Buffer, AsciiBuffer);
+for (Index = 0; Index < Size; Index++) {
+  if (!((AsciiBuffer[Index] >= 0) && (AsciiBuffer[Index] < 128))){
+FreePool(AsciiBuffer);
+return EFI_INVALID_PARAMETER;
+  }
+}
 
 Size = AsciiStrSize(AsciiBuffer) - sizeof(CHAR8);
 Status = FileHandleWrite(Handle, , AsciiBuffer);
-- 
1.9.5.msysgit.1

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

Re: [edk2] Help debugging PEIM on Minnowboard Max

2015-09-08 Thread Tian, Feng 
Hi, Eric

>From XHCI spec view, there is no data buffer alignment requirement for control 
>transfer (you can refer to XHCI1.0 section 6.4.1.2.2 Table 72).

So I don't know why you found 16 byte alignment issue for reading usb 
device/config descriptor. Did you ensure the device descriptor you get is 
correct?

I also have no idea on the issue of reading 272 bytes config descriptor. You 
may have to dig into it by debugger.

Per my experience, some xHCI related initialization jobs are done in PEI/DXE 
phase chipset init driver, you may need copy them to your PEI driver before 
using xHCI to do transaction. 

Thanks
Feng

-Original Message-
From: Eric Wittmayer [mailto:e...@frescologic.com] 
Sent: Tuesday, September 08, 2015 14:38
To: Tian, Feng; edk2-devel@lists.01.org
Subject: RE: [edk2] Help debugging PEIM on Minnowboard Max

Thanks to Feng pointing me in the right direction, I've got this starting to 
work but I have encountered some strangeness.

I wrote the PEIM below to configure the xHCI controller and install the 
PEI_USB_CONTROLLER_PPI.  
I was seeing some problems reading device and config descriptors and found that 
if the memory locations I was reading descriptors into aren't at least
16 byte aligned, I wasn't getting all the data in memory.  I know PCIe accesses 
are dword aligned and use byte enables for the first/last dword to handle byte 
alignment but I don't understand why it would be sensitive to alignment larger 
than dwords.  

I'm currently stuck trying to read a 272 byte config descriptor.  I'm reading 
it into a page aligned chunk of memory but I'm still getting an exception in 
the debugger when I execute the request on the xHCI host.  What I see in the 
xHCI event ring after the doorbell ring are event TRBs for the setup and data 
stages of the control transfer but no event for the status stage.  After that 
things seem wedged.  My USB analyzer shows the Setup and data stages happening 
over USB but the host doesn't initiate the Status stage.

I'm using PeiServicesAllocatePages to allocate memory to read the descriptor 
into.  Do I need to use a specific type of memory to allow DMA transfers from 
the xHCI host to function properly?

Thanks for any ideas,
Eric

Here is the Xhci initialization code, the Base address I'm using is the same as 
the system uses in the Dxe phase per Feng's suggestion.

Index: Vlv2TbltDevicePkg/XhciPpi/XhciPpi.inf
===
--- Vlv2TbltDevicePkg/XhciPpi/XhciPpi.inf   (revision 0)
+++ Vlv2TbltDevicePkg/XhciPpi/XhciPpi.inf   (revision 29801)
@@ -0,0 +1,49 @@
+#  Copyright (c) 2015, Fresco Logic, Inc. All rights reserved.
+
+#  This program and the accompanying materials are licensed and made
available under
+#  the terms and conditions of the BSD License that accompanies this
distribution.  
+#  The full text of the license may be found at

+#  http://opensource.org/licenses/bsd-license.php.

+
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" 
+BASIS,

+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
IMPLIED.
+
+# Module Name:
+
+#  XhciPpi.h
+
+# Abstract:
+
+#  For Minnowboard Max set the xHCI controller memory BAR address and #  
+configure the usb ports to connect to the xhci controller instead #  of 
+the EHCI controller.
+#  Implement and install the PEI_USB_CONTROLLER_PPI during the Pei 
+phase #  for use in recovery.
+
+[defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = XhciPpi
+  FILE_GUID  = 340188CF-27A4-417E-8285-758AFF34F91C
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  PI_SPECIFICATION_VERSION   = 0x0001000A
+  ENTRY_POINT= XhciPpiEntry
+
+[sources.common]
+  XhciPpi.c
+  XhciPpi.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  PeimEntryPoint
+  DebugLib
+  PciLib
+
+[Ppis]
+  gPeiUsbControllerPpiGuid ## PRODUCES
+
+[Depex]
+  TRUE
Index: Vlv2TbltDevicePkg/XhciPpi/XhciPpi.c
===
--- Vlv2TbltDevicePkg/XhciPpi/XhciPpi.c (revision 0)
+++ Vlv2TbltDevicePkg/XhciPpi/XhciPpi.c (revision 29801)
@@ -0,0 +1,149 @@
+/*++
+
+Copyright (c) 2015, Fresco Logic, Inc. All rights reserved.
+
+This program and the accompanying materials are licensed and made 
+available
under
+the terms and conditions of the BSD License that accompanies this
distribution.
+The full text of the license may be found at 
+http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
IMPLIED.
+
+Module Name:
+  XhciPpi.c
+
+Abstract:
+
+For Minnowboard Max set the xHCI controller memory BAR address and 
+configure the usb ports to connect to the xhci controller instead of 
+the EHCI

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: adapt SectorCount when shortening transfers

2015-09-08 Thread Paolo Bonzini 


On 08/09/2015 09:02, Tian, Feng wrote:
> Hi, Laszlo
> 
> 1. I don't think the code in VirtioScsi.c is correct. It hardcodes
> the block size as 512bytes. The driver should send READ_CAPACITY cmd to get
> block size, then convert it to InTransferLength & OutTransferLength.

This is the controller's maximum transfer size, not the LUN.  The LUN's
limits are available from VPD and depend on the block size.  The
controller's limit apply to all LUNs and to all commands, even those
that transfer bytes rather than blocks.

For historical reasons the controller's maximum transfer size is
provided in 512-byte units.  In fact, Laszlo raised this point to the
virtio committee just to be sure that his code is correct, which it is.

> 2. Although the UEFI spec doesn't say the relationship of
> InTransferLength/OutTransferLength and CDB.TransferLength, but I think
> it's implied that InTransferLength/OutTransferLength =
> CDB.TransferLength * BlockSize. Otherwise how the lowest layer host
> controller driver constructs transfer descriptor? How many bytes to
> read? Is InTransferLength/OutTransferLength or the one of
> CDB.TransferLength * BlockSize?

It's always InTransferLength/OutTransferLength.  If it is < or >
CDB.TransferLength * BlockSize you get respectively an underrun or an
overrun.  An underrun is fatal, an overrun is not.

InTransferLength/OutTransferLength exist because the controller may not
know how to infer the transfer length for all CDB opcodes (some express
it in bytes, some express it in blocks, some are vendor specific, some
are just crazy and you have to inspect multiple CDB fields to compute
the transfer length).

Of course ScsiDisk *does* know how to infer the transfer length for the
CDBs it builds.  Therefore, it makes sense for ScsiDisk's own READ and
WRITE CDBs to always have InTransferLength/OutTransferLength =
CDB.TransferLength * BlockSize.  However, the controller driver must not
rely on this.

> 3. If we don't think #2 is implied, why we can assume "if pass thru
> driver returns EFI_BAD_BUFFER_SIZE and leave HostAdapterStatus at
> EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK, but *then* it must set either
> TargetStatus to some error code (different from
> EFI_EXT_SCSI_STATUS_TARGET_GOOD), *or* it must report some problem in
> the sense data."? IMHO, a pass thru driver could only return
> EFI_BAD_BUFFER_SIZE and leave HostAdpterStatus/TargetStatus to ok and
> no sense data.

I agree that:

- there's no need for the controller driver to set HostAdapterStatus to
anything

- even if it sets it to something, TargetStatus should not be set and
there should be no sense data.

Regarding the first point, this is not exactly an underrun, though it
may remind of one.  EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER might be just
as good, and even EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK might be okay
because the request has never reached the controller.  The important bit
is EFI_BAD_BUFFER_SIZE.  Thus, Laszlo's incremental patch in
http://article.gmane.org/gmane.comp.bios.edk2.devel/2007 is necessary if
I understand it correctly.

Regarding the second point, the TargetStatus and sense data *could* be
set if the transfer length in the CDB exceeds the maximum transfer
length in the VPD. In this case, the sense data will be filled and the
target status will be EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION.  This
is the case where the LUN's maximum transfer length is exceeded, which
is why it is based on the CDB rather than
InTransferLength/OutTransferLength.  However, in this case you cannot
expect EFI_BAD_BUFFER_SIZE to be set, because it's not the controller
driver's business to inspect target status and sense data (it's called
"passthru driver" for a reason :)).

Thus, the best you can do is to always back off to smaller sector sizes
when you get an unexpected error, like you did in r15491.

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

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Madt.aslc: Fix MADT header version

2015-09-08 Thread Leif Lindholm 
Fair enough.

Sudeep - can you point me to how I can verify the behaviour of these
patches? Has Al's sanity checking gone into 4.3 merge window, or do I
need to add it from elsewhere?

Regards,

Leif

On Mon, Sep 07, 2015 at 06:53:55PM +0100, Ryan Harkin wrote:
> Hi Sudeep,
> 
> I'm not involved in ACPI development, so am not able to review this.
> 
> Regards,
> Ryan.
> 
> On 7 September 2015 at 17:07, Sudeep Holla  wrote:
> 
> > Currently the MADT signature and revision is mapped to v1.0 macros
> > which results in MADT with incorrect entries in the header for Juno.
> > This patch fixes these EFI_ACPI_*_0_MULTIPLE_APIC_DESCRIPTION_TABLE
> > macros by using appropriate v5.0 versions.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Sudeep Holla 
> > Cc: Ryan Harkin 
> > Cc: Leif Lindholm 
> > Cc: Al Stone 
> > ---
> >  ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc | 24
> > 
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > Hi Ryan, Lief,
> >
> > Resending as my previous attempt was before I had subscribed to the list.
> > Al Stone is introducing strict check for MADT in Linux kernel and this
> > bug got exposed as result of that.
> >
> > Regards,
> > Sudeep
> >
> > diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
> > b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
> > index 406bd94f5636..d63a19b3904a 100644
> > --- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
> > +++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
> > @@ -26,19 +26,19 @@
> >#pragma pack (1)
> >
> >typedef struct {
> > -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
> >  EFI_ACPI_5_0_GIC_STRUCTURE
> > GicInterfaces[FixedPcdGet32 (PcdCoreCount)];
> >  EFI_ACPI_5_0_GIC_DISTRIBUTOR_STRUCTUREGicDistributor;
> > -  } EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE;
> > +  } EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE;
> >
> >#pragma pack ()
> >
> > -  EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = {
> > +  EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = {
> >  {
> >ARM_ACPI_HEADER (
> > -EFI_ACPI_1_0_APIC_SIGNATURE,
> > -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE,
> > -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE,
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
> >),
> >//
> >// MADT specific fields
> > @@ -68,20 +68,20 @@
> >#pragma pack (1)
> >
> >typedef struct {
> > -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
> >  EFI_ACPI_5_1_GIC_STRUCTURE
> > GicInterfaces[FixedPcdGet32 (PcdCoreCount)];
> >  EFI_ACPI_5_0_GIC_DISTRIBUTOR_STRUCTUREGicDistributor;
> >  EFI_ACPI_6_0_GIC_MSI_FRAME_STRUCTURE  MsiFrame;
> > -  } EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE;
> > +  } EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE;
> >
> >#pragma pack ()
> >
> > -  EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = {
> > +  EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = {
> >  {
> >ARM_ACPI_HEADER (
> > -EFI_ACPI_1_0_APIC_SIGNATURE,
> > -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE,
> > -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE,
> > +EFI_ACPI_5_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
> >),
> >//
> >// MADT specific fields
> > --
> > 1.9.1
> >
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Madt.aslc: Fix MADT header version

2015-09-08 Thread Sudeep Holla 



On 08/09/15 11:02, Leif Lindholm wrote:

Fair enough.

Sudeep - can you point me to how I can verify the behaviour of these
patches? Has Al's sanity checking gone into 4.3 merge window, or do I
need to add it from elsewhere?



No, not yet made to upstream kernel, but I briefly checked now and I can
see the commmit[0] in leg-kernel[1]. It just fails to boot so we need to
get this change in before the kernel change is merged IMO. I wonder if
that branch was tested on Juno though.

"...
ACPI: undefined MADT subtable type for FADT 5.1: 12 (length 24).
Kernel panic - not syncing: No interrupt controller.
"

Anyway this change can go irrespective of the kernel change as we
started supporting ACPI on ARM only from v5.0 and it makes no sense to
mark MADT as v1.0. I think this bug was not visible as most of the
macros have same value expect revision which got caught by Al's sanity
check.

Regards,
Sudeep

[0] 
https://git.linaro.org/leg/acpi/leg-kernel.git/commit/bd8aac91f1ee9a90b6b51b2c48b1e0ee3755c5b0
[1] 
https://git.linaro.org/leg/acpi/leg-kernel.git/shortlog/refs/heads/leg-kernel

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

[edk2] How do I get rid of /ALIGN:32 ?

2015-09-08 Thread Shubha Ramani 
Seems to be a linker option by default when I compile EDK2. I want to get rid 
of it.
        "C:\Program Files (x86)\Microsoft Visual Studio 
10.0\Vc\bin\x86_amd64\link.exe" 
/OUT:c:\edk2\myworkspace\Build\DuetPkgX64\DEBUG_VS2010x86\X64\MdeModulePkg\Application\testPTU\testPTU\DEBUG\testPTU.dll
 /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /MAP /ALIGN:32 
/SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /LTCG /DLL 
/ENTRY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 
/DRIVER /DEBUG  
@c:\edk2\myworkspace\Build\DuetPkgX64\DEBUG_VS2010x86\X64\MdeModulePkg\Application\testPTU\testPTU\OUTPUT\static_library_files.lstGenerating
 code
Shubha Shubha D. ramanishubharam...@gmail.com
shubharam...@yahoo.com
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

2015-09-08 Thread Laszlo Ersek 
On 09/08/15 19:26, Anthony PERARD wrote:
> On Fri, Aug 28, 2015 at 10:17:28AM +0200, Laszlo Ersek wrote:
>> On 08/08/15 02:02, Zeng, Star wrote:
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
 Laszlo Ersek
 Sent: Saturday, August 8, 2015 12:00 AM
 To: edk2-devel-01
 Cc: Paolo Bonzini; Zeng, Star; Justen, Jordan L
 Subject: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack

 SVN rev 18166 ("MdeModulePkg DxeIpl: Add stack NX support") enables
 platforms to request non-executable stack for the DXE phase, by setting
 PcdSetNxForStack to TRUE.

 The PCD defaults to FALSE, because:

 (a) A non-executable DXE stack is a new feature and causes changes in
 behavior. Some platform could rely on executing code from the stack.

 (b) The code enabling NX in the DXE IPL PEIM enforces the

   PcdSetNxForStack ==> PcdDxeIplBuildPageTables

 implication for "64-bit PEI + 64-bit DXE" platforms, with a new
 ASSERT(). Some platform might not comply with this requirement
 immediately.

 Regarding (a), in none of the OVMF builds do we try to execute code from
 the stack.

 Regarding (b):

 - In the OvmfPkgX64.dsc build (which is where (b) applies) we simply
   inherit the PcdDxeIplBuildPageTables|TRUE default from
   "MdeModulePkg/MdeModulePkg.dec". Therefore we can set
 PcdSetNxForStack
   to TRUE.

 - In OvmfPkgIa32X64.dsc, page tables are built by default for DXE. Hence
   we can set PcdSetNxForStack to TRUE.

 - In OvmfPkgIa32.dsc, page tables used not to be necessary until now.
   After we set PcdSetNxForStack to TRUE in this patch, the DXE IPL will
   construct page tables even when it is built as part of OvmfPkgIa32.dsc,
   provided the (virtual) hardware supports both PAE mode and the XD bit.

 Should this setting cause problems in a GPU (or other device) passthru
 scenario, with a UEFI_DRIVER in the PCI option rom attempting to execute
 code from the stack, the feature can be dynamically disabled on the QEMU
 command line, with "-cpu ,-nx".

 Cc: Paolo Bonzini 
 Cc: Jordan Justen 
 Cc: "Zeng, Star" 
 Suggested-by: Paolo Bonzini 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Laszlo Ersek 
>>>
>>> Reviewed by: Star Zeng 
>>
>> Committed as SVN r18360. Thanks!
>> Laszlo
> 
> Hi,
> 
> This change breaks Debian installer 7.2, or wheezy while running in a Xen
> guest.
> http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg00845.html
> 
> I've reproduce this using this iso:
> http://ftp.uk.debian.org/debian/dists/wheezy/main/installer-amd64/current/images/netboot/mini.iso
> 
> And I get this on the console:
> Welcome to GRUB!
> 
>  X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -  
> RIP  - 0F5F8918, CS  - 0028, RFLAGS - 00210206
> ExceptionData - 0011
> RAX  - , RCX - 07FCE000, RDX - 
> RBX  - 0B6092C0, RSP - 0F5F8590, RBP - 0B608EA0
> RSI  - 0F5F8838, RDI - 0B608EA0
> R8   - , R9  - 0B609200, R10 - 
> R11  - 000A, R12 - , R13 - 001B
> R14  - 0B609360, R15 - 
> DS   - 0008, ES  - 0008, FS  - 0008
> GS   - 0008, SS  - 0008
> CR0  - 8033, CR2 - 0F5F8918, CR3 - 0F597000
> CR4  - 0668, CR8 - 
> DR0  - , DR1 - , DR2 - 
> DR3  - , DR6 - 0FF0, DR7 - 0400
> GDTR - 0F57BF18 003F, LDTR - 
> IDTR - 0EEA5018 0FFF,   TR - 
> FXSAVE_STATE - 0F5F81F0
>  Find PE image 
> /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll
>  (ImageBase=0F556000, EntryPoint=0F55628F) 
> 
> I did check with other guest (Windows, Ubuntu, Debian Jessie), and they are
> working correctly. Debian Wheezy is the only one that fail.

I don't have an environment to reproduce this in. I think we should try
to understand this problem better, before deciding how to make it go away.

Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
directory (ie. under the location listed in the error report). Then,
please disassemble it with "objdump -S". The fault location in the
disassembly can be found

[edk2] [PATCH v2 2/4] BaseTools/GenFw: make ARM's .data adhere to PE/COFF section alignment

2015-09-08 Thread Ard Biesheuvel 
There is a special case in the ElfConvert code where ARM's .data is
not aligned to section alignment, presumably to preserve the relative
offset between .text and .data in the presence of relative relocations
that are not recomputed at PE/COFF conversion time.

This violates the PE/COFF spec, so it is better to actively check
whether the relative section offsets have been preserved during
PE/COFF conversion instead of assuming things will still line up if
we just don't align .data, especially now that we set the PE/COFF
section alignment based on the alignment requirements of the ELF
input file.

So make the .data alignment unconditional, and add a check whether
relative relocations are still valid.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index e1b92ebd713e..da3cde8f8e9b 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -366,10 +366,7 @@ ScanSections32 (
   }
 
   mDebugOffset = mCoffOffset;
-
-  if (mEhdr->e_machine != EM_ARM) {
-mCoffOffset = CoffAlign(mCoffOffset);
-  }
+  mCoffOffset = CoffAlign(mCoffOffset);
 
   if (SectionCount > 1 && mOutImageType == FW_EFI_IMAGE) {
 Warning (NULL, 0, 0, NULL, "Mulitple sections in %s are merged into 1 text 
section. Source level debug might not work correctly.", mInImageName);
@@ -719,7 +716,7 @@ WriteSections32 (
   switch (ELF32_R_TYPE(Rel->r_info)) {
   case R_ARM_RBASE:
 // No relocation - no action required
-// break skipped
+break;
 
   case R_ARM_PC24:
   case R_ARM_XPC25:
@@ -756,8 +753,17 @@ WriteSections32 (
   case R_ARM_TLS_GD32:
   case R_ARM_TLS_LDM32:
   case R_ARM_TLS_IE32:
-// Thease are all PC-relative relocations and don't require 
modification
-// GCC does not seem to have the concept of a application that 
just needs to get relocated.
+//
+// These are all PC-relative relocations and don't require 
modification
+// as long as the relative offset between the section containing 
the
+// place and the section containing the reference has been 
preserved
+// by the PE/COFF conversion.
+//
+if ((SymShdr->sh_addr - SecShdr->sh_addr) !=
+(mCoffSectionsOffset[Sym->st_shndx] - SecOffset)) {
+  Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s ARM 
relative relocations require identical ELF and PE/COFF section offsets",
+  mInImageName);
+}
 break;
 
   case R_ARM_THM_MOVW_ABS_NC:
-- 
1.9.1

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

[edk2] [PATCH v2 1/4] BaseTools/GenFw: remove ARM and RVCT references from ELF64 code

2015-09-08 Thread Ard Biesheuvel 
ARM and RVCT apply to 32-bit code only, so remove any references
to them from the 64-bit version of ElfConvert.c

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 17 +
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c758ed9d64a6..ad169b141c24 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -360,10 +360,7 @@ ScanSections64 (
   }
 
   mDebugOffset = mCoffOffset;
-
-  if (mEhdr->e_machine != EM_ARM) {
-mCoffOffset = CoffAlign(mCoffOffset);
-  }
+  mCoffOffset = CoffAlign(mCoffOffset);
 
   if (SectionCount > 1 && mOutImageType == FW_EFI_IMAGE) {
 Warning (NULL, 0, 0, NULL, "Mulitple sections in %s are merged into 1 text 
section. Source level debug might not work correctly.", mInImageName);
@@ -383,12 +380,6 @@ ScanSections64 (
 if ((shdr->sh_addr & (shdr->sh_addralign - 1)) == 0) {
   // if the section address is aligned we must align PE/COFF
   mCoffOffset = (UINT32) ((mCoffOffset + shdr->sh_addralign - 1) & 
~(shdr->sh_addralign - 1));
-} else if ((shdr->sh_addr % shdr->sh_addralign) != (mCoffOffset % 
shdr->sh_addralign)) {
-  // ARM RVCT tools have behavior outside of the ELF specification to 
try
-  // and make images smaller.  If sh_addr is not aligned to 
sh_addralign
-  // then the section needs to preserve sh_addr MOD sh_addralign.
-  // Normally doing nothing here works great.
-  Error (NULL, 0, 3000, "Invalid", "Unsupported section alignment.");
 }
   }
 
@@ -439,12 +430,6 @@ ScanSections64 (
 if ((shdr->sh_addr & (shdr->sh_addralign - 1)) == 0) {
   // if the section address is aligned we must align PE/COFF
   mCoffOffset = (UINT32) ((mCoffOffset + shdr->sh_addralign - 1) & 
~(shdr->sh_addralign - 1));
-} else if ((shdr->sh_addr % shdr->sh_addralign) != (mCoffOffset % 
shdr->sh_addralign)) {
-  // ARM RVCT tools have behavior outside of the ELF specification to 
try
-  // and make images smaller.  If sh_addr is not aligned to 
sh_addralign
-  // then the section needs to preserve sh_addr MOD sh_addralign.
-  // Normally doing nothing here works great.
-  Error (NULL, 0, 3000, "Invalid", "Unsupported section alignment.");
 }
   }
   if (shdr->sh_size != 0) {
-- 
1.9.1

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

[edk2] [PATCH v2 3/4] BaseTools/ARM: move to unified GCC linker script

2015-09-08 Thread Ard Biesheuvel 
Instead of using the ARM builtin linker script for GNU ld, use the
new unified one instead. This will allow us to increase the section
alignment for DXE_RUNTIME_MODULEs, which is a prerequisite for
enabling the UEFIv2.5 Properties Table memory protection feature.

Also, remove the -mword-relocations GCC option, since we now fully
support all relative and absolute 32-bit ARM/ELF relocations.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index aa2b6b1488c4..92c0993208e1 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3811,13 +3811,13 @@ DEFINE GCC_ALL_CC_FLAGS= -g -Os 
-fshort-wchar -fno-strict-aliasing -
 DEFINE GCC_IA32_CC_FLAGS   = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double 
-freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe
 DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
-Wno-address -mno-stack-arg-probe
 DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-minline-int-divide-min-latency
-DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mword-relocations 
-mlittle-endian -mabi=aapcs -mapcs -fno-short-enums -save-temps -fsigned-char 
-ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb 
-mfloat-abi=soft
+DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -mapcs -fno-short-enums -save-temps -fsigned-char 
-ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb 
-mfloat-abi=soft
 DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mcmodel=tiny 
-mlittle-endian -fno-short-enums -save-temps -fverbose-asm -fsigned-char  
-ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin 
-Wno-address -fno-asynchronous-unwind-tables
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
 DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) --gc-sections
 DEFINE GCC_ARM_AARCH64_DLINK_COMMON= --emit-relocs -nostdlib --gc-sections -u 
$(IMAGE_ENTRY_POINT) -e $(IMAGE_ENTRY_POINT) -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map
-DEFINE GCC_ARM_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) 
-Ttext=0x0
+DEFINE GCC_ARM_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) -z 
common-page-size=0x20
 DEFINE GCC_AARCH64_DLINK_FLAGS = DEF(GCC_ARM_AARCH64_DLINK_COMMON) -z 
common-page-size=0x20
 DEFINE GCC_IA32_X64_ASLDLINK_FLAGS = DEF(GCC_IA32_X64_DLINK_COMMON) --entry 
_ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
 DEFINE GCC_ARM_ASLDLINK_FLAGS  = DEF(GCC_ARM_DLINK_FLAGS) --entry 
ReferenceAcpiTable -u $(IMAGE_ENTRY_POINT)
@@ -3871,6 +3871,7 @@ DEFINE GCC46_ASM_FLAGS   = 
DEF(GCC45_ASM_FLAGS)
 DEFINE GCC46_ARM_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
 DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector
 DEFINE GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) 
--oformat=elf32-littlearm
+DEFINE GCC46_ARM_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x220
 DEFINE GCC46_ARM_ASLDLINK_FLAGS  = DEF(GCC_ARM_ASLDLINK_FLAGS) 
--oformat=elf32-littlearm
 
 DEFINE GCC47_IA32_CC_FLAGS   = DEF(GCC46_IA32_CC_FLAGS)
@@ -3887,6 +3888,7 @@ DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS) 
$(PLATFORM_FLAGS) DEF(GC
 DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS) 
-mno-unaligned-access
 DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)
 DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
+DEFINE GCC47_ARM_DLINK2_FLAGS= DEF(GCC46_ARM_DLINK2_FLAGS)
 DEFINE GCC47_AARCH64_DLINK_FLAGS = DEF(GCC_AARCH64_DLINK_FLAGS)
 DEFINE GCC47_AARCH64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x228
 DEFINE GCC47_ARM_ASLDLINK_FLAGS  = DEF(GCC46_ARM_ASLDLINK_FLAGS)
@@ -3906,6 +3908,7 @@ DEFINE GCC48_AARCH64_ASM_FLAGS   = 
DEF(GCC47_AARCH64_ASM_FLAGS)
 DEFINE GCC48_ARM_CC_FLAGS= DEF(GCC47_ARM_CC_FLAGS)
 DEFINE GCC48_AARCH64_CC_FLAGS= DEF(GCC47_AARCH64_CC_FLAGS)
 DEFINE GCC48_ARM_DLINK_FLAGS = DEF(GCC47_ARM_DLINK_FLAGS)
+DEFINE GCC48_ARM_DLINK2_FLAGS= DEF(GCC47_ARM_DLINK2_FLAGS)
 DEFINE GCC48_AARCH64_DLINK_FLAGS = DEF(GCC47_AARCH64_DLINK_FLAGS)
 DEFINE GCC48_AARCH64_DLINK2_FLAGS= DEF(GCC47_AARCH64_DLINK2_FLAGS)
 DEFINE GCC48_ARM_ASLDLINK_FLAGS  = DEF(GCC47_ARM_ASLDLINK_FLAGS)
@@ -3925,6 +3928,7 @@ DEFINE GCC49_AARCH64_ASM_FLAGS   = 
DEF(GCC48_AARCH64_ASM_FLAGS)
 DEFINE

[edk2] [PATCH v2 4/4] ArmVirtPkg: use 4 KB section alignment for ARM DXE_RUNTIME modules

2015-09-08 Thread Ard Biesheuvel 
In order to support the Properties Table memory protection feature
on 32-bit ARM, build DXE_RUNTIME_MODULE type binaries with 4 KB section
alignment by setting the common-page-size linker command line option.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Acked-by: Laszlo Ersek 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 8372c5813cb3..b49c1bfb8b04 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -17,7 +17,8 @@ [Defines]
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
   DEFINE TTY_TERMINAL= FALSE
 
-[BuildOptions.AARCH64.EDKII.DXE_RUNTIME_DRIVER]
+[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
+  GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1
 
 [LibraryClasses.common]
-- 
1.9.1

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

Re: [edk2] [PATCH] MdePkg: Refine UefiFileHandleLib to avoid write non-ASCII char into ASCII file.

2015-09-08 Thread Carsey, Jaben 
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Qiu, Shumin
> Sent: Tuesday, September 08, 2015 12:05 AM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; Carsey, Jaben
> ; Gao, Liming 
> Subject: [PATCH] MdePkg: Refine UefiFileHandleLib to avoid write non-ASCII
> char into ASCII file.
> Importance: High
> 
> Cc: Jaben Carsey 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin 
> ---
>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> index 04a2f18..dfec5fa 100644
> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> @@ -1079,6 +1079,7 @@ FileHandleWriteLine(
>EFI_STATUS  Status;
>CHAR16  CharBuffer;
>UINTN   Size;
> +  UINTN   Index;
>UINTN   CharSize;
>UINT64  FileSize;
>UINT64  OriginalFilePosition;
> @@ -1136,6 +1137,12 @@ FileHandleWriteLine(
>return EFI_OUT_OF_RESOURCES;
>  }
>  UnicodeStrToAsciiStr (Buffer, AsciiBuffer);
> +for (Index = 0; Index < Size; Index++) {
> +  if (!((AsciiBuffer[Index] >= 0) && (AsciiBuffer[Index] < 128))){
> +FreePool(AsciiBuffer);
> +return EFI_INVALID_PARAMETER;
> +  }
> +}
> 
>  Size = AsciiStrSize(AsciiBuffer) - sizeof(CHAR8);
>  Status = FileHandleWrite(Handle, , AsciiBuffer);
> --
> 1.9.5.msysgit.1

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

Re: [edk2] How do I get rid of /ALIGN:32 ?

2015-09-08 Thread Gao, Liming 
Shubha:
  You can override it in [BuildOptions] section of your DSC file. You can see 
the example in Nt32Pkg\Nt32Pkg.dsc file that overrides its value to 4096. 

[BuildOptions]
  DEBUG_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) 
/BASE:0x1 /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
  RELEASE_*_*_DLINK_FLAGS = /ALIGN:4096 /FILEALIGN:4096

Thanks
Liming
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shubha 
Ramani
Sent: Wednesday, September 9, 2015 6:27 AM
To: edk2-devel@lists.01.org
Subject: [edk2] How do I get rid of /ALIGN:32 ?

Seems to be a linker option by default when I compile EDK2. I want to get rid 
of it.
        "C:\Program Files (x86)\Microsoft Visual Studio 
10.0\Vc\bin\x86_amd64\link.exe" 
/OUT:c:\edk2\myworkspace\Build\DuetPkgX64\DEBUG_VS2010x86\X64\MdeModulePkg\Application\testPTU\testPTU\DEBUG\testPTU.dll
 /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /MAP /ALIGN:32 
/SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /LTCG /DLL 
/ENTRY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 
/DRIVER /DEBUG  
@c:\edk2\myworkspace\Build\DuetPkgX64\DEBUG_VS2010x86\X64\MdeModulePkg\Application\testPTU\testPTU\OUTPUT\static_library_files.lstGenerating
 code Shubha Shubha D. ramanishubharam...@gmail.com shubharam...@yahoo.com 
___
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 1/4] ArmPkg/Mmu: Fix bug of aligning new allocated page table

2015-09-08 Thread Heyi Guo 



On 09/07/2015 06:17 PM, Ard Biesheuvel wrote:

On 7 September 2015 at 10:41, Heyi Guo  wrote:


On 09/06/2015 07:43 PM, Ard Biesheuvel wrote:

On 6 September 2015 at 10:15, Heyi Guo  wrote:

The code has a simple bug on calculating aligned page table address.
We need to add alignment - 1 to allocated address first and then mask
the unaligned bits.


Nice find!


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
   ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index 3d58d5d..4db4bbe 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -381,7 +381,7 @@ GetBlockEntryListFromAddress (
   if (TranslationTable == NULL) {
 return NULL;
   }
-TranslationTable = (UINT64*)((UINTN)TranslationTable &
TT_ADDRESS_MASK_DESCRIPTION_TABLE);
+TranslationTable = (UINT64*)(((UINTN)TranslationTable +
TT_ALIGNMENT_DESCRIPTION_TABLE - 1) & TT_ADDRESS_MASK_DESCRIPTION_TABLE);


Could we do (TranslationTable | (TT_ALIGNMENT_DESCRIPTION_TABLE - 1))
+ 1 instead of using two different symbolic constants?


   // Populate the newly created lower level table
   SubTableBlockEntry = TranslationTable;
@@ -409,7 +409,7 @@ GetBlockEntryListFromAddress (
   if (TranslationTable == NULL) {
 return NULL;
   }
-TranslationTable = (UINT64*)((UINTN)TranslationTable &
TT_ADDRESS_MASK_DESCRIPTION_TABLE);
+TranslationTable = (UINT64*)(((UINTN)TranslationTable +
TT_ALIGNMENT_DESCRIPTION_TABLE - 1) & TT_ADDRESS_MASK_DESCRIPTION_TABLE);

   ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));


I think you missed another instance at line 625

So what is hidden from view by all these symbolic constants is that we
are aligning the return value of AllocatePages() to 4 KB, which is
rather pointless, and is actually resulting in every allocation to
waste 4 KB. But that also means the buggy rounding code never produces
incorrect values, making the severity of this bug low.

So instead, could we get a comprehensive patch that:
- replaces all three instances with a call to a static function that
does the allocation and the rounding
- apply the rounding only if the alignment exceeds 4 KB
- fix the rounding logic.


Good suggestion to make code clean; thanks :)


Just as a note, you could implement something like:

Table = AllocatePages (size + rounding)
RoundedTable = (Table | (Alignment - 1)) +1
FreePages (Table, RoundedTable - Table)
FreePages()

to immediately free the pages that you will not use after rounding.


Can we just use AllocateAlignedPages in MemoryAllocationLib to get the 
same result?


Heyi




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

Re: [edk2] [PATCH 1/2] ShellPkg: Fix 'for' command fail with multiple fields.

2015-09-08 Thread Carsey, Jaben 
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Qiu, Shumin
> Sent: Sunday, September 06, 2015 8:06 PM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; Carsey, Jaben
> 
> Subject: [PATCH 1/2] ShellPkg: Fix 'for' command fail with multiple fields.
> Importance: High
> 
> When multiple fields are found in 'for' command return invalid parameters
> error.
> 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin 
> ---
>  ShellPkg/Library/UefiShellLevel1CommandsLib/For.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> b/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> index 2ecc5cd..cbf0517 100644
> --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> @@ -438,6 +438,11 @@ ShellCommandRunFor (
>  gEfiShellParametersProtocol->Argv[2]) == 0) {
>for (LoopVar = 0x3 ; LoopVar < gEfiShellParametersProtocol->Argc ;
> LoopVar++) {
>  ASSERT((ArgSet == NULL && ArgSize == 0) || (ArgSet != NULL));
> +if (StrStr (gEfiShellParametersProtocol->Argv[LoopVar], L")") != NULL
> &&
> +(LoopVar + 1) < gEfiShellParametersProtocol->Argc
> +   ) {
> +  return (SHELL_INVALID_PARAMETER);
> +}
>  if (ArgSet == NULL) {
>  //ArgSet = StrnCatGrow(, , L"\"", 0);
>  } else {
> --
> 1.9.5.msysgit.1

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

[edk2] [PATCH 0/3] large memory support for 32-bit ARM

2015-09-08 Thread Ard Biesheuvel 
Some changes are needed to support memory over the 4 GB mark on 32-bit
ARM, e.g., qemu with more than 3 GB.

Ard Biesheuvel (3):
  ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM
  ArmVirtPkg/ArmVirtMemoryInitPeiLib: handle memory above 4 GB on 32-bit
ARM
  ArmVirtPkg: set max physical address width to 40 bits

 ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c|  7 +++--
 ArmVirtPkg/ArmVirt.dsc.inc   |  3 ++
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 31 

 3 files changed, 33 insertions(+), 8 deletions(-)

-- 
1.9.1

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

[edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

2015-09-08 Thread Ard Biesheuvel 
Make sure that the PEI memory region is carved out of memory that is
32-bit addressable, by taking MAX_ADDRESS into account (which is
defined as '4 GB - 1' on ARM)

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
index 93ab16ca4a74..9f26d26a04d3 100755
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
@@ -96,7 +96,7 @@ InitializeMemory (
 {
   EFI_STATUSStatus;
   UINTN SystemMemoryBase;
-  UINTN SystemMemoryTop;
+  UINT64SystemMemoryTop;
   UINTN FdBase;
   UINTN FdTop;
   UINTN UefiMemoryBase;
@@ -115,7 +115,10 @@ InitializeMemory (
   ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
 
   SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
-  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 (PcdSystemMemorySize);
+  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
+  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
+SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
+  }
   FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
   FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
 
-- 
1.9.1

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

[edk2] [PATCH 3/3] ArmVirtPkg: set max physical address width to 40 bits

2015-09-08 Thread Ard Biesheuvel 
When executing on a LPAE capable 32-bit ARM platform, we support
up to 40 bits of physical address space so set PcdPrePiCpuMemorySize
accordingly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index b49c1bfb8b04..c1b78be84e74 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -371,6 +371,9 @@ [PcdsFixedAtBuild.common]
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 
0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
 !endif
 
+[PcdsFixedAtBuild.ARM]
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
+
 [Components.common]
   #
   # Networking stack
-- 
1.9.1

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

[edk2] [PATCH 2/3] ArmVirtPkg/ArmVirtMemoryInitPeiLib: handle memory above 4 GB on 32-bit ARM

2015-09-08 Thread Ard Biesheuvel 
On 32-bit ARM, split system memory into a region below (and up to) 4 GB
and a region above 4 GB. This is necessary to get the DXE core to consider
the former as the resource descriptor that describes the primary memory
region that also covers the PHIT region.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 31 

 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git 
a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c 
b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
index 8ce63b4596e2..310d51b19358 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
@@ -56,6 +56,7 @@ MemoryPeim (
   )
 {
   EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
+  UINT64  SystemMemoryTop;
 
   // Ensure PcdSystemMemorySize has been set
   ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
@@ -73,12 +74,30 @@ MemoryPeim (
   EFI_RESOURCE_ATTRIBUTE_TESTED
   );
 
-  BuildResourceDescriptorHob (
-  EFI_RESOURCE_SYSTEM_MEMORY,
-  ResourceAttributes,
-  PcdGet64 (PcdSystemMemoryBase),
-  PcdGet64 (PcdSystemMemorySize)
-  );
+  SystemMemoryTop = PcdGet64 (PcdSystemMemoryBase) +
+PcdGet64 (PcdSystemMemorySize);
+
+  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
+BuildResourceDescriptorHob (
+EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+PcdGet64 (PcdSystemMemoryBase),
+(UINT64) MAX_ADDRESS - PcdGet64 (PcdSystemMemoryBase) + 1
+);
+BuildResourceDescriptorHob (
+EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+(UINT64) MAX_ADDRESS + 1,
+SystemMemoryTop - MAX_ADDRESS - 1
+);
+  } else {
+BuildResourceDescriptorHob (
+EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+PcdGet64 (PcdSystemMemoryBase),
+PcdGet64 (PcdSystemMemorySize)
+);
+  }
 
   //
   // When running under virtualization, the PI/UEFI memory region may be
-- 
1.9.1

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

[edk2] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

2015-09-08 Thread Anthony PERARD 
On Fri, Aug 28, 2015 at 10:17:28AM +0200, Laszlo Ersek wrote:
> On 08/08/15 02:02, Zeng, Star wrote:
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Saturday, August 8, 2015 12:00 AM
> >> To: edk2-devel-01
> >> Cc: Paolo Bonzini; Zeng, Star; Justen, Jordan L
> >> Subject: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack
> >>
> >> SVN rev 18166 ("MdeModulePkg DxeIpl: Add stack NX support") enables
> >> platforms to request non-executable stack for the DXE phase, by setting
> >> PcdSetNxForStack to TRUE.
> >>
> >> The PCD defaults to FALSE, because:
> >>
> >> (a) A non-executable DXE stack is a new feature and causes changes in
> >> behavior. Some platform could rely on executing code from the stack.
> >>
> >> (b) The code enabling NX in the DXE IPL PEIM enforces the
> >>
> >>   PcdSetNxForStack ==> PcdDxeIplBuildPageTables
> >>
> >> implication for "64-bit PEI + 64-bit DXE" platforms, with a new
> >> ASSERT(). Some platform might not comply with this requirement
> >> immediately.
> >>
> >> Regarding (a), in none of the OVMF builds do we try to execute code from
> >> the stack.
> >>
> >> Regarding (b):
> >>
> >> - In the OvmfPkgX64.dsc build (which is where (b) applies) we simply
> >>   inherit the PcdDxeIplBuildPageTables|TRUE default from
> >>   "MdeModulePkg/MdeModulePkg.dec". Therefore we can set
> >> PcdSetNxForStack
> >>   to TRUE.
> >>
> >> - In OvmfPkgIa32X64.dsc, page tables are built by default for DXE. Hence
> >>   we can set PcdSetNxForStack to TRUE.
> >>
> >> - In OvmfPkgIa32.dsc, page tables used not to be necessary until now.
> >>   After we set PcdSetNxForStack to TRUE in this patch, the DXE IPL will
> >>   construct page tables even when it is built as part of OvmfPkgIa32.dsc,
> >>   provided the (virtual) hardware supports both PAE mode and the XD bit.
> >>
> >> Should this setting cause problems in a GPU (or other device) passthru
> >> scenario, with a UEFI_DRIVER in the PCI option rom attempting to execute
> >> code from the stack, the feature can be dynamically disabled on the QEMU
> >> command line, with "-cpu ,-nx".
> >>
> >> Cc: Paolo Bonzini 
> >> Cc: Jordan Justen 
> >> Cc: "Zeng, Star" 
> >> Suggested-by: Paolo Bonzini 
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Laszlo Ersek 
> > 
> > Reviewed by: Star Zeng 
> 
> Committed as SVN r18360. Thanks!
> Laszlo

Hi,

This change breaks Debian installer 7.2, or wheezy while running in a Xen
guest.
http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg00845.html

I've reproduce this using this iso:
http://ftp.uk.debian.org/debian/dists/wheezy/main/installer-amd64/current/images/netboot/mini.iso

And I get this on the console:
Welcome to GRUB!

 X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -  
RIP  - 0F5F8918, CS  - 0028, RFLAGS - 00210206
ExceptionData - 0011
RAX  - , RCX - 07FCE000, RDX - 
RBX  - 0B6092C0, RSP - 0F5F8590, RBP - 0B608EA0
RSI  - 0F5F8838, RDI - 0B608EA0
R8   - , R9  - 0B609200, R10 - 
R11  - 000A, R12 - , R13 - 001B
R14  - 0B609360, R15 - 
DS   - 0008, ES  - 0008, FS  - 0008
GS   - 0008, SS  - 0008
CR0  - 8033, CR2 - 0F5F8918, CR3 - 0F597000
CR4  - 0668, CR8 - 
DR0  - , DR1 - , DR2 - 
DR3  - , DR6 - 0FF0, DR7 - 0400
GDTR - 0F57BF18 003F, LDTR - 
IDTR - 0EEA5018 0FFF,   TR - 
FXSAVE_STATE - 0F5F81F0
 Find PE image 
/build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll
 (ImageBase=0F556000, EntryPoint=0F55628F) 

I did check with other guest (Windows, Ubuntu, Debian Jessie), and they are
working correctly. Debian Wheezy is the only one that fail.

Thanks,

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

Re: [edk2] [PATCH 1/4] ArmPkg/Mmu: Fix bug of aligning new allocated page table

2015-09-08 Thread Ard Biesheuvel 
On 9 September 2015 at 05:16, Heyi Guo  wrote:
>
>
> On 09/07/2015 06:17 PM, Ard Biesheuvel wrote:
>>
>> On 7 September 2015 at 10:41, Heyi Guo  wrote:
>>>
>>>
>>> On 09/06/2015 07:43 PM, Ard Biesheuvel wrote:

 On 6 September 2015 at 10:15, Heyi Guo  wrote:
>
> The code has a simple bug on calculating aligned page table address.
> We need to add alignment - 1 to allocated address first and then mask
> the unaligned bits.
>
 Nice find!

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> ---
>ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 4 ++--
>1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index 3d58d5d..4db4bbe 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -381,7 +381,7 @@ GetBlockEntryListFromAddress (
>if (TranslationTable == NULL) {
>  return NULL;
>}
> -TranslationTable = (UINT64*)((UINTN)TranslationTable &
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
> +TranslationTable = (UINT64*)(((UINTN)TranslationTable +
> TT_ALIGNMENT_DESCRIPTION_TABLE - 1) &
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
>
 Could we do (TranslationTable | (TT_ALIGNMENT_DESCRIPTION_TABLE - 1))
 + 1 instead of using two different symbolic constants?

>// Populate the newly created lower level table
>SubTableBlockEntry = TranslationTable;
> @@ -409,7 +409,7 @@ GetBlockEntryListFromAddress (
>if (TranslationTable == NULL) {
>  return NULL;
>}
> -TranslationTable = (UINT64*)((UINTN)TranslationTable &
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
> +TranslationTable = (UINT64*)(((UINTN)TranslationTable +
> TT_ALIGNMENT_DESCRIPTION_TABLE - 1) &
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
>
>ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));
>
 I think you missed another instance at line 625

 So what is hidden from view by all these symbolic constants is that we
 are aligning the return value of AllocatePages() to 4 KB, which is
 rather pointless, and is actually resulting in every allocation to
 waste 4 KB. But that also means the buggy rounding code never produces
 incorrect values, making the severity of this bug low.

 So instead, could we get a comprehensive patch that:
 - replaces all three instances with a call to a static function that
 does the allocation and the rounding
 - apply the rounding only if the alignment exceeds 4 KB
 - fix the rounding logic.
>>>
>>>
>>> Good suggestion to make code clean; thanks :)
>>>
>> Just as a note, you could implement something like:
>>
>> Table = AllocatePages (size + rounding)
>> RoundedTable = (Table | (Alignment - 1)) +1
>> FreePages (Table, RoundedTable - Table)
>> FreePages()
>>
>> to immediately free the pages that you will not use after rounding.
>
>
> Can we just use AllocateAlignedPages in MemoryAllocationLib to get the same
> result?
>

Yes, even better! Then you don't even need to factor it out into a
separate function, I think

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

Re: [edk2] [Patch] MdeModulePkg: Fix a performance data buffer overrun issue

2015-09-08 Thread Zeng, Star 

On 2015/9/9 13:36, Ruiyu Ni wrote:

The mBmPerfHeader.Count isn't reset to 0 in BmWriteBootToOsPerformanceData()
so when the actual performance data entry count exceeds the LimitCount,
the performance data collection breaks on condition
if (mBmPerfHeader.Count == LimitCount), but 2nd time calling this function
will not break on condition if (mBmPerfHeader.Count == LimitCount) because
the mBmPerfHeader.Count always bigger than LimitCount, which results buffer
overrun.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Star Zeng 
---
  MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c | 5 +
  1 file changed, 5 insertions(+)


Reviewed-by: Star Zeng 



diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c
index 7b13ec6..e45c0bd 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c
@@ -186,6 +186,11 @@ BmWriteBootToOsPerformanceData (
PERF_END(NULL, "BDS", NULL, 0);

//
+  // Reset the entry count
+  //
+  mBmPerfHeader.Count = 0;
+
+  //
// Retrieve time stamp count as early as possible
//
Ticker  = GetPerformanceCounter ();



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

[edk2] [patch] MdePkg/UefiScsiLib: comments update to add EFI_INVALID_PARAMETER status

2015-09-08 Thread Tian Feng 
EFI_SCSI_IO_PROTOCOL has alignment requirement on any data buffer
used in SCSI data transfer. As a wrap of this protocol, UefiScsiLib
have same request. Adding EFI_INVALID_PARAMETER return status in
function comments to ask the caller to guarantee this alignment.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
Cc: Star Zeng 
---
 MdePkg/Include/Library/UefiScsiLib.h | 379 ++
 MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 385 +++
 2 files changed, 469 insertions(+), 295 deletions(-)

diff --git a/MdePkg/Include/Library/UefiScsiLib.h 
b/MdePkg/Include/Library/UefiScsiLib.h
index 768a912..26e4aa4 100644
--- a/MdePkg/Include/Library/UefiScsiLib.h
+++ b/MdePkg/Include/Library/UefiScsiLib.h
@@ -5,7 +5,7 @@
   for hard drive, CD and DVD devices that are the most common SCSI boot 
targets used by UEFI platforms.
   This library class depends on SCSI I/O Protocol defined in UEFI 
Specification and SCSI-2 industry standard.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials  
 are licensed and made available under the terms and conditions of the BSD 
License 
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -32,6 +32,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
   If HostAdapterStatus is NULL, then ASSERT().
   If TargetStatus is NULL, then ASSERT().
 
+  If SenseDataLength is non-zero and SenseData is not NULL, SenseData must 
meet buffer
+  alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise 
EFI_INVALID_PARAMETER
+  gets returned.
 
   @param[in] ScsiIo A pointer to the SCSI I/O Protocol instance
 for the specific SCSI target.
@@ -61,27 +64,28 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 Protocol in the UEFI Specification for 
details on
 the possible return values. 
 
-  @retval EFI_SUCCESS  The command was executed successfully.
-   See HostAdapterStatus, TargetStatus, 
SenseDataLength,
-   and SenseData in that order for additional 
status
-   information.
-  @retval EFI_NOT_READYThe SCSI Request Packet could not be sent 
because
-   there are too many SCSI Command Packets already
-   queued. The SCSI Request Packet was not sent, so
-   no additional status information is available.
-   The caller may retry again later.
-  @retval EFI_DEVICE_ERROR A device error occurred while attempting to send
-   SCSI Request Packet.  See HostAdapterStatus,
-   TargetStatus, SenseDataLength, and SenseData in 
that
-   order for additional status information.
-  @retval EFI_UNSUPPORTED  The command described by the SCSI Request Packet
-   is not supported by the SCSI initiator(i.e., 
SCSI
-   Host Controller). The SCSI Request Packet was 
not
-   sent, so no additional status information is 
available.
-  @retval EFI_TIMEOUT  A timeout occurred while waiting for the SCSI 
Request
-   Packet to execute.  See HostAdapterStatus, 
TargetStatus,
-   SenseDataLength, and SenseData in that order for
-   additional status information.
+  @retval EFI_SUCCESS   The command was executed successfully.
+See HostAdapterStatus, TargetStatus, 
SenseDataLength,
+and SenseData in that order for additional 
status
+information.
+  @retval EFI_NOT_READY The SCSI Request Packet could not be sent 
because
+there are too many SCSI Command Packets 
already
+queued. The SCSI Request Packet was not 
sent, so
+no additional status information is 
available.
+The caller may retry again later.
+  @retval EFI_DEVICE_ERROR  A device error occurred while attempting 
to send
+SCSI Request Packet.  See 
HostAdapterStatus,
+TargetStatus, SenseDataLength, and 
SenseData in that
+order for additional status information.
+  @retval EFI_UNSUPPORTED

[edk2] [Patch] MdeModulePkg: Fix a performance data buffer overrun issue

2015-09-08 Thread Ruiyu Ni 
The mBmPerfHeader.Count isn't reset to 0 in BmWriteBootToOsPerformanceData()
so when the actual performance data entry count exceeds the LimitCount,
the performance data collection breaks on condition
if (mBmPerfHeader.Count == LimitCount), but 2nd time calling this function
will not break on condition if (mBmPerfHeader.Count == LimitCount) because
the mBmPerfHeader.Count always bigger than LimitCount, which results buffer
overrun.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Star Zeng 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c
index 7b13ec6..e45c0bd 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmPerformance.c
@@ -186,6 +186,11 @@ BmWriteBootToOsPerformanceData (
   PERF_END(NULL, "BDS", NULL, 0);
 
   //
+  // Reset the entry count
+  //
+  mBmPerfHeader.Count = 0;
+
+  //
   // Retrieve time stamp count as early as possible
   //
   Ticker  = GetPerformanceCounter ();
-- 
1.9.5.msysgit.1

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