Re: [edk2] [MdeModulePkg/TerminalDxe] Why do we delay 2s for ESC being pressed?

[Ni, Ruiyu]

Heyi,

If you check the comments below in TerminalConIn.c:
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c#L1319

TerminalDxe driver needs to determine whether user wants to press ESC alone,
or press "ESC [ V" for F10 (PCANSI terminal).

So a 2 second timeout is added to wait additional keys.

Thanks/Ray

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, November 8, 2017 3:25 PM
> To: Heyi Guo ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Dong, Eric ; Zeng,
> Star 
> Subject: RE: [MdeModulePkg/TerminalDxe] Why do we delay 2s for ESC
> being pressed?
> 
> Cc Terminal expert Ray to see if any comments on this.
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Heyi Guo [mailto:heyi@linaro.org]
> Sent: Wednesday, November 8, 2017 3:04 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric 
> Subject: [MdeModulePkg/TerminalDxe] Why do we delay 2s for ESC being
> pressed?
> 
> Hi folks,
> 
> We found ESC key responded fairly slow on serial port terminal, and we think
> it might be caused by the code in UnicodeToEfiKey in TerminalConIn.c:
> 
>      if (UnicodeChar == ESC) {
>    TerminalDevice->InputState = INPUT_STATE_ESC;
>      }
> 
>      if (UnicodeChar == CSI) {
>    TerminalDevice->InputState = INPUT_STATE_CSI;
>      }
> 
>      if (TerminalDevice->InputState != INPUT_STATE_DEFAULT) {
>    Status = gBS->SetTimer(
>    TerminalDevice->TwoSecondTimeOut,
>    TimerRelative,
>    (UINT64)2000
>    );
>    ASSERT_EFI_ERROR (Status);
>    continue;
>      }
> 
> It seems we intentionally add 2 seconds delay for ESC key press. This
> provides not so good user experience when we press ESC to exit or cancel
> some operation.
> 
> We tried reducing this timeout value to 1 second, then the experience
> improved much and we didn't find any issue introduced.
> 
> What's the reason for this timeout value and is there any improvement?
> 
> Thanks and regards,
> 
> Heyi

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

Re: [edk2] [Patch] MdeModulePkg/TerminalDxe: Fix PCANSI mapping for TRIANGLE and ARROW

[Ni, Ruiyu]

Mike,
I am a bit confused about mapping 0x18 to upper arrow.
I remembered that in old days, pressing ALT+18 in DOS window can generate upper 
arrow.

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, November 8, 2017 12:24 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Zeng, Star 
> Subject: [edk2] [Patch] MdeModulePkg/TerminalDxe: Fix PCANSI mapping
> for TRIANGLE and ARROW
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=761
> 
> When a TerminalType is set to PCANSI, characters in the range 0x00 to 0x1F
> are control characters.  The mapping table for PCANSI maps TRIANGLE glyphs,
> ARROW_UP glyph, and ARROW_DOWN glyph into this control character
> range and that causes no characters to be displayed by PCANSI compatible
> terminal emulators.
> 
> The mappings are updated so these glyphs are mapped to ANSI characters in
> the range 0x20 to 0x7E.
> 
> GEOMETRICSHAPE_UP_TRIANGLE '^'
> GEOMETRICSHAPE_RIGHT_TRIANGLE  '>'
> GEOMETRICSHAPE_DOWN_TRIANGLE   'v'
> GEOMETRICSHAPE_LEFT_TRIANGLE   '<'
> ARROW_UP   '^'
> ARROW_DOWN 'v'
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney 
> ---
>  .../Universal/Console/TerminalDxe/TerminalConOut.c | 18 +
> -
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index e677a76e6b..5a8343162f 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -66,15 +66,15 @@ UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
>{ BLOCKELEMENT_FULL_BLOCK,0xdb, L'*' },
>{ BLOCKELEMENT_LIGHT_SHADE,   0xb0, L'+' },
> 
> -  { GEOMETRICSHAPE_UP_TRIANGLE, 0x1e, L'^' },
> -  { GEOMETRICSHAPE_RIGHT_TRIANGLE,  0x10, L'>' },
> -  { GEOMETRICSHAPE_DOWN_TRIANGLE,   0x1f, L'v' },
> -  { GEOMETRICSHAPE_LEFT_TRIANGLE,   0x11, L'<' },
> -
> -  { ARROW_LEFT, 0x3c, L'<' },
> -  { ARROW_UP,   0x18, L'^' },
> -  { ARROW_RIGHT,0x3e, L'>' },
> -  { ARROW_DOWN, 0x19, L'v' },
> +  { GEOMETRICSHAPE_UP_TRIANGLE, '^', L'^' },
> +  { GEOMETRICSHAPE_RIGHT_TRIANGLE,  '>', L'>' },
> +  { GEOMETRICSHAPE_DOWN_TRIANGLE,   'v', L'v' },
> +  { GEOMETRICSHAPE_LEFT_TRIANGLE,   '<', L'<' },
> +
> +  { ARROW_LEFT, '<', L'<' },
> +  { ARROW_UP,   '^', L'^' },
> +  { ARROW_RIGHT,'>', L'>' },
> +  { ARROW_DOWN, 'v', L'v' },
> 
>{ 0x, 0x00, L'\0' }
>  };
> --
> 2.14.2.windows.3
> 
> ___
> 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] [MdeModulePkg/TerminalDxe] Why do we delay 2s for ESC being pressed?

[Zeng, Star]

Cc Terminal expert Ray to see if any comments on this.


Thanks,
Star
-Original Message-
From: Heyi Guo [mailto:heyi@linaro.org] 
Sent: Wednesday, November 8, 2017 3:04 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric 
Subject: [MdeModulePkg/TerminalDxe] Why do we delay 2s for ESC being pressed?

Hi folks,

We found ESC key responded fairly slow on serial port terminal, and we think it 
might be caused by the code in UnicodeToEfiKey in TerminalConIn.c:

     if (UnicodeChar == ESC) {
   TerminalDevice->InputState = INPUT_STATE_ESC;
     }

     if (UnicodeChar == CSI) {
   TerminalDevice->InputState = INPUT_STATE_CSI;
     }

     if (TerminalDevice->InputState != INPUT_STATE_DEFAULT) {
   Status = gBS->SetTimer(
   TerminalDevice->TwoSecondTimeOut,
   TimerRelative,
   (UINT64)2000
   );
   ASSERT_EFI_ERROR (Status);
   continue;
     }

It seems we intentionally add 2 seconds delay for ESC key press. This provides 
not so good user experience when we press ESC to exit or cancel some operation.

We tried reducing this timeout value to 1 second, then the experience improved 
much and we didn't find any issue introduced.

What's the reason for this timeout value and is there any improvement?

Thanks and regards,

Heyi

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

[edk2] [MdeModulePkg/TerminalDxe] Why do we delay 2s for ESC being pressed?

[Heyi Guo]

Hi folks,

We found ESC key responded fairly slow on serial port terminal, and we 
think it might be caused by the code in UnicodeToEfiKey in TerminalConIn.c:


    if (UnicodeChar == ESC) {
  TerminalDevice->InputState = INPUT_STATE_ESC;
    }

    if (UnicodeChar == CSI) {
  TerminalDevice->InputState = INPUT_STATE_CSI;
    }

    if (TerminalDevice->InputState != INPUT_STATE_DEFAULT) {
  Status = gBS->SetTimer(
  TerminalDevice->TwoSecondTimeOut,
  TimerRelative,
  (UINT64)2000
  );
  ASSERT_EFI_ERROR (Status);
  continue;
    }

It seems we intentionally add 2 seconds delay for ESC key press. This 
provides not so good user experience when we press ESC to exit or cancel 
some operation.


We tried reducing this timeout value to 1 second, then the experience 
improved much and we didn't find any issue introduced.


What's the reason for this timeout value and is there any improvement?

Thanks and regards,

Heyi

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

[edk2] [PATCH v2 1/2] MdePkg/Btt.h: Add Block Translation Table definitions

[Ruiyu Ni]

BTT definitions are defined in UEFI spec 2.7, to defines
a layout and set of rules for doing block I/O that provide
powerfail write atomicity of a single block in NVDIMM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Liming Gao 
---
 MdePkg/Include/Guid/Btt.h | 228 ++
 MdePkg/MdePkg.dec |   6 ++
 2 files changed, 234 insertions(+)
 create mode 100644 MdePkg/Include/Guid/Btt.h

diff --git a/MdePkg/Include/Guid/Btt.h b/MdePkg/Include/Guid/Btt.h
new file mode 100644
index 00..92977259a4
--- /dev/null
+++ b/MdePkg/Include/Guid/Btt.h
@@ -0,0 +1,228 @@
+/** @file
+  Block Translation Table (BTT) metadata layout definition.
+
+  BTT is a layout and set of rules for doing block I/O that provide powerfail
+  write atomicity of a single block.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under
+the terms and conditions of the BSD License 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.
+
+  @par Revision Reference:
+  This metadata layout definition was introduced in UEFI Specification 2.7.
+
+**/
+
+#ifndef _BTT_H_
+#define _BTT_H_
+
+///
+/// The BTT layout and behavior is described by the GUID as below.
+///
+#define EFI_BTT_ABSTRACTION_GUID \
+  { \
+0x18633bfc, 0x1735, 0x4217, { 0x8a, 0xc9, 0x17, 0x23, 0x92, 0x82, 0xd3, 
0xf8 } \
+  }
+
+//
+// Alignment of all BTT structures
+//
+#define EFI_BTT_ALIGNMENT4096
+
+#define EFI_BTT_INFO_UNUSED_LEN  3968
+
+#define EFI_BTT_INFO_BLOCK_SIG_LEN   16
+
+///
+/// Indicate inconsistent metadata or lost metadata due to unrecoverable media 
errors.
+///
+#define EFI_BTT_INFO_BLOCK_FLAGS_ERROR   0x0001
+
+#define EFI_BTT_INFO_BLOCK_MAJOR_VERSION 2
+#define EFI_BTT_INFO_BLOCK_MINOR_VERSION 0
+
+///
+/// Block Translation Table (BTT) Info Block
+///
+typedef struct _EFI_BTT_INFO_BLOCK {
+  ///
+  /// Signature of the BTT Index Block data structure.
+  /// Shall be "BTT_ARENA_INFO\0\0".
+  ///
+  CHAR8Sig[EFI_BTT_INFO_BLOCK_SIG_LEN];
+
+  ///
+  /// UUID identifying this BTT instance.
+  ///
+  GUID Uuid;
+
+  ///
+  /// UUID of containing namespace.
+  ///
+  GUID ParentUuid;
+
+  ///
+  /// Attributes of this BTT Info Block.
+  ///
+  UINT32   Flags;
+
+  ///
+  /// Major version number. Currently at version 2.
+  ///
+  UINT16   Major;
+
+  ///
+  /// Minor version number. Currently at version 0.
+  ///
+  UINT16   Minor;
+
+  ///
+  /// Advertised LBA size in bytes. I/O requests shall be in this size chunk.
+  ///
+  UINT32   ExternalLbaSize;
+
+  ///
+  /// Advertised number of LBAs in this arena.
+  ///
+  UINT32   ExternalNLba;
+
+  ///
+  /// Internal LBA size shall be greater than or equal to ExternalLbaSize and 
shall not be smaller than 512 bytes.
+  ///
+  UINT32   InternalLbaSize;
+
+  ///
+  /// Number of internal blocks in the arena data area.
+  ///
+  UINT32   InternalNLba;
+
+  ///
+  /// Number of free blocks maintained for writes to this arena.
+  ///
+  UINT32   NFree;
+
+  ///
+  /// The size of this info block in bytes.
+  ///
+  UINT32   InfoSize;
+
+  ///
+  /// Offset of next arena, relative to the beginning of this arena.
+  ///
+  UINT64   NextOff;
+
+  ///
+  /// Offset of the data area for this arena, relative to the beginning of 
this arena.
+  ///
+  UINT64   DataOff;
+
+  ///
+  /// Offset of the map for this arena, relative to the beginning of this 
arena.
+  ///
+  UINT64   MapOff;
+
+  ///
+  /// Offset of the flog for this arena, relative to the beginning of this 
arena.
+  ///
+  UINT64   FlogOff;
+
+  ///
+  /// Offset of the backup copy of this arena's info block, relative to the 
beginning of this arena.
+  ///
+  UINT64   InfoOff;
+
+  ///
+  /// Shall be zero.
+  ///
+  CHAR8Unused[EFI_BTT_INFO_UNUSED_LEN];
+
+  ///
+  /// 64-bit Fletcher64 checksum of all fields.
+  ///
+  UINT64   Checksum;
+} EFI_BTT_INFO_BLOCK;
+
+///
+/// BTT Map entry maps an LBA that indexes into the arena, to its actual 
location.
+///
+typedef struct _EFI_BTT_MAP_ENTRY {
+  ///
+  /// Post-map LBA number (block number in this arena's data area)
+  ///
+  UINT32 PostMapLba : 30;
+
+  ///
+  /// When set and Zero is not set, reads on this block return an error.
+  /// When set and Zero is set, indicate a map entry in its normal, non-error 
state.
+  ///
+  UINT32 Error : 1;
+
+  ///
+  /// When set and Error is not set, reads on this block return a full block 
of zeros.
+  /// When set and Error is set, indicate a map entry in its normal, non-error 
state.
+  ///
+  UINT32 Zero : 1;
+} EFI_BTT_MAP_ENTRY;
+
+///
+/// Alignment of each flog structure
+///

[edk2] [PATCH v2 0/2] Add NVDIMM related spec definitions

[Ruiyu Ni]

v2:
  1. Create Btt.h in Include/Guid directory per Liming's comments.
  2. Change the GUID C name to match macro name.

Ruiyu Ni (2):
  MdePkg/Btt.h: Add Block Translation Table definitions
  MdePkg/NvdimmLabel.h: Add NVDIMM_LABEL protocol definition

 MdePkg/Include/Guid/Btt.h | 228 ++
 MdePkg/Include/Protocol/NvdimmLabel.h | 351 ++
 MdePkg/MdePkg.dec |   9 +
 3 files changed, 588 insertions(+)
 create mode 100644 MdePkg/Include/Guid/Btt.h
 create mode 100644 MdePkg/Include/Protocol/NvdimmLabel.h

-- 
2.12.2.windows.2

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

[edk2] [PATCH v2 2/2] MdePkg/NvdimmLabel.h: Add NVDIMM_LABEL protocol definition

[Ruiyu Ni]

NVDIMM_LABEL protocol is defined in UEFI 2.7 spec, to provide
services that allow management of labels contained in a Label
Storage Area in NVDIMM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Liming Gao 
---
 MdePkg/Include/Protocol/NvdimmLabel.h | 351 ++
 MdePkg/MdePkg.dec |   3 +
 2 files changed, 354 insertions(+)
 create mode 100644 MdePkg/Include/Protocol/NvdimmLabel.h

diff --git a/MdePkg/Include/Protocol/NvdimmLabel.h 
b/MdePkg/Include/Protocol/NvdimmLabel.h
new file mode 100644
index 00..0d70bdff74
--- /dev/null
+++ b/MdePkg/Include/Protocol/NvdimmLabel.h
@@ -0,0 +1,351 @@
+/** @file
+  EFI NVDIMM Label Protocol Definition
+
+  The EFI NVDIMM Label Protocol is used to Provides services that allow 
management
+  of labels contained in a Label Storage Area that are associated with a 
specific
+  NVDIMM Device Path.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under
+the terms and conditions of the BSD License 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.
+
+  @par Revision Reference:
+  This Protocol was introduced in UEFI Specification 2.7.
+
+**/
+
+#ifndef __EFI_NVDIMM_LABEL_PROTOCOL_H__
+#define __EFI_NVDIMM_LABEL_PROTOCOL_H__
+
+#define EFI_NVDIMM_LABEL_PROTOCOL_GUID \
+  { \
+0xd40b6b80, 0x97d5, 0x4282, {0xbb, 0x1d, 0x22, 0x3a, 0x16, 0x91, 0x80, 
0x58 } \
+  }
+
+typedef struct _EFI_NVDIMM_LABEL_PROTOCOL EFI_NVDIMM_LABEL_PROTOCOL;
+
+#define EFI_NVDIMM_LABEL_INDEX_SIG_LEN 16
+#define EFI_NVDIMM_LABEL_INDEX_ALIGN   256
+typedef struct {
+  ///
+  /// Signature of the Index Block data structure. Must be "NAMESPACE_INDEX\0".
+  ///
+  CHAR8  Sig[EFI_NVDIMM_LABEL_INDEX_SIG_LEN];
+
+  ///
+  /// Attributes of this Label Storage Area.
+  ///
+  UINT8  Flags[3];
+
+  ///
+  /// Size of each label in bytes, 128 bytes << LabelSize.
+  /// 1 means 256 bytes, 2 means 512 bytes, etc. Shall be 1 or greater.
+  ///
+  UINT8  LabelSize;
+
+  ///
+  /// Sequence number used to identify which of the two Index Blocks is 
current.
+  ///
+  UINT32 Seq;
+
+  ///
+  /// The offset of this Index Block in the Label Storage Area.
+  ///
+  UINT64 MyOff;
+
+  ///
+  /// The size of this Index Block in bytes.
+  /// This field must be a multiple of the EFI_NVDIMM_LABEL_INDEX_ALIGN.
+  ///
+  UINT64 MySize;
+
+  ///
+  /// The offset of the other Index Block paired with this one.
+  ///
+  UINT64 OtherOff;
+
+  ///
+  /// The offset of the first slot where labels are stored in this Label 
Storage Area.
+  ///
+  UINT64 LabelOff;
+
+  ///
+  /// The total number of slots for storing labels in this Label Storage Area.
+  ///
+  UINT32 NSlot;
+
+  ///
+  /// Major version number. Value shall be 1.
+  ///
+  UINT16 Major;
+
+  ///
+  /// Minor version number. Value shall be 2.
+  ///
+  UINT16 Minor;
+
+  ///
+  /// 64-bit Fletcher64 checksum of all fields in this Index Block.
+  ///
+  UINT64 Checksum;
+
+  ///
+  /// Array of unsigned bytes implementing a bitmask that tracks which label 
slots are free.
+  /// A bit value of 0 indicates in use, 1 indicates free.
+  /// The size of this field is the number of bytes required to hold the 
bitmask with NSlot bits,
+  /// padded with additional zero bytes to make the Index Block size a 
multiple of EFI_NVDIMM_LABEL_INDEX_ALIGN.
+  /// Any bits allocated beyond NSlot bits must be zero.
+  ///
+  UINT8  Free[];
+} EFI_NVDIMM_LABEL_INDEX_BLOCK;
+
+#define EFI_NVDIMM_LABEL_NAME_LEN 64
+
+///
+/// The label is read-only.
+///
+#define EFI_NVDIMM_LABEL_FLAGS_ROLABEL 0x0001
+
+///
+/// When set, the complete label set is local to a single NVDIMM Label Storage 
Area.
+/// When clear, the complete label set is contained on multiple NVDIMM Label 
Storage Areas.
+///
+#define EFI_NVDIMM_LABEL_FLAGS_LOCAL 0x0002
+
+///
+/// This reserved flag is utilized on older implementations and has been 
deprecated.
+/// Do not use.
+//
+#define EFI_NVDIMM_LABEL_FLAGS_RESERVED 0x0004
+
+///
+/// When set, the label set is being updated.
+///
+#define EFI_NVDIMM_LABEL_FLAGS_UPDATING 0x0008
+
+typedef struct {
+  ///
+  /// Unique Label Identifier UUID per RFC 4122.
+  ///
+  EFI_GUID Uuid;
+
+  ///
+  /// NULL-terminated string using UTF-8 character formatting.
+  ///
+  CHAR8Name[EFI_NVDIMM_LABEL_NAME_LEN];
+
+  ///
+  /// Attributes of this namespace.
+  ///
+  UINT32   Flags;
+
+  ///
+  /// Total number of labels describing this namespace.
+  ///
+  UINT16   NLabel;
+
+  ///
+  /// Position of this label in list of labels for this namespace.
+  ///
+  UINT16   Position;
+
+  ///
+  /// The SetCookie is 

Re: [edk2] [PATCH 1/2] PcAtChipsetPkg: Define FixePCD's for RTC register values

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Leo Duran [mailto:leo.du...@amd.com]
> Sent: Wednesday, November 1, 2017 1:55 AM
> To: edk2-devel@lists.01.org
> Cc: Leo Duran ; Ni, Ruiyu 
> Subject: [PATCH 1/2] PcAtChipsetPkg: Define FixePCD's for RTC register
> values
> 
> Define FixedPCD's to replace macros in RTC driver, to allow for platform-
> specific configurations.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leo Duran 
> ---
>  PcAtChipsetPkg/PcAtChipsetPkg.dec | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> index b0b2b62..f11d204 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> @@ -5,6 +5,7 @@
>  # PcAt defacto standard.
>  #
>  # Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017, AMD Inc. All rights reserved.
>  #
>  # This program and the accompanying materials  # are licensed and made
> available under the terms and conditions of the BSD License @@ -181,5
> +182,17 @@
># @Prompt Reset Control Register value for cold reset
> 
> gPcAtChipsetPkgTokenSpaceGuid.PcdResetControlValueColdReset|0xFE|UI
> NT8|0x001A
> 
> +  ## Specifies the initial value for Register_A in RTC.
> +  # @Prompt Initial value for Register_A in RTC.
> +
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA|0x26|UINT8|
> 0
> + x001B
> +
> +  ## Specifies the initial value for Register_B in RTC.
> +  # @Prompt Initial value for Register_B in RTC.
> +
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB|0x02|UINT8|
> 0
> + x001C
> +
> +  ## Specifies the initial value for Register_D in RTC.
> +  # @Prompt Initial value for Register_D in RTC.
> +
> +
> gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterD|0x00|UINT8|
> 0
> + x001D
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>PcAtChipsetPkgExtra.uni
> --
> 2.7.4

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

Re: [edk2] [Patch] MdeModulePkg/TerminalDxe: Fix PCANSI mapping for TRIANGLE and ARROW

[Zeng, Star]

Reviewed-by: Star Zeng 

Cc Ray to see if any comments.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Michael 
D Kinney
Sent: Wednesday, November 8, 2017 12:24 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
Subject: [edk2] [Patch] MdeModulePkg/TerminalDxe: Fix PCANSI mapping for 
TRIANGLE and ARROW

https://bugzilla.tianocore.org/show_bug.cgi?id=761

When a TerminalType is set to PCANSI, characters in the range 0x00 to 0x1F are 
control characters.  The mapping table for PCANSI maps TRIANGLE glyphs, 
ARROW_UP glyph, and ARROW_DOWN glyph into this control character range and that 
causes no characters to be displayed by PCANSI compatible terminal emulators.

The mappings are updated so these glyphs are mapped to ANSI characters in the 
range 0x20 to 0x7E.

GEOMETRICSHAPE_UP_TRIANGLE '^'
GEOMETRICSHAPE_RIGHT_TRIANGLE  '>'
GEOMETRICSHAPE_DOWN_TRIANGLE   'v'
GEOMETRICSHAPE_LEFT_TRIANGLE   '<'
ARROW_UP   '^'
ARROW_DOWN 'v'

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 .../Universal/Console/TerminalDxe/TerminalConOut.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c 
b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index e677a76e6b..5a8343162f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -66,15 +66,15 @@ UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
   { BLOCKELEMENT_FULL_BLOCK,0xdb, L'*' },
   { BLOCKELEMENT_LIGHT_SHADE,   0xb0, L'+' },
 
-  { GEOMETRICSHAPE_UP_TRIANGLE, 0x1e, L'^' },
-  { GEOMETRICSHAPE_RIGHT_TRIANGLE,  0x10, L'>' },
-  { GEOMETRICSHAPE_DOWN_TRIANGLE,   0x1f, L'v' },
-  { GEOMETRICSHAPE_LEFT_TRIANGLE,   0x11, L'<' },
-
-  { ARROW_LEFT, 0x3c, L'<' },
-  { ARROW_UP,   0x18, L'^' },
-  { ARROW_RIGHT,0x3e, L'>' },
-  { ARROW_DOWN, 0x19, L'v' },
+  { GEOMETRICSHAPE_UP_TRIANGLE, '^', L'^' },
+  { GEOMETRICSHAPE_RIGHT_TRIANGLE,  '>', L'>' },
+  { GEOMETRICSHAPE_DOWN_TRIANGLE,   'v', L'v' },
+  { GEOMETRICSHAPE_LEFT_TRIANGLE,   '<', L'<' },
+
+  { ARROW_LEFT, '<', L'<' },
+  { ARROW_UP,   '^', L'^' },
+  { ARROW_RIGHT,'>', L'>' },
+  { ARROW_DOWN, 'v', L'v' },
 
   { 0x, 0x00, L'\0' }
 };
--
2.14.2.windows.3

___
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] GRUB issue on device priority

[Haojian Zhuang]

On 2017/11/7 23:14, Vladimir 'phcoder' Serbinenko wrote:



On Tue, Nov 7, 2017, 15:23 Haojian Zhuang > wrote:


Hi all,

It seems there's a device priority issue in GRUB.

Please use mailing list for grub, not just messaging random people some 
of whom have left project 10 years ago.


Sure. Just find the mailing account. Loop grub-de...@gnu.org.




All block io handles are linked into the list in edk2, and GRUB could
fetch it. Then GRUB creates its own ascending on HD priority.

What do you mean by "priority"? It's just disk numbers that don't change 
anything.


Yes, it's disk numbers and it causes an issue.

In edk2, I organize all handles in sequence. For example, eMMC is the 
first BlockIO device, and SD is the second BlockIO device.


In GRUB, it resorts the sequence by ascending order on UUID (device 
path). Since both eMMC and SD controllers are varient of one IP, use the 
same device driver for these two devices. Then UUIDs are in below. GRUB 
names SD with HD0 and eMMC with HD1. The disk numbers are inverted by GRUB.




add_device():
    for (p = devices; *p; p = &((*p)->next)) {
      ret = grub_efi_compare_device_paths
(grub_efi_find_last_device_path
((*p->device_path),
  
grub_efi_find_last_device_path->device_path));

      if (ret == 0) {
        ret = grub_efi_compare_device_paths ((*p)->device_path,
                                             d->device_path);
      }
      if (ret == 0) {
        return;
      } else if (ret > 0) {
        break;
      }
    }
    ...

In the HiKey platform, I prepared the same driver for both eMMC and SD.
So the device paths are in below.
SD: /HardwareVendor(0d51905b-b77e-452a-a2c0-eca0
cc8d514a)[9: 00 e0 23 f7 00 00 00 00 00 ]/UnknownMessaging(1a)/EndEntire
eMMC: /HardwareVendor(0d51905b-b77e-452a-a2c0-eca0
cc8d514a)[9: 00 d0 23 f7 00 00 00 00 00
]/UnknownMessaging(1d)/Ctrl(0)/EndEntire

#define MSG_SD_DP                   0x1A
#define MSG_EMMC_DP                 0x1D

In the second level, the device paths are different.

And GRUB resort the sequence by ascending order (with above code). So SD
device always gets higher priority than eMMC device.

If we always use installer to install OS, it may not an issue. Since
installer could create grub.cfg by itself. But it imports another issue
on lacking of persistent variable storage. And we need to deploy system
without installer on embedded device.

How to fix the priority issue? Fix in GRUB or something else?

You shouldn't rely on any particular GRUB device ordering. The easiest 
way is to use UUID.


I failed to find root by uuid. I don't know why. But I found that I can 
make use of "-hint" with multiple parameters. Then this issue could be 
fixed.


Great thanks for your help.

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

Re: [edk2] [PATCH] Tftp assert fix for openfile failure case

[Udit Kumar]

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Tuesday, November 07, 2017 11:25 PM
> To: Vabhav Sharma 
> Cc: edk2-devel@lists.01.org; ruiyu...@intel.com; jaben.car...@intel.com;
> ard.biesheu...@linaro.org; siyuan...@intel.com; ting...@intel.com; Udit
> Kumar 
> Subject: Re: [PATCH] Tftp assert fix for openfile failure case
> 
> On Fri, Nov 03, 2017 at 07:56:32PM +0530, Vabhav wrote:
> > Issue:
> > when file open is failed, assert was seen due to freeing 0 size page
> >
> > Reason:
> > DataSize is remain zero if error is reported in ShellOpenFileByName
> >
> > Fix:
> > Update DataSize as soon as FileSize is available
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Udit Kumar 
> > Signed-off-by: Vabhav 
> > ---
> >  ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> > b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> > index fbde3bf..6425fc5 100755
> > --- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> > +++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> > @@ -509,6 +509,7 @@ ShellCommandRunTftp (
> >);
> >goto NextHandle;
> 
> Wow, a goto in a foor loop in a 320-line function.
> What could possibly go wrong?

Instead of being on some volume, if you are on Shell. 
Then file open will fail. 
 
> >  }
> > +DataSize = FileSize;
> >
> >  Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, );
> >  if (EFI_ERROR (Status)) {
> > @@ -539,7 +540,6 @@ ShellCommandRunTftp (
> >goto NextHandle;
> >  }
> >
> > -DataSize = FileSize;
> >  Status = ShellWriteFile (FileHandle, , Data);
> >  if (!EFI_ERROR (Status)) {
> >ShellStatus = SHELL_SUCCESS;
> > --
> > 1.9.1
> 
> So, a wider question:
> This shell command was introduced in the heyday of "let's reimplement U-Boot
> in the EDK2 tree". Mainly, from my impression, it seems to be used in order 
> that
> people don't need to learn how boot managers and device paths work.

When you say about complete boot, then this may not be useful. 

> Am I being too harsh?
> Are there practical uses for this?

For doing some sort of unit testing of given interface. I found this useful.
During development, this is useful to transfer generic file to development 
board. 

> Or should we delete it from the tree?
 
 
> If the code is to be kept, I think (from a quick glance) that I would also 
> like to see
> *Data = NULL in the error path of DownloadFile().
> 
> /
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: fix memory override bug

[Heyi Guo]

Thanks,

Heyi


在 11/8/2017 12:53 PM, Zeng, Star 写道:

Just pushed at 710d9e69fae6753a1a826aa18dd37bcadd3e0c3e.

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi Guo
Sent: Tuesday, November 7, 2017 5:33 PM
To: Ard Biesheuvel 
Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Dong, Eric 
; Zeng, Star ; linaro-uefi 

Subject: Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: fix memory override 
bug

Hi Ray,

We had Ard's R-B already; could you help to commit it?

Thanks and regards,

Heyi


在 10/30/2017 4:14 PM, Ard Biesheuvel 写道:

On 30 October 2017 at 05:47, Heyi Guo  wrote:

For PciIoPciRead interface, memory prior to Buffer would be written
with zeros if Offset was larger than sizeof (Dev->ConfigSpace), which
would cause serious system exception.

So we add a pre-check branch to avoid memory override.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 

Reviewed-by: Ard Biesheuvel 


---
   .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c | 5 
+
   1 file changed, 5 insertions(+)

diff --git
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
DeviceIo.c
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
DeviceIo.c
index c836ad6..0e42ae4 100644
---
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
DeviceIo.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverabl
+++ ePciDeviceIo.c
@@ -465,6 +465,11 @@ PciIoPciRead (
 Address = (UINT8 *)>ConfigSpace + Offset;
 Length = Count << ((UINTN)Width & 0x3);

+  if (Offset >= sizeof (Dev->ConfigSpace)) {
+ZeroMem (Buffer, Length);
+return EFI_SUCCESS;
+  }
+
 if (Offset + Length > sizeof (Dev->ConfigSpace)) {
   //
   // Read all zeroes for config space accesses beyond the first
--
1.9.1


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


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

Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: fix memory override bug

[Zeng, Star]

Just pushed at 710d9e69fae6753a1a826aa18dd37bcadd3e0c3e.

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi Guo
Sent: Tuesday, November 7, 2017 5:33 PM
To: Ard Biesheuvel 
Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Dong, Eric 
; Zeng, Star ; linaro-uefi 

Subject: Re: [edk2] [PATCH] MdeModulePkg/NonDiscoverable: fix memory override 
bug

Hi Ray,

We had Ard's R-B already; could you help to commit it?

Thanks and regards,

Heyi


在 10/30/2017 4:14 PM, Ard Biesheuvel 写道:
> On 30 October 2017 at 05:47, Heyi Guo  wrote:
>> For PciIoPciRead interface, memory prior to Buffer would be written 
>> with zeros if Offset was larger than sizeof (Dev->ConfigSpace), which 
>> would cause serious system exception.
>>
>> So we add a pre-check branch to avoid memory override.
>>
>> Cc: Star Zeng 
>> Cc: Eric Dong 
>> Cc: Ard Biesheuvel 
>> Cc: Ruiyu Ni 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Heyi Guo 
> Reviewed-by: Ard Biesheuvel 
>
>> ---
>>   .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c | 5 
>> +
>>   1 file changed, 5 insertions(+)
>>
>> diff --git 
>> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
>> DeviceIo.c 
>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
>> DeviceIo.c
>> index c836ad6..0e42ae4 100644
>> --- 
>> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
>> DeviceIo.c
>> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverabl
>> +++ ePciDeviceIo.c
>> @@ -465,6 +465,11 @@ PciIoPciRead (
>> Address = (UINT8 *)>ConfigSpace + Offset;
>> Length = Count << ((UINTN)Width & 0x3);
>>
>> +  if (Offset >= sizeof (Dev->ConfigSpace)) {
>> +ZeroMem (Buffer, Length);
>> +return EFI_SUCCESS;
>> +  }
>> +
>> if (Offset + Length > sizeof (Dev->ConfigSpace)) {
>>   //
>>   // Read all zeroes for config space accesses beyond the first
>> --
>> 1.9.1
>>

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

Re: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

[Wang, Jian J]

Make sense. Thanks for the comment.

> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, November 08, 2017 12:42 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Yao, Jiewen ;
> Dong, Eric 
> Subject: RE: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of
> RT_CODE in memory map
> 
> Jian,
> Can you add more comments to explain why changing the capabilities of GCD
> entry?
> 
> The background is clear by checking the Bugzilla. But it would be great to 
> know
> the issue
> by just reading the code.
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Jian J Wang
> > Sent: Friday, November 3, 2017 8:57 AM
> > To: edk2-devel@lists.01.org
> > Cc: Laszlo Ersek ; Yao, Jiewen ;
> > Dong, Eric 
> > Subject: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of
> > RT_CODE in memory map
> >
> > > v2
> > > a. Fix an issue which will cause setting capability failure if size is 
> > > smaller
> > >than a page.
> >
> > More than one entry of RT_CODE memory might cause boot problem for
> > some old OSs. This patch will fix this issue to keep OS compatibility as 
> > much as
> > possible.
> >
> > More detailed information, please refer to
> > https://bugzilla.tianocore.org/show_bug.cgi?id=753
> >
> > Cc: Eric Dong 
> > Cc: Jiewen Yao 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang 
> > ---
> >  UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 ++
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > index d312eb66f8..4a7827ebc9 100644
> > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > @@ -809,7 +809,9 @@ RefreshGcdMemoryAttributesFromPaging (
> >PageLength= 0;
> >
> >for (Index = 0; Index < NumberOfDescriptors; Index++) {
> > -if (MemorySpaceMap[Index].GcdMemoryType ==
> > EfiGcdMemoryTypeNonExistent) {
> > +if (MemorySpaceMap[Index].GcdMemoryType ==
> > EfiGcdMemoryTypeNonExistent
> > +|| (MemorySpaceMap[Index].BaseAddress & EFI_PAGE_MASK) != 0
> > +|| (MemorySpaceMap[Index].Length & EFI_PAGE_MASK) != 0) {
> >continue;
> >  }
> >
> > @@ -829,6 +831,15 @@ RefreshGcdMemoryAttributesFromPaging (
> >  // Sync real page attributes to GCD
> >  BaseAddress   = MemorySpaceMap[Index].BaseAddress;
> >  MemorySpaceLength = MemorySpaceMap[Index].Length;
> > +Capabilities  = MemorySpaceMap[Index].Capabilities |
> > +EFI_MEMORY_PAGETYPE_MASK;
> 
> 
> > +Status = gDS->SetMemorySpaceCapabilities (
> > +BaseAddress,
> > +MemorySpaceLength,
> > +Capabilities
> > +);
> > +ASSERT_EFI_ERROR (Status);
> > +
> >  while (MemorySpaceLength > 0) {
> >if (PageLength == 0) {
> >  PageEntry = GetPageTableEntry (, BaseAddress,
> > ); @@ -846,7 +857,6 @@
> > RefreshGcdMemoryAttributesFromPaging (
> >  if (Attributes != (MemorySpaceMap[Index].Attributes &
> > EFI_MEMORY_PAGETYPE_MASK)) {
> >DoUpdate = TRUE;
> >Attributes |= (MemorySpaceMap[Index].Attributes &
> > ~EFI_MEMORY_PAGETYPE_MASK);
> > -  Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
> >  } else {
> >DoUpdate = FALSE;
> >  }
> > @@ -854,8 +864,8 @@ RefreshGcdMemoryAttributesFromPaging (
> >
> >Length = MIN (PageLength, MemorySpaceLength);
> >if (DoUpdate) {
> > -gDS->SetMemorySpaceCapabilities (BaseAddress, Length, 
> > Capabilities);
> > -gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
> > +Status = gDS->SetMemorySpaceAttributes (BaseAddress, Length,
> > Attributes);
> > +ASSERT_EFI_ERROR (Status);
> >  DEBUG ((DEBUG_INFO, "Update memory space attribute:
> > [%02d] %016lx - %016lx (%08lx -> %08lx)\r\n",
> >   Index, BaseAddress, BaseAddress + Length - 1,
> >   MemorySpaceMap[Index].Attributes, 
> > Attributes));
> > --
> > 2.14.1.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

[Ni, Ruiyu]

Jian,
Can you add more comments to explain why changing the capabilities of GCD entry?

The background is clear by checking the Bugzilla. But it would be great to know 
the issue
by just reading the code.

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jian J Wang
> Sent: Friday, November 3, 2017 8:57 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Yao, Jiewen ;
> Dong, Eric 
> Subject: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of
> RT_CODE in memory map
> 
> > v2
> > a. Fix an issue which will cause setting capability failure if size is 
> > smaller
> >than a page.
> 
> More than one entry of RT_CODE memory might cause boot problem for
> some old OSs. This patch will fix this issue to keep OS compatibility as much 
> as
> possible.
> 
> More detailed information, please refer to
> https://bugzilla.tianocore.org/show_bug.cgi?id=753
> 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index d312eb66f8..4a7827ebc9 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -809,7 +809,9 @@ RefreshGcdMemoryAttributesFromPaging (
>PageLength= 0;
> 
>for (Index = 0; Index < NumberOfDescriptors; Index++) {
> -if (MemorySpaceMap[Index].GcdMemoryType ==
> EfiGcdMemoryTypeNonExistent) {
> +if (MemorySpaceMap[Index].GcdMemoryType ==
> EfiGcdMemoryTypeNonExistent
> +|| (MemorySpaceMap[Index].BaseAddress & EFI_PAGE_MASK) != 0
> +|| (MemorySpaceMap[Index].Length & EFI_PAGE_MASK) != 0) {
>continue;
>  }
> 
> @@ -829,6 +831,15 @@ RefreshGcdMemoryAttributesFromPaging (
>  // Sync real page attributes to GCD
>  BaseAddress   = MemorySpaceMap[Index].BaseAddress;
>  MemorySpaceLength = MemorySpaceMap[Index].Length;
> +Capabilities  = MemorySpaceMap[Index].Capabilities |
> +EFI_MEMORY_PAGETYPE_MASK;


> +Status = gDS->SetMemorySpaceCapabilities (
> +BaseAddress,
> +MemorySpaceLength,
> +Capabilities
> +);
> +ASSERT_EFI_ERROR (Status);
> +
>  while (MemorySpaceLength > 0) {
>if (PageLength == 0) {
>  PageEntry = GetPageTableEntry (, BaseAddress,
> ); @@ -846,7 +857,6 @@
> RefreshGcdMemoryAttributesFromPaging (
>  if (Attributes != (MemorySpaceMap[Index].Attributes &
> EFI_MEMORY_PAGETYPE_MASK)) {
>DoUpdate = TRUE;
>Attributes |= (MemorySpaceMap[Index].Attributes &
> ~EFI_MEMORY_PAGETYPE_MASK);
> -  Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
>  } else {
>DoUpdate = FALSE;
>  }
> @@ -854,8 +864,8 @@ RefreshGcdMemoryAttributesFromPaging (
> 
>Length = MIN (PageLength, MemorySpaceLength);
>if (DoUpdate) {
> -gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
> -gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
> +Status = gDS->SetMemorySpaceAttributes (BaseAddress, Length,
> Attributes);
> +ASSERT_EFI_ERROR (Status);
>  DEBUG ((DEBUG_INFO, "Update memory space attribute:
> [%02d] %016lx - %016lx (%08lx -> %08lx)\r\n",
>   Index, BaseAddress, BaseAddress + Length - 1,
>   MemorySpaceMap[Index].Attributes, Attributes));
> --
> 2.14.1.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] MdeModulePkg/TerminalDxe: Fix PCANSI mapping for TRIANGLE and ARROW

[Michael D Kinney]

https://bugzilla.tianocore.org/show_bug.cgi?id=761

When a TerminalType is set to PCANSI, characters in the range 0x00
to 0x1F are control characters.  The mapping table for PCANSI maps
TRIANGLE glyphs, ARROW_UP glyph, and ARROW_DOWN glyph into this
control character range and that causes no characters to be
displayed by PCANSI compatible terminal emulators.

The mappings are updated so these glyphs are mapped to ANSI
characters in the range 0x20 to 0x7E.

GEOMETRICSHAPE_UP_TRIANGLE '^'
GEOMETRICSHAPE_RIGHT_TRIANGLE  '>'
GEOMETRICSHAPE_DOWN_TRIANGLE   'v'
GEOMETRICSHAPE_LEFT_TRIANGLE   '<'
ARROW_UP   '^'
ARROW_DOWN 'v'

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 .../Universal/Console/TerminalDxe/TerminalConOut.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c 
b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index e677a76e6b..5a8343162f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -66,15 +66,15 @@ UNICODE_TO_CHAR  UnicodeToPcAnsiOrAscii[] = {
   { BLOCKELEMENT_FULL_BLOCK,0xdb, L'*' },
   { BLOCKELEMENT_LIGHT_SHADE,   0xb0, L'+' },
 
-  { GEOMETRICSHAPE_UP_TRIANGLE, 0x1e, L'^' },
-  { GEOMETRICSHAPE_RIGHT_TRIANGLE,  0x10, L'>' },
-  { GEOMETRICSHAPE_DOWN_TRIANGLE,   0x1f, L'v' },
-  { GEOMETRICSHAPE_LEFT_TRIANGLE,   0x11, L'<' },
-
-  { ARROW_LEFT, 0x3c, L'<' },
-  { ARROW_UP,   0x18, L'^' },
-  { ARROW_RIGHT,0x3e, L'>' },
-  { ARROW_DOWN, 0x19, L'v' },
+  { GEOMETRICSHAPE_UP_TRIANGLE, '^', L'^' },
+  { GEOMETRICSHAPE_RIGHT_TRIANGLE,  '>', L'>' },
+  { GEOMETRICSHAPE_DOWN_TRIANGLE,   'v', L'v' },
+  { GEOMETRICSHAPE_LEFT_TRIANGLE,   '<', L'<' },
+
+  { ARROW_LEFT, '<', L'<' },
+  { ARROW_UP,   '^', L'^' },
+  { ARROW_RIGHT,'>', L'>' },
+  { ARROW_DOWN, 'v', L'v' },
 
   { 0x, 0x00, L'\0' }
 };
-- 
2.14.2.windows.3

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

Re: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

[Zeng, Star]

https://bugzilla.tianocore.org/show_bug.cgi?id=763 is submitted to track this.


Thanks,
Star
-Original Message-
From: Wang, Jian J 
Sent: Tuesday, November 7, 2017 8:55 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Laszlo Ersek ; Yao, Jiewen ; Dong, 
Eric 
Subject: RE: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of 
RT_CODE in memory map

Thanks for the review. And I agree that GCD.SetMemoryAttributes should be used 
all the time in DxeCore. Let's fix it in another patch.

> -Original Message-
> From: Zeng, Star
> Sent: Monday, November 06, 2017 5:16 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Yao, Jiewen 
> ; Dong, Eric ; Zeng, Star 
> 
> Subject: RE: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries 
> of RT_CODE in memory map
> 
> I am ok to this code approach.
> 
> Acknowledged-by: Star Zeng 
> 
> Besides, I think except GCD services, DxeCore should not call gCpu-
> >SetMemoryAttributes() directly, for example 
> >ApplyMemoryProtectionPolicy(), it
> should have no assumption of the CPU code (to set capabilities), and 
> call GCD setcapabilities first, and then call GCD setattributes since
> 14dde9e903bb9a719ebb8f3381da72b19509bc36 "MdeModulePkg/Core: Fix out- 
> of-sync issue in GCD", otherwise there may be mismatch of page 
> attributes between GCD and gCPU after 
> RefreshGcdMemoryAttributesFromPaging() by the calling 
> gCpu->SetMemoryAttributes() in ApplyMemoryProtectionPolicy().
> 
> Anyway, that could be in a separated patch. :)
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Jian J Wang
> Sent: Friday, November 3, 2017 8:57 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Yao, Jiewen 
> ; Dong, Eric 
> Subject: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of 
> RT_CODE in memory map
> 
> > v2
> > a. Fix an issue which will cause setting capability failure if size is 
> > smaller
> >than a page.
> 
> More than one entry of RT_CODE memory might cause boot problem for 
> some old OSs. This patch will fix this issue to keep OS compatibility as much 
> as possible.
> 
> More detailed information, please refer to
> https://bugzilla.tianocore.org/show_bug.cgi?id=753
> 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index d312eb66f8..4a7827ebc9 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -809,7 +809,9 @@ RefreshGcdMemoryAttributesFromPaging (
>PageLength= 0;
> 
>for (Index = 0; Index < NumberOfDescriptors; Index++) {
> -if (MemorySpaceMap[Index].GcdMemoryType ==
> EfiGcdMemoryTypeNonExistent) {
> +if (MemorySpaceMap[Index].GcdMemoryType ==
> EfiGcdMemoryTypeNonExistent
> +|| (MemorySpaceMap[Index].BaseAddress & EFI_PAGE_MASK) != 0
> +|| (MemorySpaceMap[Index].Length & EFI_PAGE_MASK) != 0) {
>continue;
>  }
> 
> @@ -829,6 +831,15 @@ RefreshGcdMemoryAttributesFromPaging (
>  // Sync real page attributes to GCD
>  BaseAddress   = MemorySpaceMap[Index].BaseAddress;
>  MemorySpaceLength = MemorySpaceMap[Index].Length;
> +Capabilities  = MemorySpaceMap[Index].Capabilities |
> +EFI_MEMORY_PAGETYPE_MASK;
> +Status = gDS->SetMemorySpaceCapabilities (
> +BaseAddress,
> +MemorySpaceLength,
> +Capabilities
> +);
> +ASSERT_EFI_ERROR (Status);
> +
>  while (MemorySpaceLength > 0) {
>if (PageLength == 0) {
>  PageEntry = GetPageTableEntry (, BaseAddress, 
> ); @@ -846,7 +857,6 @@ 
> RefreshGcdMemoryAttributesFromPaging (
>  if (Attributes != (MemorySpaceMap[Index].Attributes &
> EFI_MEMORY_PAGETYPE_MASK)) {
>DoUpdate = TRUE;
>Attributes |= (MemorySpaceMap[Index].Attributes & 
> ~EFI_MEMORY_PAGETYPE_MASK);
> -  Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
>  } else {
>DoUpdate = FALSE;
>  }
> @@ -854,8 +864,8 @@ RefreshGcdMemoryAttributesFromPaging (
> 
>Length = MIN (PageLength, MemorySpaceLength);
>if (DoUpdate) {
> -gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
> -gDS->SetMemorySpaceAttributes (BaseAddress, 

Re: [edk2] [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool

[Zeng, Star]

Fine, https://bugzilla.tianocore.org/show_bug.cgi?id=762 is submitted.

Reviewed-by: Star Zeng 

-Original Message-
From: Wang, Jian J 
Sent: Wednesday, November 8, 2017 10:46 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Bi, Dandan 
Subject: RE: [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool

Hi Star,

I agree the issues you mentioned. But they're already there before this patch.
I'd suggest to file a new bug tracker for them instead of fixing them in this 
one.

Thanks,
Jian

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, November 08, 2017 10:38 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Bi, Dandan 
> ; Zeng, Star 
> Subject: RE: [PATCH v3 1/3] MdeModulePkg: Fix misuses of 
> AllocateCopyPool
> 
> In FrontPageCustomizedUiSupport.c, suggest to use "(CurrentSize + 
> UI_HII_DRIVER_LIST_SIZE)" instead of "(Count + 
> UI_HII_DRIVER_LIST_SIZE)" to be consistent with the following code 
> "CurrentSize += UI_HII_DRIVER_LIST_SIZE".
> 
> Same comment to BootMaintenanceManagerCustomizedUiSupport.c
> 
> In HiiLib.c, suggest removing "FormSetBuffer = NULL".
> And the code logic below in HiiLib.c is strange and suggest refining the code.
> TempBuffer = AllocatePool (TempSize + ((EFI_IFR_OP_HEADER *) 
> OpCodeData)-
> >Length);
> ...
> CopyMem (TempBuffer, OpCodeData, ((EFI_IFR_OP_HEADER *) OpCodeData)-
> >Length);
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, November 8, 2017 10:12 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric 
> ; Bi, Dandan 
> Subject: [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool
> 
> >v3:
> > a. Add ASSERT for returned pointer
> > b. Correct DestMax parameter in calling StrCpyS  c. Fix coding style
> 
> >v2:
> > a. Use ReallocatePool to replace AllocateCopyPool wherever applicable.
> 
> AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" 
> bytes of memory from old "Buffer" to new allocated one. If 
> "AllocationSize" is bigger than size of "Buffer", heap memory overflow occurs 
> during copy.
> 
> One solution is to allocate pool first then copy the necessary bytes 
> to new memory. Another is using ReallocatePool instead if old buffer 
> will be freed on spot.
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Bi Dandan 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  .../Application/UiApp/FrontPageCustomizedUiSupport.c |  8 ++--
>  .../BootMaintenanceManagerCustomizedUiSupport.c  |  8 ++--
>  MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c  | 10 +--
> ---
>  MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 12 
> 
>  .../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c |  3 ++-
>  MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c |  9
> ++---
>  6 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git 
> a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> index 1505ef9319..17fc3db507 100644
> --- a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> +++ b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> @@ -639,9 +639,13 @@ UiListThirdPartyDrivers (
> 
>  Count++;
>  if (Count >= CurrentSize) {
> -  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) *
> sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
> +  DriverListPtr = ReallocatePool (
> +CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
> +(Count + UI_HII_DRIVER_LIST_SIZE)
> +  * sizeof (UI_HII_DRIVER_INSTANCE),
> +gHiiDriverList
> +);
>ASSERT (DriverListPtr != NULL);
> -  FreePool (gHiiDriverList);
>gHiiDriverList = DriverListPtr;
>CurrentSize += UI_HII_DRIVER_LIST_SIZE;
>  }
> diff --git
> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> index b25bc67c06..6dd4fce139 100644
> ---
> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> +++
> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> @@ -435,9 +435,13 @@ BmmListThirdPartyDrivers (
> 
>  Count++;
>  if (Count >= CurrentSize) {
> -  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) *
> sizeof 

Re: [edk2] [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool

[Wang, Jian J]

Hi Star,

I agree the issues you mentioned. But they're already there before this patch.
I'd suggest to file a new bug tracker for them instead of fixing them in this 
one.

Thanks,
Jian

> -Original Message-
> From: Zeng, Star
> Sent: Wednesday, November 08, 2017 10:38 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Bi, Dandan ;
> Zeng, Star 
> Subject: RE: [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool
> 
> In FrontPageCustomizedUiSupport.c, suggest to use "(CurrentSize +
> UI_HII_DRIVER_LIST_SIZE)" instead of "(Count + UI_HII_DRIVER_LIST_SIZE)" to
> be consistent with the following code "CurrentSize +=
> UI_HII_DRIVER_LIST_SIZE".
> 
> Same comment to BootMaintenanceManagerCustomizedUiSupport.c
> 
> In HiiLib.c, suggest removing "FormSetBuffer = NULL".
> And the code logic below in HiiLib.c is strange and suggest refining the code.
> TempBuffer = AllocatePool (TempSize + ((EFI_IFR_OP_HEADER *) OpCodeData)-
> >Length);
> ...
> CopyMem (TempBuffer, OpCodeData, ((EFI_IFR_OP_HEADER *) OpCodeData)-
> >Length);
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, November 8, 2017 10:12 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric ; Bi,
> Dandan 
> Subject: [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool
> 
> >v3:
> > a. Add ASSERT for returned pointer
> > b. Correct DestMax parameter in calling StrCpyS
> > c. Fix coding style
> 
> >v2:
> > a. Use ReallocatePool to replace AllocateCopyPool wherever applicable.
> 
> AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
> memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
> than size of "Buffer", heap memory overflow occurs during copy.
> 
> One solution is to allocate pool first then copy the necessary bytes to new
> memory. Another is using ReallocatePool instead if old buffer will be freed
> on spot.
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Bi Dandan 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  .../Application/UiApp/FrontPageCustomizedUiSupport.c |  8 ++--
>  .../BootMaintenanceManagerCustomizedUiSupport.c  |  8 ++--
>  MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c  | 10 +--
> ---
>  MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 12 
> 
>  .../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c |  3 ++-
>  MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c |  9
> ++---
>  6 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> index 1505ef9319..17fc3db507 100644
> --- a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> +++ b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
> @@ -639,9 +639,13 @@ UiListThirdPartyDrivers (
> 
>  Count++;
>  if (Count >= CurrentSize) {
> -  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) *
> sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
> +  DriverListPtr = ReallocatePool (
> +CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
> +(Count + UI_HII_DRIVER_LIST_SIZE)
> +  * sizeof (UI_HII_DRIVER_INSTANCE),
> +gHiiDriverList
> +);
>ASSERT (DriverListPtr != NULL);
> -  FreePool (gHiiDriverList);
>gHiiDriverList = DriverListPtr;
>CurrentSize += UI_HII_DRIVER_LIST_SIZE;
>  }
> diff --git
> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> index b25bc67c06..6dd4fce139 100644
> ---
> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> +++
> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceM
> anagerCustomizedUiSupport.c
> @@ -435,9 +435,13 @@ BmmListThirdPartyDrivers (
> 
>  Count++;
>  if (Count >= CurrentSize) {
> -  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) *
> sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
> +  DriverListPtr = ReallocatePool (
> +CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
> +(Count + UI_HII_DRIVER_LIST_SIZE)
> +* sizeof (UI_HII_DRIVER_INSTANCE),
> +gHiiDriverList
> +);
>ASSERT (DriverListPtr != NULL);
> -  FreePool (gHiiDriverList);
>

Re: [edk2] [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool

[Zeng, Star]

In FrontPageCustomizedUiSupport.c, suggest to use "(CurrentSize + 
UI_HII_DRIVER_LIST_SIZE)" instead of "(Count + UI_HII_DRIVER_LIST_SIZE)" to be 
consistent with the following code "CurrentSize += UI_HII_DRIVER_LIST_SIZE".

Same comment to BootMaintenanceManagerCustomizedUiSupport.c

In HiiLib.c, suggest removing "FormSetBuffer = NULL".
And the code logic below in HiiLib.c is strange and suggest refining the code.
TempBuffer = AllocatePool (TempSize + ((EFI_IFR_OP_HEADER *) 
OpCodeData)->Length);
...
CopyMem (TempBuffer, OpCodeData, ((EFI_IFR_OP_HEADER *) OpCodeData)->Length);


Thanks,
Star
-Original Message-
From: Wang, Jian J 
Sent: Wednesday, November 8, 2017 10:12 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric ; Bi, 
Dandan 
Subject: [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool

>v3:
> a. Add ASSERT for returned pointer
> b. Correct DestMax parameter in calling StrCpyS
> c. Fix coding style

>v2:
> a. Use ReallocatePool to replace AllocateCopyPool wherever applicable.

AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
than size of "Buffer", heap memory overflow occurs during copy.

One solution is to allocate pool first then copy the necessary bytes to new
memory. Another is using ReallocatePool instead if old buffer will be freed
on spot.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 .../Application/UiApp/FrontPageCustomizedUiSupport.c |  8 ++--
 .../BootMaintenanceManagerCustomizedUiSupport.c  |  8 ++--
 MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c  | 10 +-
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 12 
 .../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c |  3 ++-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c |  9 ++---
 6 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c 
b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
index 1505ef9319..17fc3db507 100644
--- a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
+++ b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
@@ -639,9 +639,13 @@ UiListThirdPartyDrivers (
 
 Count++;
 if (Count >= CurrentSize) {
-  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) * 
sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
+  DriverListPtr = ReallocatePool (
+CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
+(Count + UI_HII_DRIVER_LIST_SIZE)
+  * sizeof (UI_HII_DRIVER_INSTANCE),
+gHiiDriverList
+);
   ASSERT (DriverListPtr != NULL);
-  FreePool (gHiiDriverList);
   gHiiDriverList = DriverListPtr;
   CurrentSize += UI_HII_DRIVER_LIST_SIZE;
 }
diff --git 
a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
index b25bc67c06..6dd4fce139 100644
--- 
a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
+++ 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
@@ -435,9 +435,13 @@ BmmListThirdPartyDrivers (
 
 Count++;
 if (Count >= CurrentSize) {
-  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) * 
sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
+  DriverListPtr = ReallocatePool (
+CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
+(Count + UI_HII_DRIVER_LIST_SIZE)
+* sizeof (UI_HII_DRIVER_INSTANCE),
+gHiiDriverList
+);
   ASSERT (DriverListPtr != NULL);
-  FreePool (gHiiDriverList);
   gHiiDriverList = DriverListPtr;
   CurrentSize += UI_HII_DRIVER_LIST_SIZE;
 }
diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c 
b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
index 23ae6c5392..ac8a975bf6 100644
--- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
+++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
@@ -240,7 +240,11 @@ AddIdToMacDeviceList (
   } else {
 mMacDeviceList.MaxListLen += MAX_MAC_ADDRESS_NODE_LIST_LEN;
 if (mMacDeviceList.CurListLen != 0) {
-  TempDeviceList = (MENU_INFO_ITEM *)AllocateCopyPool (sizeof 
(MENU_INFO_ITEM) * mMacDeviceList.MaxListLen, (VOID *)mMacDeviceList.NodeList);
+  TempDeviceList = ReallocatePool (
+   

Re: [edk2] [PATCH v3 3/3] IntelFrameworkModulePkg: Fix misuses of AllocateCopyPool

[Bi, Dandan]

Reviewed-by: Dandan Bi 

Thanks,
Dandan

-Original Message-
From: Wang, Jian J 
Sent: Wednesday, November 8, 2017 10:12 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Bi, Dandan 
Subject: [PATCH v3 3/3] IntelFrameworkModulePkg: Fix misuses of AllocateCopyPool

> v3:
>No updates.

> v2:
> a. Use ReallocatePool instead of allocating then copying wherever 
> applicable

AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of 
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger 
than size of "Buffer", heap memory overflow occurs during copy.

One solution is to allocate pool first then copy the necessary bytes to new 
memory. Another is using ReallocatePool instead if old buffer will be freed on 
spot.

Cc: Liming Gao 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 .../Universal/BdsDxe/DeviceMngr/DeviceManager.c| 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git 
a/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
index 125c49db5e..5103c7e5d1 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.
+++ c
@@ -480,7 +480,11 @@ AddIdToMacDeviceList (
   } else {
 mMacDeviceList.MaxListLen += MAX_MAC_ADDRESS_NODE_LIST_LEN;
 if (mMacDeviceList.CurListLen != 0) {
-  TempDeviceList = (MENU_INFO_ITEM *)AllocateCopyPool (sizeof 
(MENU_INFO_ITEM) * mMacDeviceList.MaxListLen, (VOID *)mMacDeviceList.NodeList);
+  TempDeviceList = ReallocatePool (
+ sizeof (MENU_INFO_ITEM) * mMacDeviceList.CurListLen,
+ sizeof (MENU_INFO_ITEM) * mMacDeviceList.MaxListLen,
+ mMacDeviceList.NodeList
+ );
 } else {
   TempDeviceList = (MENU_INFO_ITEM *)AllocatePool (sizeof (MENU_INFO_ITEM) 
* mMacDeviceList.MaxListLen);
 }
@@ -491,10 +495,6 @@ AddIdToMacDeviceList (
 TempDeviceList[mMacDeviceList.CurListLen].PromptId = PromptId;  
 TempDeviceList[mMacDeviceList.CurListLen].QuestionId = (EFI_QUESTION_ID) 
(mMacDeviceList.CurListLen + NETWORK_DEVICE_LIST_KEY_OFFSET);
 
-if (mMacDeviceList.CurListLen > 0) {
-  FreePool(mMacDeviceList.NodeList);
-}
-
 mMacDeviceList.NodeList = TempDeviceList;
   }
   mMacDeviceList.CurListLen ++;
--
2.14.1.windows.1

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

[edk2] Why do we use -g option of gcc even for RELEASE build?

[Heyi Guo]

Hi folks,

From gcc manual, -g option seems to produce debugging information. In 
tools_def.template, -g is included in GCC_ALL_CC_FLAGS, so it will also 
be enabled for RELEASE build with gcc tool chain. Any special reason to 
do that?


Thanks and regards,

Heyi

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

[edk2] [PATCH v3 2/3] ShellPkg: Fix misuses of AllocateCopyPool

[Jian J Wang]

> v3:
>   No update

> v2:
> a. Use ReallocatePool instead of allocating then copying wherever applicable

AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
than size of "Buffer", heap memory overflow occurs during copy.

One solution is to allocate pool first then copy the necessary bytes to new
memory. Another is using ReallocatePool instead if old buffer will be freed
on spot.

Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 ShellPkg/Application/Shell/Shell.c | 4 +++-
 ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c | 7 +--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index 5471930ba1..656206fdce 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1646,7 +1646,7 @@ ShellConvertVariables (
   //
   // now do the replacements...
   //
-  NewCommandLine1 = AllocateCopyPool(NewSize, OriginalCommandLine);
+  NewCommandLine1 = AllocateZeroPool (NewSize);
   NewCommandLine2 = AllocateZeroPool(NewSize);
   ItemTemp= AllocateZeroPool(ItemSize+(2*sizeof(CHAR16)));
   if (NewCommandLine1 == NULL || NewCommandLine2 == NULL || ItemTemp == NULL) {
@@ -1655,6 +1655,8 @@ ShellConvertVariables (
 SHELL_FREE_NON_NULL(ItemTemp);
 return (NULL);
   }
+  CopyMem (NewCommandLine1, OriginalCommandLine, StrSize 
(OriginalCommandLine));
+
   for (MasterEnvList = EfiShellGetEnv(NULL)
 ;  MasterEnvList != NULL && *MasterEnvList != CHAR_NULL
 ;  MasterEnvList += StrLen(MasterEnvList) + 1
diff --git a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c 
b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
index 1122c89b8b..ee3db63358 100644
--- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
+++ b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
@@ -143,10 +143,11 @@ UpdateOptionalData(
 OriginalOptionDataSize += (*(UINT16*)(OriginalData + sizeof(UINT32)));
 OriginalOptionDataSize -= OriginalSize;
 NewSize = OriginalSize - OriginalOptionDataSize + DataSize;
-NewData = AllocateCopyPool(NewSize, OriginalData);
+NewData = AllocatePool(NewSize);
 if (NewData == NULL) {
   Status = EFI_OUT_OF_RESOURCES;
 } else {
+  CopyMem (NewData, OriginalData, OriginalSize - OriginalOptionDataSize);
   CopyMem(NewData + OriginalSize - OriginalOptionDataSize, Data, DataSize);
 }
   }
@@ -1120,11 +1121,13 @@ BcfgAddOpt(
 // Now we know how many EFI_INPUT_KEY structs we need to attach to the 
end of the EFI_KEY_OPTION struct.  
 // Re-allocate with the added information.
 //
-KeyOptionBuffer = AllocateCopyPool(sizeof(EFI_KEY_OPTION) + 
(sizeof(EFI_INPUT_KEY) * NewKeyOption.KeyData.Options.InputKeyCount), 
);
+KeyOptionBuffer = AllocatePool (sizeof(EFI_KEY_OPTION) + 
(sizeof(EFI_INPUT_KEY) * NewKeyOption.KeyData.Options.InputKeyCount));
 if (KeyOptionBuffer == NULL) {
   ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_MEM), 
gShellBcfgHiiHandle, L"bcfg");  
   ShellStatus = SHELL_OUT_OF_RESOURCES;
+  return ShellStatus;
 }
+CopyMem (KeyOptionBuffer, , sizeof(EFI_KEY_OPTION));
   }
   for (LoopCounter = 0 ; ShellStatus == SHELL_SUCCESS && LoopCounter < 
NewKeyOption.KeyData.Options.InputKeyCount; LoopCounter++) {
 //
-- 
2.14.1.windows.1

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

[edk2] [PATCH v3 1/3] MdeModulePkg: Fix misuses of AllocateCopyPool

[Jian J Wang]

>v3:
> a. Add ASSERT for returned pointer
> b. Correct DestMax parameter in calling StrCpyS
> c. Fix coding style

>v2:
> a. Use ReallocatePool to replace AllocateCopyPool wherever applicable.

AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
than size of "Buffer", heap memory overflow occurs during copy.

One solution is to allocate pool first then copy the necessary bytes to new
memory. Another is using ReallocatePool instead if old buffer will be freed
on spot.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 .../Application/UiApp/FrontPageCustomizedUiSupport.c |  8 ++--
 .../BootMaintenanceManagerCustomizedUiSupport.c  |  8 ++--
 MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c  | 10 +-
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 12 
 .../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c |  3 ++-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c |  9 ++---
 6 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c 
b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
index 1505ef9319..17fc3db507 100644
--- a/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
+++ b/MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c
@@ -639,9 +639,13 @@ UiListThirdPartyDrivers (
 
 Count++;
 if (Count >= CurrentSize) {
-  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) * 
sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
+  DriverListPtr = ReallocatePool (
+CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
+(Count + UI_HII_DRIVER_LIST_SIZE)
+  * sizeof (UI_HII_DRIVER_INSTANCE),
+gHiiDriverList
+);
   ASSERT (DriverListPtr != NULL);
-  FreePool (gHiiDriverList);
   gHiiDriverList = DriverListPtr;
   CurrentSize += UI_HII_DRIVER_LIST_SIZE;
 }
diff --git 
a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
index b25bc67c06..6dd4fce139 100644
--- 
a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
+++ 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerCustomizedUiSupport.c
@@ -435,9 +435,13 @@ BmmListThirdPartyDrivers (
 
 Count++;
 if (Count >= CurrentSize) {
-  DriverListPtr = AllocateCopyPool ((Count + UI_HII_DRIVER_LIST_SIZE) * 
sizeof (UI_HII_DRIVER_INSTANCE), gHiiDriverList);
+  DriverListPtr = ReallocatePool (
+CurrentSize * sizeof (UI_HII_DRIVER_INSTANCE),
+(Count + UI_HII_DRIVER_LIST_SIZE)
+* sizeof (UI_HII_DRIVER_INSTANCE),
+gHiiDriverList
+);
   ASSERT (DriverListPtr != NULL);
-  FreePool (gHiiDriverList);
   gHiiDriverList = DriverListPtr;
   CurrentSize += UI_HII_DRIVER_LIST_SIZE;
 }
diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c 
b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
index 23ae6c5392..ac8a975bf6 100644
--- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
+++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
@@ -240,7 +240,11 @@ AddIdToMacDeviceList (
   } else {
 mMacDeviceList.MaxListLen += MAX_MAC_ADDRESS_NODE_LIST_LEN;
 if (mMacDeviceList.CurListLen != 0) {
-  TempDeviceList = (MENU_INFO_ITEM *)AllocateCopyPool (sizeof 
(MENU_INFO_ITEM) * mMacDeviceList.MaxListLen, (VOID *)mMacDeviceList.NodeList);
+  TempDeviceList = ReallocatePool (
+ sizeof (MENU_INFO_ITEM) * mMacDeviceList.CurListLen,
+ sizeof (MENU_INFO_ITEM) * mMacDeviceList.MaxListLen,
+ mMacDeviceList.NodeList
+ );
 } else {
   TempDeviceList = (MENU_INFO_ITEM *)AllocatePool (sizeof (MENU_INFO_ITEM) 
* mMacDeviceList.MaxListLen);
 }
@@ -251,10 +255,6 @@ AddIdToMacDeviceList (
 TempDeviceList[mMacDeviceList.CurListLen].PromptId = PromptId;  
 TempDeviceList[mMacDeviceList.CurListLen].QuestionId = (EFI_QUESTION_ID) 
(mMacDeviceList.CurListLen + NETWORK_DEVICE_LIST_KEY_OFFSET);
 
-if (mMacDeviceList.CurListLen > 0) {
-  FreePool(mMacDeviceList.NodeList);
-}
-
 mMacDeviceList.NodeList = TempDeviceList;
   }
   mMacDeviceList.CurListLen ++;
diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c 
b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
index 

[edk2] [PATCH v3 0/3] Fix misuses of AllocateCopyPool

[Jian J Wang]

> v3:
> a. Add necessary ASSERT
> b. Correct StrCpyS parameter
> c. Coding style clean-up

> v2:
> a. Use ReallocatePool instead of allocating then copying wherever applicable

AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
than size of "Buffer", heap memory overflow occurs during copy.

One solution is to allocate pool first then copy the necessary bytes to new
memory. Another is using ReallocatePool instead if old buffer will be freed
on spot.

Jian J Wang (3):
  MdeModulePkg: Fix misuses of AllocateCopyPool
  ShellPkg: Fix misuses of AllocateCopyPool
  IntelFrameworkModulePkg: Fix misuses of AllocateCopyPool

 .../Universal/BdsDxe/DeviceMngr/DeviceManager.c  | 10 +-
 .../Application/UiApp/FrontPageCustomizedUiSupport.c |  8 ++--
 .../BootMaintenanceManagerCustomizedUiSupport.c  |  8 ++--
 MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c  | 10 +-
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 12 
 .../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c |  3 ++-
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c |  9 ++---
 ShellPkg/Application/Shell/Shell.c   |  4 +++-
 .../UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c|  7 +--
 9 files changed, 46 insertions(+), 25 deletions(-)

-- 
2.14.1.windows.1

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

[edk2] [PATCH v3 3/3] IntelFrameworkModulePkg: Fix misuses of AllocateCopyPool

[Jian J Wang]

> v3:
>No updates.

> v2:
> a. Use ReallocatePool instead of allocating then copying wherever applicable

AllocateCopyPool(AllocationSize, *Buffer) will copy "AllocationSize" bytes of
memory from old "Buffer" to new allocated one. If "AllocationSize" is bigger
than size of "Buffer", heap memory overflow occurs during copy.

One solution is to allocate pool first then copy the necessary bytes to new
memory. Another is using ReallocatePool instead if old buffer will be freed
on spot.

Cc: Liming Gao 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 .../Universal/BdsDxe/DeviceMngr/DeviceManager.c| 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git 
a/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
index 125c49db5e..5103c7e5d1 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
@@ -480,7 +480,11 @@ AddIdToMacDeviceList (
   } else {
 mMacDeviceList.MaxListLen += MAX_MAC_ADDRESS_NODE_LIST_LEN;
 if (mMacDeviceList.CurListLen != 0) {
-  TempDeviceList = (MENU_INFO_ITEM *)AllocateCopyPool (sizeof 
(MENU_INFO_ITEM) * mMacDeviceList.MaxListLen, (VOID *)mMacDeviceList.NodeList);
+  TempDeviceList = ReallocatePool (
+ sizeof (MENU_INFO_ITEM) * mMacDeviceList.CurListLen,
+ sizeof (MENU_INFO_ITEM) * mMacDeviceList.MaxListLen,
+ mMacDeviceList.NodeList
+ );
 } else {
   TempDeviceList = (MENU_INFO_ITEM *)AllocatePool (sizeof (MENU_INFO_ITEM) 
* mMacDeviceList.MaxListLen);
 }
@@ -491,10 +495,6 @@ AddIdToMacDeviceList (
 TempDeviceList[mMacDeviceList.CurListLen].PromptId = PromptId;  
 TempDeviceList[mMacDeviceList.CurListLen].QuestionId = (EFI_QUESTION_ID) 
(mMacDeviceList.CurListLen + NETWORK_DEVICE_LIST_KEY_OFFSET);
 
-if (mMacDeviceList.CurListLen > 0) {
-  FreePool(mMacDeviceList.NodeList);
-}
-
 mMacDeviceList.NodeList = TempDeviceList;
   }
   mMacDeviceList.CurListLen ++;
-- 
2.14.1.windows.1

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

Re: [edk2] [PATCH] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE

[Heyi Guo]

Thanks,

Heyi


在 11/7/2017 9:30 PM, Ard Biesheuvel 写道:

On 7 November 2017 at 13:29, Leif Lindholm  wrote:

On Tue, Nov 07, 2017 at 01:22:19PM +, Ard Biesheuvel wrote:

On 7 November 2017 at 12:56, Heyi Guo  wrote:

From: Peicong Li 

Flash region needs to be set as cacheable (write back) to increase
performance, if PEI is still XIP on flash or DXE FV is decompressed
from flash FV. However some ARM platforms do not support to set flash
as inner shareable since flash is not normal DDR memory and it will
not respond to cache snoop request, which will causes system hang
after MMU is enabled.

So we need a new ARM memory region attribute WRITE_BACK_NONSHAREABLE
for flash region on these platforms specifically. This attribute will
set the region as write back but not inner shared.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Peicong Li 
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 

Reviewed-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 


Pushed as 829633e3a82d. Thanks!


---
  ArmPkg/Include/Library/ArmLib.h  | 7 +++
  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
  2 files changed, 11 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 24ffe9f..38199be 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -41,6 +41,13 @@ typedef enum {
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
+  // On some platforms, memory mapped flash region is designed as not 
supporting
+  // shareable attribute, so WRITE_BACK_NONSHAREABLE is added for such special
+  // need.
+  // Do NOT use below two attributes if you are not sure.
+  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE,
+  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE,
+
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH,
ARM_MEMORY_REGION_ATTRIBUTE_DEVICE,
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 8bd1c6f..4b62ecb 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
)
  {
switch (Attributes) {
+  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
+  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
+return TT_ATTR_INDX_MEMORY_WRITE_BACK;
+
case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
--
2.7.2.windows.1



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

Re: [edk2] [Patch 0/2] Add missed Depex Protocol/Ppi.

[Dong, Eric]

Hi Laszlo,

Agree with you, I'm not aware the consume protocol/Ppi code is in the library 
till now. I will discard the changes.

Thanks,
Eric
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, November 8, 2017 2:18 AM
> To: Dong, Eric 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch 0/2] Add missed Depex Protocol/Ppi.
> 
> Hi Eric,
> 
> On 11/06/17 13:26, Eric Dong wrote:
> > The Protocol/Ppi used in the drivers but the it not add them in the
> > dependence section, it may cause driver assert.
> > This patch series add the missed Protocol/Ppi.
> >
> > Eric Dong (2):
> >   UefiCpuPkg/CpuFeaturesDxe.inf: Add missed Depex protocol.
> >   UefiCpuPkg/CpuFeaturesPei.inf: Add missed Depex Ppi.
> >
> >  UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 2 +-
> > UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> 
> (hopefully I'm commenting soon enough,)
> 
> where exactly are the MP protocol and PPI used in
> UefiCpuPkg/CpuFeatures/?
> 
> I grepped the directory for both protocol and PPI, and I found no matches.
> 
> If the dependencies are incurred via library instances, then those library
> instances should spell out the depex.
> 
> The only library class used by CpuFeaturesDxe.inf and CpuFeaturesPei.inf,
> for which multi-processing looks remotely relevant,
> is: RegisterCpuFeaturesLib.
> 
> For this class, two instances appear to exist:
> 
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.inf
> 
> The DXE instance consumes gEfiMpServiceProtocolGuid alright, but it also
> spells out the protocol in the depex:
> 
> [Depex]
>   gEfiMpServiceProtocolGuid AND gEdkiiCpuFeaturesSetDoneGuid
> 
> The PEI instance is similar, wrt. the PPI:
> 
> [Depex]
>   gEfiPeiMpServicesPpiGuid AND gEdkiiCpuFeaturesSetDoneGuid
> 
> So, this series appears unnecessary. What am I missing?
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

[Wang, Jian J]

Hi Laszlo,

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, November 08, 2017 1:14 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Dong, Eric 
> Subject: Re: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of
> RT_CODE in memory map
> 
> sorry about the late response
> 
> On 11/03/17 01:57, Jian J Wang wrote:
> >> v2
> >> a. Fix an issue which will cause setting capability failure if size is 
> >> smaller
> >>than a page.
> >
> > More than one entry of RT_CODE memory might cause boot problem for
> some
> > old OSs. This patch will fix this issue to keep OS compatibility as much
> > as possible.
> >
> > More detailed information, please refer to
> > https://bugzilla.tianocore.org/show_bug.cgi?id=753
> >
> > Cc: Eric Dong 
> > Cc: Jiewen Yao 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang 
> > ---
> >  UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 ++
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > index d312eb66f8..4a7827ebc9 100644
> > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > @@ -809,7 +809,9 @@ RefreshGcdMemoryAttributesFromPaging (
> >PageLength= 0;
> >
> >for (Index = 0; Index < NumberOfDescriptors; Index++) {
> > -if (MemorySpaceMap[Index].GcdMemoryType ==
> EfiGcdMemoryTypeNonExistent) {
> > +if (MemorySpaceMap[Index].GcdMemoryType ==
> EfiGcdMemoryTypeNonExistent
> > +|| (MemorySpaceMap[Index].BaseAddress & EFI_PAGE_MASK) != 0
> > +|| (MemorySpaceMap[Index].Length & EFI_PAGE_MASK) != 0) {
> >continue;
> >  }
> 
> When exactly do the new conditions match?
> 
> I thought the base addresses and the lengths in the GCD memory space map
> are all page aligned. Is that not the case?
> 
> If these conditions are just a sanity check (i.e. we never expect them
> to fire), then should we perpahs turn them into ASSERT()s?
> 

I found that there's a mmio entry in memory map on OVMF which has size
less than a page. I didn't encounter this before. Maybe some recent changes
in other part of EDKII caused this situation. So ASSERT is not enough.

> >
> > @@ -829,6 +831,15 @@ RefreshGcdMemoryAttributesFromPaging (
> >  // Sync real page attributes to GCD
> >  BaseAddress   = MemorySpaceMap[Index].BaseAddress;
> >  MemorySpaceLength = MemorySpaceMap[Index].Length;
> > +Capabilities  = MemorySpaceMap[Index].Capabilities |
> > +EFI_MEMORY_PAGETYPE_MASK;
> > +Status = gDS->SetMemorySpaceCapabilities (
> > +BaseAddress,
> > +MemorySpaceLength,
> > +Capabilities
> > +);
> > +ASSERT_EFI_ERROR (Status);
> > +
> 
> OK, so I guess we simply add EFI_MEMORY_PAGETYPE_MASK to the
> capabilities of all memory space map entries that have a type different
> from non-existent. We discussed it before and (apparently) it is
> considered safe.
> 

Yes. I've validated different OSs boot. It's safe to stay this way.

> >  while (MemorySpaceLength > 0) {
> >if (PageLength == 0) {
> >  PageEntry = GetPageTableEntry (, BaseAddress,
> );
> > @@ -846,7 +857,6 @@ RefreshGcdMemoryAttributesFromPaging (
> >  if (Attributes != (MemorySpaceMap[Index].Attributes &
> EFI_MEMORY_PAGETYPE_MASK)) {
> >DoUpdate = TRUE;
> >Attributes |= (MemorySpaceMap[Index].Attributes &
> ~EFI_MEMORY_PAGETYPE_MASK);
> > -  Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
> >  } else {
> >DoUpdate = FALSE;
> >  }
> > @@ -854,8 +864,8 @@ RefreshGcdMemoryAttributesFromPaging (
> >
> >Length = MIN (PageLength, MemorySpaceLength);
> >if (DoUpdate) {
> > -gDS->SetMemorySpaceCapabilities (BaseAddress, Length, 
> > Capabilities);
> > -gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
> > +Status = gDS->SetMemorySpaceAttributes (BaseAddress, Length,
> Attributes);
> > +ASSERT_EFI_ERROR (Status);
> >  DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx
> - %016lx (%08lx -> %08lx)\r\n",
> >   Index, BaseAddress, BaseAddress + Length - 1,
> >   MemorySpaceMap[Index].Attributes, 
> > Attributes));
> >
> 
> I'll let you decide about the EFI_PAGE_MASK conditions near the top.
> 
> Acked-by: Laszlo Ersek 
> 

Thanks.

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

Re: [edk2] [PATCH V2] MdeModulePkg SerialDxe: Handle Timeout change more robustly

[Laszlo Ersek]

oops, forgot to write down my only comment:

On 11/07/17 19:27, Laszlo Ersek wrote:
> On 11/07/17 02:36, Star Zeng wrote:
>> https://lists.01.org/pipermail/edk2-devel/2017-October/016479.html
>> reported "Xen Console input very slow in recent UEFI" that appears
>> after 4cf3f37c87ba1f9d58072444bd735e40e4779e70 "MdeModulePkg
>> SerialDxe: Process timeout consistently in SerialRead".
>>
>> Julien did more debugging and find out the following is happening in
>> TerminalConInTimerHandler (MdeModulePkg/Universal/Console/TerminalDxe)
>> when a character is received:
>> 1) GetControl will return EFI_SERIAL_INPUT_BUFFER_EMPTY unset
>>   => Entering in the loop to fetch character from the serial
>> 2) GetOneKeyFromSerial()
>>   => Return directly with the character read
>> 3) Looping as the fifo is not full and no error
>> 4) GetOneKeyFromSerial() -> SerialRead()
>>   => No more character so SerialPortPoll() will return FALSE and loop
>>  until timeout
>>   => Return EFI_TIMEOUT
>> 5) Exiting the loop from TerminalConInTimerHandler
>> 6) Characters are printed
>>
>> After some investigation, I found it is related to the Timeout value.
>>
>> The Timeout is 100 (1s) by default to follow UEFI spec.
>> And the Terminal driver will recalculate and set the Timeout value
>> based on the properties of UART in TerminalDriverBindingStart()/
>> TerminalConInTimerHandler().
>>
>>   SerialInTimeOut = 0;
>>   if (Mode->BaudRate != 0) {
>> //
>> // According to BAUD rate to calculate the timeout value.
>> //
>> SerialInTimeOut = (1 + Mode->DataBits + Mode->StopBits) *
>>   2 * 100 / (UINTN) Mode->BaudRate;
>>   }
>>
>> For example, based on the PCD values of PcdUartDefaultBaudRate,
>> PcdUartDefaultDataBits and PcdUartDefaultStopBits, SerialInTimeOut =
>> (1 + 8  + 1) * 2 * 100 / (UINTN) 115200 = 173 (us).
>>
>> When SerialDxe is used,
>> TerminalDriverBindingStart()/TerminalConInTimerHandler() ->
>>   SerialIo->SetAttributes() ->
>> SerialSetAttributes() ->
>>   SerialPortSetAttributes()
>>
>> Some implementations of SerialPortSetAttributes() could handle the
>> input parameters and return RETURN_SUCCESS, for example
>> BaseSerialPortLib16550, then Timeout value will be changed to 173 (us),
>> no "slow down" will be observed.
>> But some implementations of SerialPortSetAttributes() just return
>> RETURN_UNSUPPORTED, for example XenConsoleSerialPortLib, then Timeout
>> value will be not changed and kept 100 (1s), "slow down" will be
>> observed.
>>
>> SerialPortLib instance can be enhanced to
>> 1. Handle the input parameters and return status accordingly instead of
>> just returning RETURN_UNSUPPORTED in SerialPortSetAttributes().
>> 2. Just return RETURN_SUCCESS instead of RETURN_UNSUPPORTED in
>> SerialPortSetAttributes() if the instance does not care the input
>> parameters at all.
>>
>> And SerialDxe can also be enhanced like this patch to be more robust
>> to handle Timeout change.
>>
>> Cc: Julien Grall 
>> Cc: Laszlo Ersek 
>> Cc: Ruiyu Ni 
>> Compare against the original parameters
>>   Suggested-by: Laszlo Ersek 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Star Zeng 
>>
>> V2: Compare against the original parameters
>> ---
>>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 45 
>> +++--
>>  1 file changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
>> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
>> index ebcd92726314..5be77e7acfb0 100644
>> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
>> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
>> @@ -280,12 +280,51 @@ SerialSetAttributes (
>>IN EFI_STOP_BITS_TYPE StopBits
>>)
>>  {
>> -  EFI_STATUSStatus;
>> -  EFI_TPL   Tpl;
>> +  EFI_STATUSStatus;
>> +  EFI_TPL   Tpl;
>> +  UINT64OriginalBaudRate;
>> +  UINT32OriginalReceiveFifoDepth;
>> +  UINT32OriginalTimeout;
>> +  EFI_PARITY_TYPE   OriginalParity;
>> +  UINT8 OriginalDataBits;
>> +  EFI_STOP_BITS_TYPEOriginalStopBits;
>>  
>> +  //
>> +  // Preserve the original input values in case
>> +  // SerialPortSetAttributes() updates the input/output parameters even on 
>> error.

This line is 81 characters long; for better edk2 style conformance,
please consider wrapping it, before you push the patch.

Thanks!
Laszlo

>> +  //
>> +  OriginalBaudRate = BaudRate;
>> +  OriginalReceiveFifoDepth = ReceiveFifoDepth;
>> +  OriginalTimeout = Timeout;
>> +  OriginalParity = Parity;
>> +  OriginalDataBits = DataBits;
>> +  OriginalStopBits = StopBits;
>>Status = SerialPortSetAttributes (, , , 
>> , , );
>>if (EFI_ERROR (Status)) {
>> -return Status;
>> +//
>> +// If it is just to set Timeout 

Re: [edk2] [PATCH V2] MdeModulePkg SerialDxe: Handle Timeout change more robustly

[Laszlo Ersek]

On 11/07/17 02:36, Star Zeng wrote:
> https://lists.01.org/pipermail/edk2-devel/2017-October/016479.html
> reported "Xen Console input very slow in recent UEFI" that appears
> after 4cf3f37c87ba1f9d58072444bd735e40e4779e70 "MdeModulePkg
> SerialDxe: Process timeout consistently in SerialRead".
> 
> Julien did more debugging and find out the following is happening in
> TerminalConInTimerHandler (MdeModulePkg/Universal/Console/TerminalDxe)
> when a character is received:
> 1) GetControl will return EFI_SERIAL_INPUT_BUFFER_EMPTY unset
>   => Entering in the loop to fetch character from the serial
> 2) GetOneKeyFromSerial()
>   => Return directly with the character read
> 3) Looping as the fifo is not full and no error
> 4) GetOneKeyFromSerial() -> SerialRead()
>   => No more character so SerialPortPoll() will return FALSE and loop
>  until timeout
>   => Return EFI_TIMEOUT
> 5) Exiting the loop from TerminalConInTimerHandler
> 6) Characters are printed
> 
> After some investigation, I found it is related to the Timeout value.
> 
> The Timeout is 100 (1s) by default to follow UEFI spec.
> And the Terminal driver will recalculate and set the Timeout value
> based on the properties of UART in TerminalDriverBindingStart()/
> TerminalConInTimerHandler().
> 
>   SerialInTimeOut = 0;
>   if (Mode->BaudRate != 0) {
> //
> // According to BAUD rate to calculate the timeout value.
> //
> SerialInTimeOut = (1 + Mode->DataBits + Mode->StopBits) *
>   2 * 100 / (UINTN) Mode->BaudRate;
>   }
> 
> For example, based on the PCD values of PcdUartDefaultBaudRate,
> PcdUartDefaultDataBits and PcdUartDefaultStopBits, SerialInTimeOut =
> (1 + 8  + 1) * 2 * 100 / (UINTN) 115200 = 173 (us).
> 
> When SerialDxe is used,
> TerminalDriverBindingStart()/TerminalConInTimerHandler() ->
>   SerialIo->SetAttributes() ->
> SerialSetAttributes() ->
>   SerialPortSetAttributes()
> 
> Some implementations of SerialPortSetAttributes() could handle the
> input parameters and return RETURN_SUCCESS, for example
> BaseSerialPortLib16550, then Timeout value will be changed to 173 (us),
> no "slow down" will be observed.
> But some implementations of SerialPortSetAttributes() just return
> RETURN_UNSUPPORTED, for example XenConsoleSerialPortLib, then Timeout
> value will be not changed and kept 100 (1s), "slow down" will be
> observed.
> 
> SerialPortLib instance can be enhanced to
> 1. Handle the input parameters and return status accordingly instead of
> just returning RETURN_UNSUPPORTED in SerialPortSetAttributes().
> 2. Just return RETURN_SUCCESS instead of RETURN_UNSUPPORTED in
> SerialPortSetAttributes() if the instance does not care the input
> parameters at all.
> 
> And SerialDxe can also be enhanced like this patch to be more robust
> to handle Timeout change.
> 
> Cc: Julien Grall 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Compare against the original parameters
>   Suggested-by: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> 
> V2: Compare against the original parameters
> ---
>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 45 
> +++--
>  1 file changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> index ebcd92726314..5be77e7acfb0 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> @@ -280,12 +280,51 @@ SerialSetAttributes (
>IN EFI_STOP_BITS_TYPE StopBits
>)
>  {
> -  EFI_STATUSStatus;
> -  EFI_TPL   Tpl;
> +  EFI_STATUSStatus;
> +  EFI_TPL   Tpl;
> +  UINT64OriginalBaudRate;
> +  UINT32OriginalReceiveFifoDepth;
> +  UINT32OriginalTimeout;
> +  EFI_PARITY_TYPE   OriginalParity;
> +  UINT8 OriginalDataBits;
> +  EFI_STOP_BITS_TYPEOriginalStopBits;
>  
> +  //
> +  // Preserve the original input values in case
> +  // SerialPortSetAttributes() updates the input/output parameters even on 
> error.
> +  //
> +  OriginalBaudRate = BaudRate;
> +  OriginalReceiveFifoDepth = ReceiveFifoDepth;
> +  OriginalTimeout = Timeout;
> +  OriginalParity = Parity;
> +  OriginalDataBits = DataBits;
> +  OriginalStopBits = StopBits;
>Status = SerialPortSetAttributes (, , , 
> , , );
>if (EFI_ERROR (Status)) {
> -return Status;
> +//
> +// If it is just to set Timeout value and unsupported is returned,
> +// do not return error.
> +//
> +if ((Status == EFI_UNSUPPORTED) &&
> +(This->Mode->Timeout  != OriginalTimeout) &&
> +(This->Mode->ReceiveFifoDepth == OriginalReceiveFifoDepth) &&
> +(This->Mode->BaudRate == OriginalBaudRate) &&
> +

Re: [edk2] [Patch 0/2] Add missed Depex Protocol/Ppi.

[Laszlo Ersek]

Hi Eric,

On 11/06/17 13:26, Eric Dong wrote:
> The Protocol/Ppi used in the drivers but the it not add them
> in the dependence section, it may cause driver assert. 
> This patch series add the missed Protocol/Ppi.
> 
> Eric Dong (2):
>   UefiCpuPkg/CpuFeaturesDxe.inf: Add missed Depex protocol.
>   UefiCpuPkg/CpuFeaturesPei.inf: Add missed Depex Ppi.
> 
>  UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf | 2 +-
>  UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

(hopefully I'm commenting soon enough,)

where exactly are the MP protocol and PPI used in UefiCpuPkg/CpuFeatures/?

I grepped the directory for both protocol and PPI, and I found no matches.

If the dependencies are incurred via library instances, then those
library instances should spell out the depex.

The only library class used by CpuFeaturesDxe.inf and
CpuFeaturesPei.inf, for which multi-processing looks remotely relevant,
is: RegisterCpuFeaturesLib.

For this class, two instances appear to exist:

UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.inf

The DXE instance consumes gEfiMpServiceProtocolGuid alright, but it also
spells out the protocol in the depex:

[Depex]
  gEfiMpServiceProtocolGuid AND gEdkiiCpuFeaturesSetDoneGuid

The PEI instance is similar, wrt. the PPI:

[Depex]
  gEfiPeiMpServicesPpiGuid AND gEdkiiCpuFeaturesSetDoneGuid

So, this series appears unnecessary. What am I missing?

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

Re: [edk2] [PATCH v2] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core

[Ard Biesheuvel]

On 7 November 2017 at 18:09, Laszlo Ersek  wrote:
> On 11/05/17 17:29, Ard Biesheuvel wrote:
>> On 5 November 2017 at 16:27, Ard Biesheuvel  
>> wrote:
>>> On 5 November 2017 at 05:52, Leif Lindholm  wrote:
 On Fri, Nov 03, 2017 at 11:33:52AM +, Ard Biesheuvel wrote:
> DEBUG builds of PEI code will print a diagnostic message regarding
> the utilization of temporary RAM before switching to permanent RAM.
> For example,
>
>   Total temporary memory:16352 bytes.
> temporary memory stack ever used:   4820 bytes.
> temporary memory heap used for HobList: 4720 bytes.
>
> Tracking stack utilization like this requires the stack to be seeded
> with a known magic value, and this needs to occur before entering C
> code, given that it uses the stack. Currently, only Nt32Pkg appears
> to implement this feature, but it is useful nonetheless, so let's
> wire it up for PrePeiCore as well.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=748
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

 OK, this may sound completely unreasonable, but seeing those
 implementations overwrite callee-saved registers without saving them
 makes my brain unhappy. (Yes, I know.)

 Could they either:
 - Have a comment prepended establishing the implicit ABI of which
   registers the caller cannot rely on reusing after return.
   Preferably somewhat echoed at the call site.
 - Be rewritten to use only scratch registers?

>>>
>>> I think it is implied that the startup code does not adhere to the
>>> AAPCS. That code already uses r5 and r6 without stacking them, simply
>>> because we're in the middle of preparing the stack and other execution
>>> context, precisely so the C code we call into can rely on AAPCS
>>> guarantees.
>>
>>
>> Ehm, hold on, what do you mean by 'call site'? This code just runs and
>> jumps back to a local label. There are no functions calls here until
>> the point where we call into C (with the exception of the lovely
>> ArmPlatformPeiBootAction() we added so Juno can find out how much DRAM
>> it can use)
>
> Please continue the discussion with Leif on this; from my side, I'm
> happy with the patch (I've sort of deduced what the assembly code does,
> also relying on your v1 notes).
>
> The only eyebrow-raising part was:
>
> +  MOV64 (x9, FixedPcdGet32 (PcdInitValueInTempStack) |\
> + FixedPcdGet32 (PcdInitValueInTempStack) << 32)
>
> where we left-shift a constant that is "in theory" UINT32 by 32 binary
> places, using the << operator. In C that would be undefined behavior,
> but this is assembly, so what do I know? ¯\_(ツ)_/¯
>
> Acked-by: Laszlo Ersek 
>

Thanks. And you're right, this is not C so no need to worry about that.

> (
>
> By the way, just to see if I remember correctly, isn't STP:
>
> +0:stp   x9, x9, [x8], #16
>
> the kind of instruction that modifies multiple operands at once, and so
> if it faults, it cannot be virtualized well? (Because the syndrome
> register or whatever does not tell the VMM the whole picture about the
> fault?)
>
> Totally irrelevant here, I'm just curious.
>

STP == STore Pair, and so it stores the values in the registers to
memory. The only register that gets modified here is x8, due to the
post-increment.

But its converse

LDP  , , [], #

is indeed such an instruction, given that it modifies three registers
at once, and so the registers that encode the exception run out of
space. Note that this only affects virtualized MMIO.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH v2 0/6] Armada 7k/8k SPI improvements pt 2.

[Marcin Wojtas]

2017-11-07 18:21 GMT+01:00 Leif Lindholm :
> On Fri, Nov 03, 2017 at 06:57:09PM +0100, Marcin Wojtas wrote:
>> Hi,
>>
>> I submit corrected version of the Armada SPI improvements
>> after the first round of review. There were no significant changes
>> comparing to v1, please check the changelog below for the details.
>>
>> Patches are available in the github:
>> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/spi-upstream-r20171103
>> NorFlashInfoLib:
>> https://github.com/MarvellEmbeddedProcessors/edk2/commits/norlib-upstream-r20171103
>>
>> I'm looking forward to the comments or remarks.
>
> For the remaining patches of the series:
> Reviewed-by: Leif Lindholm 
>
> Pushed as f79bce44ac..6c7c803218.
>

Thanks!

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

Re: [edk2] [PATCH v2] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core

[Laszlo Ersek]

On 11/05/17 17:29, Ard Biesheuvel wrote:
> On 5 November 2017 at 16:27, Ard Biesheuvel  wrote:
>> On 5 November 2017 at 05:52, Leif Lindholm  wrote:
>>> On Fri, Nov 03, 2017 at 11:33:52AM +, Ard Biesheuvel wrote:
 DEBUG builds of PEI code will print a diagnostic message regarding
 the utilization of temporary RAM before switching to permanent RAM.
 For example,

   Total temporary memory:16352 bytes.
 temporary memory stack ever used:   4820 bytes.
 temporary memory heap used for HobList: 4720 bytes.

 Tracking stack utilization like this requires the stack to be seeded
 with a known magic value, and this needs to occur before entering C
 code, given that it uses the stack. Currently, only Nt32Pkg appears
 to implement this feature, but it is useful nonetheless, so let's
 wire it up for PrePeiCore as well.

 Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=748
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Ard Biesheuvel 
>>>
>>> OK, this may sound completely unreasonable, but seeing those
>>> implementations overwrite callee-saved registers without saving them
>>> makes my brain unhappy. (Yes, I know.)
>>>
>>> Could they either:
>>> - Have a comment prepended establishing the implicit ABI of which
>>>   registers the caller cannot rely on reusing after return.
>>>   Preferably somewhat echoed at the call site.
>>> - Be rewritten to use only scratch registers?
>>>
>>
>> I think it is implied that the startup code does not adhere to the
>> AAPCS. That code already uses r5 and r6 without stacking them, simply
>> because we're in the middle of preparing the stack and other execution
>> context, precisely so the C code we call into can rely on AAPCS
>> guarantees.
> 
> 
> Ehm, hold on, what do you mean by 'call site'? This code just runs and
> jumps back to a local label. There are no functions calls here until
> the point where we call into C (with the exception of the lovely
> ArmPlatformPeiBootAction() we added so Juno can find out how much DRAM
> it can use)

Please continue the discussion with Leif on this; from my side, I'm
happy with the patch (I've sort of deduced what the assembly code does,
also relying on your v1 notes).

The only eyebrow-raising part was:

+  MOV64 (x9, FixedPcdGet32 (PcdInitValueInTempStack) |\
+ FixedPcdGet32 (PcdInitValueInTempStack) << 32)

where we left-shift a constant that is "in theory" UINT32 by 32 binary
places, using the << operator. In C that would be undefined behavior,
but this is assembly, so what do I know? ¯\_(ツ)_/¯

Acked-by: Laszlo Ersek 

(

By the way, just to see if I remember correctly, isn't STP:

+0:stp   x9, x9, [x8], #16

the kind of instruction that modifies multiple operands at once, and so
if it faults, it cannot be virtualized well? (Because the syndrome
register or whatever does not tell the VMM the whole picture about the
fault?)

Totally irrelevant here, I'm just curious.

)

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

Re: [edk2] [PATCH] Tftp assert fix for openfile failure case

[Leif Lindholm]

On Fri, Nov 03, 2017 at 07:56:32PM +0530, Vabhav wrote:
> Issue:
> when file open is failed, assert was seen due to freeing 0 size page
> 
> Reason:
> DataSize is remain zero if error is reported in ShellOpenFileByName
> 
> Fix:
> Update DataSize as soon as FileSize is available
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Udit Kumar 
> Signed-off-by: Vabhav 
> ---
>  ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c 
> b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> index fbde3bf..6425fc5 100755
> --- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> +++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> @@ -509,6 +509,7 @@ ShellCommandRunTftp (
>);
>goto NextHandle;

Wow, a goto in a foor loop in a 320-line function.
What could possibly go wrong?

>  }
> +DataSize = FileSize;
>  
>  Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath, 
> FileSize, BlockSize, );
>  if (EFI_ERROR (Status)) {
> @@ -539,7 +540,6 @@ ShellCommandRunTftp (
>goto NextHandle;
>  }
>  
> -DataSize = FileSize;
>  Status = ShellWriteFile (FileHandle, , Data);
>  if (!EFI_ERROR (Status)) {
>ShellStatus = SHELL_SUCCESS;
> -- 
> 1.9.1

So, a wider question:
This shell command was introduced in the heyday of "let's reimplement
U-Boot in the EDK2 tree". Mainly, from my impression, it seems to be
used in order that people don't need to learn how boot managers and
device paths work.
Am I being too harsh?
Are there practical uses for this?
Or should we delete it from the tree?

If the code is to be kept, I think (from a quick glance) that I would
also like to see *Data = NULL in the error path of DownloadFile().

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

Re: [edk2] [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctly

[Laszlo Ersek]

On 11/03/17 09:28, Ruiyu Ni wrote:
> The original code enables some BITs in PCI attributes in Start(),
> but wrongly to disable these BITs in Stop().
> 
> The correct behavior is to save the original PCI attributes before
> enables some BITs in Start(), and restore to original value
> in Stop().
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> ---
>  PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c | 44 
> +
>  PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h |  3 ++-
>  2 files changed, 25 insertions(+), 22 deletions(-)

I see that Star commented already; I agree with his points. Beyond them,
I have some comments of my own:

> diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c 
> b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
> index 32381b112d..60d2fb5a5b 100644
> --- a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
> +++ b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c

(1) The copyright notice is updated in the header file. For consistency,
I think you might want to do the same in the C file.

> @@ -172,6 +172,7 @@ PcatIsaAcpiDriverBindingStart (
>EFI_PCI_IO_PROTOCOL  *PciIo;
>PCAT_ISA_ACPI_DEV*PcatIsaAcpiDev;
>UINT64   Supports;
> +  UINT64   OriginalAttributes;
>BOOLEAN  Enabled;
>  
>Enabled = FALSE;
> @@ -210,9 +211,18 @@ PcatIsaAcpiDriverBindingStart (
>if (Supports == 0 || Supports == (EFI_PCI_IO_ATTRIBUTE_ISA_IO | 
> EFI_PCI_IO_ATTRIBUTE_ISA_IO_16)) {
>  Status = EFI_UNSUPPORTED;
>  goto Done;
> -  }  
> +  }
> +
> +  Status = PciIo->Attributes (
> +PciIo,
> +EfiPciIoAttributeOperationGet,
> +0,
> +
> +);
> +  if (EFI_ERROR (Status)) {
> +goto Done;
> +  }
>  
> -  Enabled = TRUE;
>Status = PciIo->Attributes (
>  PciIo, 
>  EfiPciIoAttributeOperationEnable, 
> @@ -222,7 +232,8 @@ PcatIsaAcpiDriverBindingStart (
>if (EFI_ERROR (Status)) {
>  goto Done;
>}
> -  
> +
> +  Enabled = TRUE;
>//
>// Allocate memory for the PCAT ISA ACPI Device structure
>//
> @@ -239,9 +250,10 @@ PcatIsaAcpiDriverBindingStart (
>//
>// Initialize the PCAT ISA ACPI Device structure
>//
> -  PcatIsaAcpiDev->Signature = PCAT_ISA_ACPI_DEV_SIGNATURE;
> -  PcatIsaAcpiDev->Handle= Controller;
> -  PcatIsaAcpiDev->PciIo = PciIo;
> +  PcatIsaAcpiDev->Signature = PCAT_ISA_ACPI_DEV_SIGNATURE;
> +  PcatIsaAcpiDev->Handle= Controller;
> +  PcatIsaAcpiDev->PciIo = PciIo;
> +  PcatIsaAcpiDev->OriginalAttribute = OriginalAttributes;
>  
>//
>// Initialize PcatIsaAcpiDeviceList
> @@ -274,8 +286,8 @@ Done:
>  if (PciIo != NULL && Enabled) {
>PciIo->Attributes (
> PciIo, 
> -   EfiPciIoAttributeOperationDisable, 
> -   EFI_PCI_DEVICE_ENABLE | Supports | 
> EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO,
> +   EfiPciIoAttributeOperationSet, 
> +   OriginalAttributes,
> NULL 
> );
>  }
> @@ -321,7 +333,6 @@ PcatIsaAcpiDriverBindingStop (
>EFI_STATUS Status;
>EFI_ISA_ACPI_PROTOCOL  *IsaAcpi;
>PCAT_ISA_ACPI_DEV  *PcatIsaAcpiDev;
> -  UINT64 Supports;
>
>//
>// Get the ISA ACPI Protocol Interface
> @@ -348,23 +359,14 @@ PcatIsaAcpiDriverBindingStop (
>//
>Status = PcatIsaAcpiDev->PciIo->Attributes (
>  PcatIsaAcpiDev->PciIo,
> -EfiPciIoAttributeOperationSupported,
> -0,
> -
> +EfiPciIoAttributeOperationSet,
> +PcatIsaAcpiDev->OriginalAttribute,
> +0
>  );

(2) For better edk2 style, the last argument should be NULL, not 0.

(3) (Mentioned earlier, also by Star) -- please add the TianoCore BZ
reference (#405).

With those fixed,

Reviewed-by: Laszlo Ersek 

Thanks!
Laszlo

>if (EFI_ERROR (Status)) {
>  return Status;
>}
>  
> -  Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_ISA_IO | 
> EFI_PCI_IO_ATTRIBUTE_ISA_IO_16);
> -
> -  PcatIsaAcpiDev->PciIo->Attributes (
> -   PcatIsaAcpiDev->PciIo, 
> -   EfiPciIoAttributeOperationDisable, 
> -   EFI_PCI_DEVICE_ENABLE | Supports | 
> EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO,
> -   NULL 
> -   );
> - 
>//
>// Uninstall protocol interface: EFI_ISA_ACPI_PROTOCOL
>//
> diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h 
> b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h
> 

Re: [edk2] [platforms: PATCH v2 0/6] Armada 7k/8k SPI improvements pt 2.

[Leif Lindholm]

On Fri, Nov 03, 2017 at 06:57:09PM +0100, Marcin Wojtas wrote:
> Hi,
> 
> I submit corrected version of the Armada SPI improvements
> after the first round of review. There were no significant changes
> comparing to v1, please check the changelog below for the details.
> 
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/spi-upstream-r20171103
> NorFlashInfoLib:
> https://github.com/MarvellEmbeddedProcessors/edk2/commits/norlib-upstream-r20171103
> 
> I'm looking forward to the comments or remarks.

For the remaining patches of the series:
Reviewed-by: Leif Lindholm 

Pushed as f79bce44ac..6c7c803218.

> Best regards,
> Marcin
> 
> Changelog:
> v1 -> v2
> 1/6
>   - Replace NOR_FLASH_ID_DEFAULT_LEN with PcdGetSize (PcdSpiFlashId)
> 
> 2/6
>   - Adjust to renamed functions and macros according to NorFlashInfoLib v2
>   - Restore handling of CMD_ERASE_32K
>   - Check NOR_FLASH_4B_ADDR only once and use SPI_DEVICE structure instead
> 
> 3/6
>   - Improve commit log
> 
> 4/6
>   - Use global variable explicitly (mSlave)
> 
> 5/6
>   - Use NOR_FLASH_ID_SPANSION from 
> EmbeddedPkg/Include/Library/NorFlashInfoLib.h
> 
> 6/6
>   - Add RB
> 
> Marcin Wojtas (6):
>   Marvell/Drivers: MvSpiFlash: Improve ReadId
>   Marvell/Drivers: MvSpiFlash: Enable dynamic SPI Flash detection
>   Marvell/Drivers: MvSpiFlash: Remove duplicated macros
>   Marvell/Applications: SpiTool: Do not override existing slave device
>   Marvell/Drivers: MvSpiFlash: Fix bank selection for Spansion
>   Marvell/Drivers: MvSpiDxe: Keep data in SPI_DEVICE structure
> 
>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c   |  25 +
>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.inf |   4 +-
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c  |  57 --
>  Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf|   2 +-
>  Platform/Marvell/Armada/Armada.dsc.inc   |   1 +
>  Platform/Marvell/Armada/Armada70x0.dsc   |   5 -
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c| 116 
> ++--
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h|   3 +
>  Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf  |   9 +-
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.c  |  63 ++-
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.h  |   1 +
>  Platform/Marvell/Drivers/Spi/MvSpiDxe.inf|   2 +
>  Platform/Marvell/Include/Protocol/Spi.h  |   7 ++
>  Platform/Marvell/Include/Protocol/SpiFlash.h |  14 +--
>  Platform/Marvell/Marvell.dec |   6 -
>  Silicon/Marvell/Documentation/PortingGuide.txt   |  18 ---
>  16 files changed, 142 insertions(+), 191 deletions(-)
> 
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

[Laszlo Ersek]

sorry about the late response

On 11/03/17 01:57, Jian J Wang wrote:
>> v2
>> a. Fix an issue which will cause setting capability failure if size is 
>> smaller
>>than a page.
> 
> More than one entry of RT_CODE memory might cause boot problem for some
> old OSs. This patch will fix this issue to keep OS compatibility as much
> as possible.
> 
> More detailed information, please refer to
> https://bugzilla.tianocore.org/show_bug.cgi?id=753
> 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c 
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index d312eb66f8..4a7827ebc9 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -809,7 +809,9 @@ RefreshGcdMemoryAttributesFromPaging (
>PageLength= 0;
>  
>for (Index = 0; Index < NumberOfDescriptors; Index++) {
> -if (MemorySpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
> +if (MemorySpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeNonExistent
> +|| (MemorySpaceMap[Index].BaseAddress & EFI_PAGE_MASK) != 0
> +|| (MemorySpaceMap[Index].Length & EFI_PAGE_MASK) != 0) {
>continue;
>  }

When exactly do the new conditions match?

I thought the base addresses and the lengths in the GCD memory space map
are all page aligned. Is that not the case?

If these conditions are just a sanity check (i.e. we never expect them
to fire), then should we perpahs turn them into ASSERT()s?

>  
> @@ -829,6 +831,15 @@ RefreshGcdMemoryAttributesFromPaging (
>  // Sync real page attributes to GCD
>  BaseAddress   = MemorySpaceMap[Index].BaseAddress;
>  MemorySpaceLength = MemorySpaceMap[Index].Length;
> +Capabilities  = MemorySpaceMap[Index].Capabilities |
> +EFI_MEMORY_PAGETYPE_MASK;
> +Status = gDS->SetMemorySpaceCapabilities (
> +BaseAddress,
> +MemorySpaceLength,
> +Capabilities
> +);
> +ASSERT_EFI_ERROR (Status);
> +

OK, so I guess we simply add EFI_MEMORY_PAGETYPE_MASK to the
capabilities of all memory space map entries that have a type different
from non-existent. We discussed it before and (apparently) it is
considered safe.

>  while (MemorySpaceLength > 0) {
>if (PageLength == 0) {
>  PageEntry = GetPageTableEntry (, BaseAddress, 
> );
> @@ -846,7 +857,6 @@ RefreshGcdMemoryAttributesFromPaging (
>  if (Attributes != (MemorySpaceMap[Index].Attributes & 
> EFI_MEMORY_PAGETYPE_MASK)) {
>DoUpdate = TRUE;
>Attributes |= (MemorySpaceMap[Index].Attributes & 
> ~EFI_MEMORY_PAGETYPE_MASK);
> -  Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
>  } else {
>DoUpdate = FALSE;
>  }
> @@ -854,8 +864,8 @@ RefreshGcdMemoryAttributesFromPaging (
>  
>Length = MIN (PageLength, MemorySpaceLength);
>if (DoUpdate) {
> -gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
> -gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
> +Status = gDS->SetMemorySpaceAttributes (BaseAddress, Length, 
> Attributes);
> +ASSERT_EFI_ERROR (Status);
>  DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx - 
> %016lx (%08lx -> %08lx)\r\n",
>   Index, BaseAddress, BaseAddress + Length - 1,
>   MemorySpaceMap[Index].Attributes, Attributes));
> 

I'll let you decide about the EFI_PAGE_MASK conditions near the top.

Acked-by: Laszlo Ersek 

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

Re: [edk2] [PATCH] MdeModulePkg SerialDxe: Handle Timeout change more robustly

[Julien Grall]

Hi Star,

On 07/11/17 01:39, Zeng, Star wrote:

I agree it could be more rigorous to compare against the original parameters.
Please check the V2 patch at 
https://lists.01.org/pipermail/edk2-devel/2017-November/016968.html.

Julien,

Please help take the test on your case with the V2 patch.


Thank you for sending the patch. I will test the v2 and tell you the result.

Cheers,



Thanks,
Star


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

Re: [edk2] [PATCH v3 1/1] EmbeddedPkg: Implement NorFlashInfoLib

[Marcin Wojtas]

2017-11-07 16:50 GMT+01:00 Leif Lindholm :
> On Tue, Nov 07, 2017 at 04:45:19PM +0100, Marcin Wojtas wrote:
>> Hi Leif,
>>
>> 2017-11-07 16:37 GMT+01:00 Leif Lindholm :
>> > On Fri, Nov 03, 2017 at 06:55:17PM +0100, Marcin Wojtas wrote:
>> >> The SPI NOR flash drivers which base on ArmPlatformPkg's
>> >> NorFlashDxe usually make use of static declarations of the
>> >> flash instances with their type and parameters. As a result
>> >> it implies hardcoding the exact way of flash handling, not to
>> >> mention the code does not look very nice. Much better solution
>> >> would be obtaining the flash ID and hence its description
>> >> in runtime.
>> >>
>> >> JEDEC compliant SPI NOR devices allow to obtain their IDs with
>> >> READ_ID command (0x9f), which should return the vendor ID byte,
>> >> followed by 2 to 4 following device ID bytes. Use this capability
>> >> for implementing a NorFlashInfoLib that gives an access to the
>> >> NOR flash description data, such as name, page size, sector
>> >> (block) size and others, of more than 50 different models.
>> >> The new library user should pass an output array from issuing
>> >> READ_ID command to the NorFlashGetInfo () routine - if the
>> >> match is found, an allocated (optionally for RT) pool with
>> >> the flash description copy will be returned.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Marcin Wojtas 
>> >>
>> >> ==
>> >
>> > Please, the separator is ---
>> > The below does not belong in the commit message (but should be
>> > included below --- for clarity).
>> >
>> >
>> > I would however also like to fold in the following hunk, to enable
>> > standalone building:
>> >
>> > diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
>> > index d7ee6a3018..bc19000e4b 100644
>> > --- a/EmbeddedPkg/EmbeddedPkg.dsc
>> > +++ b/EmbeddedPkg/EmbeddedPkg.dsc
>> > @@ -282,6 +282,8 @@ [Components.common]
>> >
>> >EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>> >
>> > +  EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
>> > +
>> >  [Components.ARM]
>> > EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf
>> >
>> > If you are OK with these changes:
>> > Reviewed-by: Leif Lindholm 
>> > (do let me know)
>>
>> Thanks, I'm of course ok with the changes. Should I resubmit?
>
> Nah, that's fine.
>
> Pushed as 7046610163. (no that's not my phone number accidentally
> pasted, it's a short hash containing no a-f)
>

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

Re: [edk2] [PATCH v3 1/1] EmbeddedPkg: Implement NorFlashInfoLib

[Leif Lindholm]

On Tue, Nov 07, 2017 at 04:45:19PM +0100, Marcin Wojtas wrote:
> Hi Leif,
> 
> 2017-11-07 16:37 GMT+01:00 Leif Lindholm :
> > On Fri, Nov 03, 2017 at 06:55:17PM +0100, Marcin Wojtas wrote:
> >> The SPI NOR flash drivers which base on ArmPlatformPkg's
> >> NorFlashDxe usually make use of static declarations of the
> >> flash instances with their type and parameters. As a result
> >> it implies hardcoding the exact way of flash handling, not to
> >> mention the code does not look very nice. Much better solution
> >> would be obtaining the flash ID and hence its description
> >> in runtime.
> >>
> >> JEDEC compliant SPI NOR devices allow to obtain their IDs with
> >> READ_ID command (0x9f), which should return the vendor ID byte,
> >> followed by 2 to 4 following device ID bytes. Use this capability
> >> for implementing a NorFlashInfoLib that gives an access to the
> >> NOR flash description data, such as name, page size, sector
> >> (block) size and others, of more than 50 different models.
> >> The new library user should pass an output array from issuing
> >> READ_ID command to the NorFlashGetInfo () routine - if the
> >> match is found, an allocated (optionally for RT) pool with
> >> the flash description copy will be returned.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas 
> >>
> >> ==
> >
> > Please, the separator is ---
> > The below does not belong in the commit message (but should be
> > included below --- for clarity).
> >
> >
> > I would however also like to fold in the following hunk, to enable
> > standalone building:
> >
> > diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> > index d7ee6a3018..bc19000e4b 100644
> > --- a/EmbeddedPkg/EmbeddedPkg.dsc
> > +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> > @@ -282,6 +282,8 @@ [Components.common]
> >
> >EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
> >
> > +  EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
> > +
> >  [Components.ARM]
> > EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf
> >
> > If you are OK with these changes:
> > Reviewed-by: Leif Lindholm 
> > (do let me know)
> 
> Thanks, I'm of course ok with the changes. Should I resubmit?

Nah, that's fine.

Pushed as 7046610163. (no that's not my phone number accidentally
pasted, it's a short hash containing no a-f)

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

Re: [edk2] [PATCH v3 1/1] EmbeddedPkg: Implement NorFlashInfoLib

[Marcin Wojtas]

Hi Leif,

2017-11-07 16:37 GMT+01:00 Leif Lindholm :
> On Fri, Nov 03, 2017 at 06:55:17PM +0100, Marcin Wojtas wrote:
>> The SPI NOR flash drivers which base on ArmPlatformPkg's
>> NorFlashDxe usually make use of static declarations of the
>> flash instances with their type and parameters. As a result
>> it implies hardcoding the exact way of flash handling, not to
>> mention the code does not look very nice. Much better solution
>> would be obtaining the flash ID and hence its description
>> in runtime.
>>
>> JEDEC compliant SPI NOR devices allow to obtain their IDs with
>> READ_ID command (0x9f), which should return the vendor ID byte,
>> followed by 2 to 4 following device ID bytes. Use this capability
>> for implementing a NorFlashInfoLib that gives an access to the
>> NOR flash description data, such as name, page size, sector
>> (block) size and others, of more than 50 different models.
>> The new library user should pass an output array from issuing
>> READ_ID command to the NorFlashGetInfo () routine - if the
>> match is found, an allocated (optionally for RT) pool with
>> the flash description copy will be returned.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas 
>>
>> ==
>
> Please, the separator is ---
> The below does not belong in the commit message (but should be
> included below --- for clarity).
>
>
> I would however also like to fold in the following hunk, to enable
> standalone building:
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index d7ee6a3018..bc19000e4b 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -282,6 +282,8 @@ [Components.common]
>
>EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
>
> +  EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
> +
>  [Components.ARM]
> EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf
>
> If you are OK with these changes:
> Reviewed-by: Leif Lindholm 
> (do let me know)
>

Thanks, I'm of course ok with the changes. Should I resubmit?

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

Re: [edk2] [PATCH v3 1/1] EmbeddedPkg: Implement NorFlashInfoLib

[Leif Lindholm]

On Fri, Nov 03, 2017 at 06:55:17PM +0100, Marcin Wojtas wrote:
> The SPI NOR flash drivers which base on ArmPlatformPkg's
> NorFlashDxe usually make use of static declarations of the
> flash instances with their type and parameters. As a result
> it implies hardcoding the exact way of flash handling, not to
> mention the code does not look very nice. Much better solution
> would be obtaining the flash ID and hence its description
> in runtime.
> 
> JEDEC compliant SPI NOR devices allow to obtain their IDs with
> READ_ID command (0x9f), which should return the vendor ID byte,
> followed by 2 to 4 following device ID bytes. Use this capability
> for implementing a NorFlashInfoLib that gives an access to the
> NOR flash description data, such as name, page size, sector
> (block) size and others, of more than 50 different models.
> The new library user should pass an output array from issuing
> READ_ID command to the NorFlashGetInfo () routine - if the
> match is found, an allocated (optionally for RT) pool with
> the flash description copy will be returned.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> 
> ==

Please, the separator is ---
The below does not belong in the commit message (but should be
included below --- for clarity).


I would however also like to fold in the following hunk, to enable
standalone building:

diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index d7ee6a3018..bc19000e4b 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -282,6 +282,8 @@ [Components.common]

   EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf

+  EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
+
 [Components.ARM]
EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf

If you are OK with these changes:
Reviewed-by: Leif Lindholm 
(do let me know)

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

Re: [edk2] [PATCH] ShellPkg/dh: Fix wrong output when dumping PciRootBridgeIo

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Monday, November 06, 2017 11:35 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: [edk2] [PATCH] ShellPkg/dh: Fix wrong output when dumping
> PciRootBridgeIo
> Importance: High
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jaben Carsey 
> ---
>  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> index d343f3352e..b7b0246ac9 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> @@ -682,7 +682,7 @@ PciRootBridgeIoDumpInformation(
>  break;
>}
>if (Temp != NULL) {
> -Temp2 = CatSPrint(RetVal, L"%s", Temp);
> +Temp2 = CatSPrint(RetVal, L"\r\n%s", Temp);
>  FreePool(Temp);
>  FreePool(RetVal);
>  RetVal = Temp2;
> @@ -690,7 +690,7 @@ PciRootBridgeIoDumpInformation(
>}
> 
>Temp2 = CatSPrint(RetVal,
> -L"\r\n%%H%02x%016lx  %016lx  %02x%%N",
> +L"%%H%02x%016lx  %016lx  %02x%%N",
>  Configuration->SpecificFlag,
>  Configuration->AddrRangeMin,
>  Configuration->AddrRangeMax,
> --
> 2.12.2.windows.2
> 
> ___
> 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] GRUB issue on device priority

[Haojian Zhuang]

Hi all,

It seems there's a device priority issue in GRUB.

All block io handles are linked into the list in edk2, and GRUB could 
fetch it. Then GRUB creates its own ascending on HD priority.

add_device():
  for (p = devices; *p; p = &((*p)->next)) {
ret = grub_efi_compare_device_paths (grub_efi_find_last_device_path 
((*p->device_path),

 grub_efi_find_last_device_path->device_path));
if (ret == 0) {
  ret = grub_efi_compare_device_paths ((*p)->device_path,
   d->device_path);
}
if (ret == 0) {
  return;
} else if (ret > 0) {
  break;
}
  }
  ...

In the HiKey platform, I prepared the same driver for both eMMC and SD. 
So the device paths are in below.

SD: /HardwareVendor(0d51905b-b77e-452a-a2c0-eca0
cc8d514a)[9: 00 e0 23 f7 00 00 00 00 00 ]/UnknownMessaging(1a)/EndEntire
eMMC: /HardwareVendor(0d51905b-b77e-452a-a2c0-eca0
cc8d514a)[9: 00 d0 23 f7 00 00 00 00 00 
]/UnknownMessaging(1d)/Ctrl(0)/EndEntire


#define MSG_SD_DP   0x1A
#define MSG_EMMC_DP 0x1D

In the second level, the device paths are different.

And GRUB resort the sequence by ascending order (with above code). So SD 
device always gets higher priority than eMMC device.


If we always use installer to install OS, it may not an issue. Since 
installer could create grub.cfg by itself. But it imports another issue 
on lacking of persistent variable storage. And we need to deploy system 
without installer on embedded device.


How to fix the priority issue? Fix in GRUB or something else?

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

Re: [edk2] [Help] UEFI boot KVM4T vm hang On TianoCore

[Laszlo Ersek]

On 11/07/17 03:39, Hangaohuai wrote:
> Hi, Laszlo Ersek;
> Thanks for your reply.
> 
> I have trying to shoot this trouble:
> 
> 1. edk get the MtrrValidBitsMask by "AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, 
> , NULL, NULL, NULL); "(0x8008)
> 2. Cpuid(0x8008) is held in kvm struct (>arch.cpuid_entries[i])
> 3. (0x8008) is set by qemu;
> 
> Part1:
> We set the 4294967296, the 
> "GetFirstNonAddress: Pci64Base=0x448"
> But the MtrrValidBitsMask get 0x3FF  ;
> Does not pass the check.
>   MtrrLibInitializeMtrrMask (, );
>   if (((BaseAddress & ~MtrrValidAddressMask) != 0) || (Length & 
> ~MtrrValidAddressMask) != 0) {
> return RETURN_UNSUPPORTED;
>   }
> 
> Part2:
> Xhci_hcd will use the address above maxMemory, so we visit the wrong address.
> Checking by starting a  unit='KiB'>3294967296 memory with USB3.0.
> 
> So , I think edk2 is ok; maxMemory depend on CPU support more the 42bit.

That's right; if you look at /proc/cpuinfo on your KVM host, it will say
something like:

address sizes   : 39 bits physical, 48 bits virtual
  

(the actual number will vary).

With EPT enabled (see "/sys/module/kvm_intel/parameters/ept"), the guest
cannot have a larger guest-phys address space than what's supported by
the host CPU's physical address width.

Thanks,
Laszlo


> 
> 
> ***
> Hi,
> 
> sorry about the delayed response.
> 
> CC'ing Igor, and commenting below:
> 
> On 10/31/17 15:29, Hangaohuai wrote:
>> Hi, Laszlo Ersek;
>>
>> I have tested the uefi booting KVM vm with the configuration(xml); but 
>> start hang.
>> Enable the memoryhotplug, with usb3.0 config. The config as Config1 
>> below.
>>
>> Tested branches:
>> UDK2017 eea98eea4ccbb1d640657770bccb5497fddc6064
>> Master   76fd5a660d704538a1b14a58d03a4eef9682b01c
>>
>> Both hang on the snapshot TianoCore in VNC
>>
>> Try to shoot the problem;
>> I find the early version can boot success with the same config.
>> Maybe the patch1 below cause the problem;
>>
>> Try to ignore the patch does, the master/ UDK2017 both can boot 
>> success.
>> But I don't know why. Hope for your help,thanks.
>>
>> Patch1
>> ***
>> commit 4f5eff8193096eb847639f090a7dfae3cff95fde
>> Author: Laszlo Ersek 
>> Date:   Fri Mar 4 20:06:26 2016 +0100
>>
>> OvmfPkg: PciHostBridgeLib: install 64-bit PCI host aperture
>>
>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c 
>> b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib
>> index 3e02778..1d3d10a 100644
>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> @@ -132,6 +132,13 @@ InitRootBridge (
>>RootBus->MemAbove4G.Base  = 0;
>>RootBus->MemAbove4G.Limit = 0;
>> +  if (PcdGet64 (PcdPciMmio64Size) > 0) {
>> +RootBus->AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
>> +RootBus->MemAbove4G.Base   = PcdGet64 (PcdPciMmio64Base);
>> +RootBus->MemAbove4G.Limit  = PcdGet64 (PcdPciMmio64Base) +
>> + (PcdGet64 (PcdPciMmio64Size) - 
>> + 1);  }
>> +
>>RootBus->Bus.Base  = RootBusNumber;
>>RootBus->Bus.Limit = MaxSubBusNumber;
>>RootBus->Io.Base   = PcdGet64 (PcdPciIoBase);
>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf 
>> b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeL
>> index bbec746..7a964c7 100644
>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> @@ -51,4 +51,6 @@
>>gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
>>gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base
>>gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
>> ***
>>
>>
>> Config1
>> ***
>> 4294967296
>>   4294967
>>   4294967
>> Xxxx
>> 
>> > function='0x0'/>  
>> ***
> 
> The patch that you identified is responsible for exposing the 64-bit MMIO 
> aperture for PCI MMIO BAR allocation purposes.
> 
> Reverting this patch might mask the issue, but I don't expect the actual 
> problem to be in this patch.
> 
> Instead, the base address and the size of the aperture in question is 
> determined in the file
> 
>   OvmfPkg/PlatformPei/MemDetect.c
> 
> in the function
> 
>   GetFirstNonAddress()
> 
> This function takes the memory (DIMM) hotplug range in question, the end of 
> which is presented by QEMU in the fw_cfg file
> 
>   etc/reserved-memory-end
> 
> To my knowledge, this fw_cfg file is an 8-byte integer in LE encoding.
> 
> Quoting the code:
> 
>>   //
>>   // If QEMU presents an E820 map, then get the highest exclusive >=4GB RAM
>>   // address from it. This can express an 

Re: [edk2] [PATCH] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE

[Ard Biesheuvel]

On 7 November 2017 at 13:29, Leif Lindholm  wrote:
> On Tue, Nov 07, 2017 at 01:22:19PM +, Ard Biesheuvel wrote:
>> On 7 November 2017 at 12:56, Heyi Guo  wrote:
>> > From: Peicong Li 
>> >
>> > Flash region needs to be set as cacheable (write back) to increase
>> > performance, if PEI is still XIP on flash or DXE FV is decompressed
>> > from flash FV. However some ARM platforms do not support to set flash
>> > as inner shareable since flash is not normal DDR memory and it will
>> > not respond to cache snoop request, which will causes system hang
>> > after MMU is enabled.
>> >
>> > So we need a new ARM memory region attribute WRITE_BACK_NONSHAREABLE
>> > for flash region on these platforms specifically. This attribute will
>> > set the region as write back but not inner shared.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Peicong Li 
>> > Signed-off-by: Heyi Guo 
>> > Cc: Leif Lindholm 
>> > Cc: Ard Biesheuvel 
>>
>> Reviewed-by: Ard Biesheuvel 
>
> Reviewed-by: Leif Lindholm 
>

Pushed as 829633e3a82d. Thanks!

>> > ---
>> >  ArmPkg/Include/Library/ArmLib.h  | 7 +++
>> >  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
>> >  2 files changed, 11 insertions(+)
>> >
>> > diff --git a/ArmPkg/Include/Library/ArmLib.h 
>> > b/ArmPkg/Include/Library/ArmLib.h
>> > index 24ffe9f..38199be 100644
>> > --- a/ArmPkg/Include/Library/ArmLib.h
>> > +++ b/ArmPkg/Include/Library/ArmLib.h
>> > @@ -41,6 +41,13 @@ typedef enum {
>> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
>> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
>> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
>> > +  // On some platforms, memory mapped flash region is designed as not 
>> > supporting
>> > +  // shareable attribute, so WRITE_BACK_NONSHAREABLE is added for such 
>> > special
>> > +  // need.
>> > +  // Do NOT use below two attributes if you are not sure.
>> > +  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE,
>> > +  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE,
>> > +
>> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
>> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH,
>> >ARM_MEMORY_REGION_ATTRIBUTE_DEVICE,
>> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
>> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> > index 8bd1c6f..4b62ecb 100644
>> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> > @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
>> >)
>> >  {
>> >switch (Attributes) {
>> > +  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
>> > +  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
>> > +return TT_ATTR_INDX_MEMORY_WRITE_BACK;
>> > +
>> >case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
>> >case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
>> >  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
>> > --
>> > 2.7.2.windows.1
>> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE

[Leif Lindholm]

On Tue, Nov 07, 2017 at 01:22:19PM +, Ard Biesheuvel wrote:
> On 7 November 2017 at 12:56, Heyi Guo  wrote:
> > From: Peicong Li 
> >
> > Flash region needs to be set as cacheable (write back) to increase
> > performance, if PEI is still XIP on flash or DXE FV is decompressed
> > from flash FV. However some ARM platforms do not support to set flash
> > as inner shareable since flash is not normal DDR memory and it will
> > not respond to cache snoop request, which will causes system hang
> > after MMU is enabled.
> >
> > So we need a new ARM memory region attribute WRITE_BACK_NONSHAREABLE
> > for flash region on these platforms specifically. This attribute will
> > set the region as write back but not inner shared.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Peicong Li 
> > Signed-off-by: Heyi Guo 
> > Cc: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> 
> Reviewed-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> > ---
> >  ArmPkg/Include/Library/ArmLib.h  | 7 +++
> >  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/ArmPkg/Include/Library/ArmLib.h 
> > b/ArmPkg/Include/Library/ArmLib.h
> > index 24ffe9f..38199be 100644
> > --- a/ArmPkg/Include/Library/ArmLib.h
> > +++ b/ArmPkg/Include/Library/ArmLib.h
> > @@ -41,6 +41,13 @@ typedef enum {
> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
> > +  // On some platforms, memory mapped flash region is designed as not 
> > supporting
> > +  // shareable attribute, so WRITE_BACK_NONSHAREABLE is added for such 
> > special
> > +  // need.
> > +  // Do NOT use below two attributes if you are not sure.
> > +  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE,
> > +  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE,
> > +
> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH,
> >ARM_MEMORY_REGION_ATTRIBUTE_DEVICE,
> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > index 8bd1c6f..4b62ecb 100644
> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
> >)
> >  {
> >switch (Attributes) {
> > +  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
> > +  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
> > +return TT_ATTR_INDX_MEMORY_WRITE_BACK;
> > +
> >case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
> >case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
> >  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
> > --
> > 2.7.2.windows.1
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE

[Ard Biesheuvel]

On 7 November 2017 at 12:56, Heyi Guo  wrote:
> From: Peicong Li 
>
> Flash region needs to be set as cacheable (write back) to increase
> performance, if PEI is still XIP on flash or DXE FV is decompressed
> from flash FV. However some ARM platforms do not support to set flash
> as inner shareable since flash is not normal DDR memory and it will
> not respond to cache snoop request, which will causes system hang
> after MMU is enabled.
>
> So we need a new ARM memory region attribute WRITE_BACK_NONSHAREABLE
> for flash region on these platforms specifically. This attribute will
> set the region as write back but not inner shared.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Peicong Li 
> Signed-off-by: Heyi Guo 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 

Reviewed-by: Ard Biesheuvel 


> ---
>  ArmPkg/Include/Library/ArmLib.h  | 7 +++
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
>  2 files changed, 11 insertions(+)
>
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 24ffe9f..38199be 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -41,6 +41,13 @@ typedef enum {
>ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
>ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
>ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
> +  // On some platforms, memory mapped flash region is designed as not 
> supporting
> +  // shareable attribute, so WRITE_BACK_NONSHAREABLE is added for such 
> special
> +  // need.
> +  // Do NOT use below two attributes if you are not sure.
> +  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE,
> +  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE,
> +
>ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
>ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH,
>ARM_MEMORY_REGION_ATTRIBUTE_DEVICE,
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 8bd1c6f..4b62ecb 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
>)
>  {
>switch (Attributes) {
> +  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
> +  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
> +return TT_ATTR_INDX_MEMORY_WRITE_BACK;
> +
>case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
>case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
>  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
> --
> 2.7.2.windows.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmPkg/ArmMmuLib: Add new attribute

[Heyi Guo]

V1 changes upon RFC version:
1. Rename NONSHARE to NONSHAREABLE per review comments.
2. Place these two uncommon attributes after normal WRITE_BACK attributes to 
imply they should not be used by default.
3. Add warnning comments.

Peicong Li (1):
  ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE

 ArmPkg/Include/Library/ArmLib.h  | 7 +++
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
 2 files changed, 11 insertions(+)

-- 
2.7.2.windows.1

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

[edk2] [PATCH] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE

[Heyi Guo]

From: Peicong Li 

Flash region needs to be set as cacheable (write back) to increase
performance, if PEI is still XIP on flash or DXE FV is decompressed
from flash FV. However some ARM platforms do not support to set flash
as inner shareable since flash is not normal DDR memory and it will
not respond to cache snoop request, which will causes system hang
after MMU is enabled.

So we need a new ARM memory region attribute WRITE_BACK_NONSHAREABLE
for flash region on these platforms specifically. This attribute will
set the region as write back but not inner shared.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Peicong Li 
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
 ArmPkg/Include/Library/ArmLib.h  | 7 +++
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
 2 files changed, 11 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 24ffe9f..38199be 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -41,6 +41,13 @@ typedef enum {
   ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
   ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
+  // On some platforms, memory mapped flash region is designed as not 
supporting
+  // shareable attribute, so WRITE_BACK_NONSHAREABLE is added for such special
+  // need.
+  // Do NOT use below two attributes if you are not sure.
+  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE,
+  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE,
+
   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
   ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH,
   ARM_MEMORY_REGION_ATTRIBUTE_DEVICE,
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 8bd1c6f..4b62ecb 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
   )
 {
   switch (Attributes) {
+  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
+  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
+return TT_ATTR_INDX_MEMORY_WRITE_BACK;
+
   case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
   case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
 return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
-- 
2.7.2.windows.1

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

Re: [edk2] [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

[Heyi Guo]

在 11/7/2017 7:25 PM, Ard Biesheuvel 写道:

On 7 November 2017 at 11:23, Leif Lindholm  wrote:

On Tue, Nov 07, 2017 at 11:08:39AM +, Ard Biesheuvel wrote:

On 7 November 2017 at 11:05, Heyi Guo  wrote:

From: Peicong Li 

Flash region needs to be set as cacheable (write back) to increase
performance, if PEI is still XIP on flash or DXE FV is decompressed
from flash FV. However some ARM platforms do not support to set flash
as inner shareable since flash is not normal DDR memory and it will
not respond to cache snoop request, which will causes system hang
after MMU is enabled.

So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for
flash region on these platforms specifically. This attribute will set
the region as write back but not inner shared.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Peicong Li 
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
  ArmPkg/Include/Library/ArmLib.h  | 2 ++
  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
  2 files changed, 6 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 24ffe9f..e43e375 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -39,6 +39,8 @@
  typedef enum {
ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0,
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
+  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE,
+  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE,
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 8bd1c6f..cc10143 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
)
  {
switch (Attributes) {
+  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE:
+  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE:
+return TT_ATTR_INDX_MEMORY_WRITE_BACK;
+
case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
--
2.7.2.windows.1


I'd prefer the name
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE but other than
that, this looks sensible to me. Leif?

And the same for NONSECURE, yes.
With that modification, this sounds like something absolutely required
in this situation.

Does this scenario have any further implications for runtime use?


I don't think so. These attributes are only used to select the
attributes UEFI uses for its own mapping, and they should only be used
for non-DRAM, so they shouldn't leak into the UEFI memory map in a way
the OS would be able to notice.
Agree, and we will convert flash region attribute into "UC" to support 
flash write and erase operation in DXE.


We may also add some comment in the code to warn the usage of these two 
types :)


Thanks,

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

Re: [edk2] [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

[Leif Lindholm]

On Tue, Nov 07, 2017 at 11:25:16AM +, Ard Biesheuvel wrote:
> >> I'd prefer the name
> >> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE but other than
> >> that, this looks sensible to me. Leif?
> >
> > And the same for NONSECURE, yes.
> > With that modification, this sounds like something absolutely required
> > in this situation.
> >
> > Does this scenario have any further implications for runtime use?
> 
> I don't think so. These attributes are only used to select the
> attributes UEFI uses for its own mapping, and they should only be used
> for non-DRAM, so they shouldn't leak into the UEFI memory map in a way
> the OS would be able to notice.

Yes, that makes sense. Just wanted to double-check.

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

Re: [edk2] [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

[Ard Biesheuvel]

On 7 November 2017 at 11:23, Leif Lindholm  wrote:
> On Tue, Nov 07, 2017 at 11:08:39AM +, Ard Biesheuvel wrote:
>> On 7 November 2017 at 11:05, Heyi Guo  wrote:
>> > From: Peicong Li 
>> >
>> > Flash region needs to be set as cacheable (write back) to increase
>> > performance, if PEI is still XIP on flash or DXE FV is decompressed
>> > from flash FV. However some ARM platforms do not support to set flash
>> > as inner shareable since flash is not normal DDR memory and it will
>> > not respond to cache snoop request, which will causes system hang
>> > after MMU is enabled.
>> >
>> > So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for
>> > flash region on these platforms specifically. This attribute will set
>> > the region as write back but not inner shared.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Peicong Li 
>> > Signed-off-by: Heyi Guo 
>> > Cc: Leif Lindholm 
>> > Cc: Ard Biesheuvel 
>> > ---
>> >  ArmPkg/Include/Library/ArmLib.h  | 2 ++
>> >  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
>> >  2 files changed, 6 insertions(+)
>> >
>> > diff --git a/ArmPkg/Include/Library/ArmLib.h 
>> > b/ArmPkg/Include/Library/ArmLib.h
>> > index 24ffe9f..e43e375 100644
>> > --- a/ArmPkg/Include/Library/ArmLib.h
>> > +++ b/ArmPkg/Include/Library/ArmLib.h
>> > @@ -39,6 +39,8 @@
>> >  typedef enum {
>> >ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0,
>> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
>> > +  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE,
>> > +  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE,
>> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
>> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
>> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
>> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
>> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> > index 8bd1c6f..cc10143 100644
>> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
>> > @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
>> >)
>> >  {
>> >switch (Attributes) {
>> > +  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE:
>> > +  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE:
>> > +return TT_ATTR_INDX_MEMORY_WRITE_BACK;
>> > +
>> >case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
>> >case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
>> >  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
>> > --
>> > 2.7.2.windows.1
>> >
>>
>> I'd prefer the name
>> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE but other than
>> that, this looks sensible to me. Leif?
>
> And the same for NONSECURE, yes.
> With that modification, this sounds like something absolutely required
> in this situation.
>
> Does this scenario have any further implications for runtime use?
>

I don't think so. These attributes are only used to select the
attributes UEFI uses for its own mapping, and they should only be used
for non-DRAM, so they shouldn't leak into the UEFI memory map in a way
the OS would be able to notice.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

[Leif Lindholm]

On Tue, Nov 07, 2017 at 11:08:39AM +, Ard Biesheuvel wrote:
> On 7 November 2017 at 11:05, Heyi Guo  wrote:
> > From: Peicong Li 
> >
> > Flash region needs to be set as cacheable (write back) to increase
> > performance, if PEI is still XIP on flash or DXE FV is decompressed
> > from flash FV. However some ARM platforms do not support to set flash
> > as inner shareable since flash is not normal DDR memory and it will
> > not respond to cache snoop request, which will causes system hang
> > after MMU is enabled.
> >
> > So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for
> > flash region on these platforms specifically. This attribute will set
> > the region as write back but not inner shared.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Peicong Li 
> > Signed-off-by: Heyi Guo 
> > Cc: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> > ---
> >  ArmPkg/Include/Library/ArmLib.h  | 2 ++
> >  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/ArmPkg/Include/Library/ArmLib.h 
> > b/ArmPkg/Include/Library/ArmLib.h
> > index 24ffe9f..e43e375 100644
> > --- a/ArmPkg/Include/Library/ArmLib.h
> > +++ b/ArmPkg/Include/Library/ArmLib.h
> > @@ -39,6 +39,8 @@
> >  typedef enum {
> >ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0,
> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
> > +  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE,
> > +  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE,
> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
> >ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
> >ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > index 8bd1c6f..cc10143 100644
> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> > @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
> >)
> >  {
> >switch (Attributes) {
> > +  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE:
> > +  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE:
> > +return TT_ATTR_INDX_MEMORY_WRITE_BACK;
> > +
> >case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
> >case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
> >  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
> > --
> > 2.7.2.windows.1
> >
> 
> I'd prefer the name
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE but other than
> that, this looks sensible to me. Leif?

And the same for NONSECURE, yes.
With that modification, this sounds like something absolutely required
in this situation.

Does this scenario have any further implications for runtime use?

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

Re: [edk2] [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

[Ard Biesheuvel]

On 7 November 2017 at 11:05, Heyi Guo  wrote:
> From: Peicong Li 
>
> Flash region needs to be set as cacheable (write back) to increase
> performance, if PEI is still XIP on flash or DXE FV is decompressed
> from flash FV. However some ARM platforms do not support to set flash
> as inner shareable since flash is not normal DDR memory and it will
> not respond to cache snoop request, which will causes system hang
> after MMU is enabled.
>
> So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for
> flash region on these platforms specifically. This attribute will set
> the region as write back but not inner shared.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Peicong Li 
> Signed-off-by: Heyi Guo 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> ---
>  ArmPkg/Include/Library/ArmLib.h  | 2 ++
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
>  2 files changed, 6 insertions(+)
>
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 24ffe9f..e43e375 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -39,6 +39,8 @@
>  typedef enum {
>ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0,
>ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
> +  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE,
> +  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE,
>ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
>ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
>ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 8bd1c6f..cc10143 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
>)
>  {
>switch (Attributes) {
> +  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE:
> +  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE:
> +return TT_ATTR_INDX_MEMORY_WRITE_BACK;
> +
>case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
>case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
>  return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
> --
> 2.7.2.windows.1
>

I'd prefer the name
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE but other than
that, this looks sensible to me. Leif?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

[Heyi Guo]

From: Peicong Li 

Flash region needs to be set as cacheable (write back) to increase
performance, if PEI is still XIP on flash or DXE FV is decompressed
from flash FV. However some ARM platforms do not support to set flash
as inner shareable since flash is not normal DDR memory and it will
not respond to cache snoop request, which will causes system hang
after MMU is enabled.

So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for
flash region on these platforms specifically. This attribute will set
the region as write back but not inner shared.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Peicong Li 
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
 ArmPkg/Include/Library/ArmLib.h  | 2 ++
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 
 2 files changed, 6 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 24ffe9f..e43e375 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -39,6 +39,8 @@
 typedef enum {
   ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0,
   ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED,
+  ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE,
+  ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE,
   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK,
   ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK,
   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH,
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 8bd1c6f..cc10143 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute (
   )
 {
   switch (Attributes) {
+  case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE:
+  case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE:
+return TT_ATTR_INDX_MEMORY_WRITE_BACK;
+
   case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
   case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
 return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
-- 
2.7.2.windows.1

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

Re: [edk2] Build error in StdLib with VS 2015 compiler

[Karunakar P]

It works Fine.
Are you going to check in this changes, could you please let us know the plan?

-Original Message-
From: Gao, Liming [mailto:liming@intel.com] 
Sent: Monday, November 06, 2017 6:59 PM
To: Karunakar P; 'Tim Lewis'; 'edk2-devel@lists.01.org'
Cc: Ye, Ting; Fu, Siyuan; Wu, Jiaxin
Subject: RE: [edk2] Build error in StdLib with VS 2015 compiler

For VS2015, don't define it. The change is like below. 

#ifndef __STDC_HOSTED__
#if !defined(_MSC_VER) || _MSC_VER != 1900 #define __STDC_HOSTED__ 1 #endif 
#endif

> -Original Message-
> From: Karunakar P [mailto:karunak...@amiindia.co.in]
> Sent: Monday, November 6, 2017 5:32 PM
> To: Gao, Liming ; 'Tim Lewis' 
> ; 'edk2-devel@lists.01.org' 
> 
> Cc: Ye, Ting ; Fu, Siyuan ; 
> Wu, Jiaxin 
> Subject: RE: [edk2] Build error in StdLib with VS 2015 compiler
> 
> What do you say...
> 
> 
> Thanks,
> Karunakar
> 
> -Original Message-
> From: Karunakar P
> Sent: Thursday, November 02, 2017 8:17 PM
> To: 'Gao, Liming'; 'Tim Lewis'; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan; Wu, Jiaxin
> Subject: RE: [edk2] Build error in StdLib with VS 2015 compiler
> 
> Then it works fine.
> But the problem is it may miss the backward compatibility with other 
> compilers.
> 
> I guess better to define as mentioned previously.
> 
> Thank You,
> Karunakar
> 
> -Original Message-
> From: Gao, Liming [mailto:liming@intel.com]
> Sent: Thursday, November 02, 2017 7:33 PM
> To: Karunakar P; 'Tim Lewis'; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan; Wu, Jiaxin
> Subject: RE: [edk2] Build error in StdLib with VS 2015 compiler
> 
> Could you remove them, and build again?
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Karunakar P
> > Sent: Thursday, November 2, 2017 8:32 PM
> > To: 'Tim Lewis' ; edk2-devel@lists.01.org
> > Cc: Ye, Ting ; Fu, Siyuan ; 
> > Wu, Jiaxin 
> > Subject: Re: [edk2] Build error in StdLib with VS 2015 compiler
> >
> > It is C only.
> >
> > -Karunakar
> >
> > -Original Message-
> > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > Sent: Thursday, November 02, 2017 8:59 AM
> > To: Karunakar P; edk2-devel@lists.01.org
> > Cc: 'Ye, Ting'; 'Fu, Siyuan'; 'Wu, Jiaxin'
> > Subject: RE: [edk2] Build error in StdLib with VS 2015 compiler
> >
> > Are you building C++ (.cpp)? Tim
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Karunakar P
> > Sent: Wednesday, November 1, 2017 8:27 PM
> > To: 'edk2-devel@lists.01.org' 
> > Cc: 'Ye, Ting' ; 'Fu, Siyuan' 
> > ; 'Wu, Jiaxin' 
> > Subject: Re: [edk2] Build error in StdLib with VS 2015 compiler
> >
> > Any comment on this?
> >
> > From: Karunakar P
> > Sent: Tuesday, October 31, 2017 3:18 PM
> > To: 'edk2-devel@lists.01.org'
> > Cc: 'Wu, Jiaxin'; 'Fu, Siyuan'; 'Ye, Ting'
> > Subject: Build error in StdLib with VS 2015 compiler
> >
> > Hello All,
> >
> > Facing an build error with Stdlib module when built with VS 2015 compiler.
> >
> > e:\test\StdLib\Include\sys/EfiCdefs.h(357): error C2220: warning 
> > treated as error - no 'object' file generated
> > e:\test\StdLib\Include\sys/EfiCdefs.h(357): warning C4117: macro 
> > name '__STDC_HOSTED__' is reserved, '#define' ignored
> >
> > below change resolving this error. Would you please review and provide 
> > comments.
> >
> > #ifndef __STDC_HOSTED__
> > #define __STDC_HOSTED__ 1
> > #endif
> >
> >
> > Thank You,
> > Karunakar
> > ___
> > 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] MdeModulePkg/NonDiscoverable: fix memory override bug

[Heyi Guo]

Hi Ray,

We had Ard's R-B already; could you help to commit it?

Thanks and regards,

Heyi


在 10/30/2017 4:14 PM, Ard Biesheuvel 写道:

On 30 October 2017 at 05:47, Heyi Guo  wrote:

For PciIoPciRead interface, memory prior to Buffer would be written
with zeros if Offset was larger than sizeof (Dev->ConfigSpace), which
would cause serious system exception.

So we add a pre-check branch to avoid memory override.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 

Reviewed-by: Ard Biesheuvel 


---
  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c | 5 +
  1 file changed, 5 insertions(+)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index c836ad6..0e42ae4 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -465,6 +465,11 @@ PciIoPciRead (
Address = (UINT8 *)>ConfigSpace + Offset;
Length = Count << ((UINTN)Width & 0x3);

+  if (Offset >= sizeof (Dev->ConfigSpace)) {
+ZeroMem (Buffer, Length);
+return EFI_SUCCESS;
+  }
+
if (Offset + Length > sizeof (Dev->ConfigSpace)) {
  //
  // Read all zeroes for config space accesses beyond the first
--
1.9.1



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

[edk2] [PATCH 10/10] Compilation : Add the fdf, dsc and dec files.

[Meenakshi Aggarwal]

The firware device, description and declaration files.
Build script and Environment setup script.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Env.cshrc   |  75 +
 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec |  29 ++
 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc |  74 +
 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 279 +
 Platform/NXP/NxpQoriqLs.dec  | 257 +++
 Platform/NXP/NxpQoriqLs.dsc  | 453 +++
 Platform/NXP/Readme.md   |  14 +
 Platform/NXP/build.sh| 100 ++
 Silicon/NXP/Chassis/Chassis2/Chassis2.dec|  19 ++
 Silicon/NXP/LS1043A/LS1043A.dec  |  22 ++
 Silicon/NXP/LS1043A/LS1043A.dsc  |  82 +
 11 files changed, 1404 insertions(+)
 create mode 100644 Platform/NXP/Env.cshrc
 create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec
 create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
 create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
 create mode 100644 Platform/NXP/NxpQoriqLs.dec
 create mode 100644 Platform/NXP/NxpQoriqLs.dsc
 create mode 100644 Platform/NXP/Readme.md
 create mode 100755 Platform/NXP/build.sh
 create mode 100644 Silicon/NXP/Chassis/Chassis2/Chassis2.dec
 create mode 100644 Silicon/NXP/LS1043A/LS1043A.dec
 create mode 100644 Silicon/NXP/LS1043A/LS1043A.dsc

diff --git a/Platform/NXP/Env.cshrc b/Platform/NXP/Env.cshrc
new file mode 100644
index 000..9b7f261
--- /dev/null
+++ b/Platform/NXP/Env.cshrc
@@ -0,0 +1,75 @@
+#  @file.
+#
+#  Copyright 2017 NXP
+#
+#  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.
+#
+#
+
+unset GCC_UTILITY GCC_VERSION MajorVersion MinorVersion
+
+if [ X"$CROSS_COMPILE_64" != X"" ]; then
+  ARM64_PREFIX="$CROSS_COMPILE_64"
+elif [ X"$CROSS_COMPILE" != X"" ]; then
+  ARM64_PREFIX="$CROSS_COMPILE"
+else
+  ARM64_PREFIX="aarch64-linux-gnu-"
+fi
+
+GCC_UTILITY="${ARM64_PREFIX}gcc"
+CheckGcc=`which $GCC_UTILITY >/dev/null 2>&1`
+if [ "$?" -eq 0 ];then
+  GCC_VERSION=`$GCC_UTILITY -v 2>&1 | tail -n 1 | awk '{print $3}'`
+  MajorVersion=`echo $GCC_VERSION | cut -d . -f 1`
+  MinorVersion=`echo $GCC_VERSION | cut -d . -f 2`
+  GCC_ARCH_PREFIX=
+  NOTSUPPORTED=0
+
+  case $MajorVersion in
+4)
+  case $MinorVersion in
+9)
+  GCC_ARCH_PREFIX="GCC49_AARCH64_PREFIX"
+;;
+*)
+  NOTSUPPORTED=1
+;;
+  esac
+;;
+5)
+  case $MinorVersion in
+  4)
+GCC_ARCH_PREFIX="GCC5_AARCH64_PREFIX"
+  ;;
+  *)
+GCC_ARCH_PREFIX="GCC5_AARCH64_PREFIX"
+echo "Warning: ${GCC_UTILITY} version ($MajorVersion.$MinorVersion) 
has not been tested, please use at own risk."
+  ;;
+  esac
+;;
+*)
+  NOTSUPPORTED=1
+;;
+  esac
+
+  [ "$NOTSUPPORTED" -eq 1 ] && {
+  echo "Error: ${GCC_UTILITY} version ($MajorVersion.$MinorVersion) not 
supported ."
+  unset GCC_UTILITY GCC_VERSION MajorVersion MinorVersion
+  }
+
+  [ -n "$GCC_ARCH_PREFIX" ] && {
+export GCC_ARCH_PREFIX="$GCC_ARCH_PREFIX"
+export "$GCC_ARCH_PREFIX=$ARM64_PREFIX"
+  }
+
+  unset ARCH
+else
+echo "Error: ${GCC_UTILITY} not found. Please check PATH variable."
+unset GCC_UTILITY GCC_VERSION MajorVersion MinorVersion
+fi
diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec 
b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec
new file mode 100644
index 000..1b639e2
--- /dev/null
+++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec
@@ -0,0 +1,29 @@
+#  LS1043aRdbPkg.dec
+#  LS1043a board package.
+#
+#  Copyright 2017 NXP
+#
+#  This program and the accompanying materials are licensed and made available 
under
+#  the terms and conditions of the BSD License which accompanies this 
distribution.
+#  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+
+[Defines]
+  PACKAGE_NAME   = LS1043aRdbPkg
+  PACKAGE_GUID   = 6eba6648-d853-4eb3-9761-528b82d5ab04
+
+
+#
+# Include Section - list of Include Paths that are provided by this package.
+#   Comments are used for Keywords and Module Types.
+#
+# Supported Module Types:
+#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER 

[edk2] [PATCH 09/10] SocLib : Add support for initialization of peripherals

[Meenakshi Aggarwal]

Add SocInit function that initializes peripherals
and print board and soc information.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Include/Bitops.h   | 179 +++
 Silicon/NXP/Chassis/Chassis.c   | 393 
 Silicon/NXP/Chassis/Chassis.h   | 123 ++
 Silicon/NXP/Chassis/Chassis2/SerDes.h   |  82 +++
 Silicon/NXP/Chassis/Chassis2/Soc.c  | 146 
 Silicon/NXP/Chassis/Chassis2/Soc.h  | 376 ++
 Silicon/NXP/Chassis/LS1043aSocLib.inf   |  48 
 Silicon/NXP/Chassis/SerDes.c| 253 
 Silicon/NXP/LS1043A/Include/SocSerDes.h |  55 +
 9 files changed, 1655 insertions(+)
 create mode 100644 Platform/NXP/Include/Bitops.h
 create mode 100644 Silicon/NXP/Chassis/Chassis.c
 create mode 100644 Silicon/NXP/Chassis/Chassis.h
 create mode 100644 Silicon/NXP/Chassis/Chassis2/SerDes.h
 create mode 100644 Silicon/NXP/Chassis/Chassis2/Soc.c
 create mode 100644 Silicon/NXP/Chassis/Chassis2/Soc.h
 create mode 100644 Silicon/NXP/Chassis/LS1043aSocLib.inf
 create mode 100644 Silicon/NXP/Chassis/SerDes.c
 create mode 100644 Silicon/NXP/LS1043A/Include/SocSerDes.h

diff --git a/Platform/NXP/Include/Bitops.h b/Platform/NXP/Include/Bitops.h
new file mode 100644
index 000..90e6404
--- /dev/null
+++ b/Platform/NXP/Include/Bitops.h
@@ -0,0 +1,179 @@
+/** Bitops.h
+  Header defining the general bitwise operations
+
+  Copyright 2017 NXP
+
+  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 __BITOPS_H__
+#define __BITOPS_H__
+
+#include 
+
+#define MASK_LOWER_16  0x
+#define MASK_UPPER_16  0x
+#define MASK_LOWER_8   0xFF00
+#define MASK_UPPER_8   0x00FF
+
+/*
+ * Returns the bit mask for a bit index from 0 to 31
+ */
+#define BIT(_BitIndex) (0x1u << (_BitIndex))
+
+/**
+ * Upper32Bits - return bits 32-63 of a number
+ * @N: the number we're accessing
+ *
+ * A basic shift-right of a 64- or 32-bit quantity.  Use this to suppress
+ * the "right shift count >= width of type" warning when that quantity is
+ * 32-bits.
+ */
+#define Upper32Bits(N) ((UINT32)(((N) >> 16) >> 16))
+
+/**
+ * Lower32Bits - return bits 0-31 of a number
+ * @N: the number we're accessing
+ */
+#define Lower32Bits(N) ((UINT32)(N))
+
+
+/*
+ * Stores a value for a given bit field in 32-bit '_Container'
+ */
+
+#define SET_BIT_FIELD32(_Container, _BitShift, _BitWidth, _Value) \
+  __SET_BIT_FIELD32(_Container,   \
+  __GEN_BIT_FIELD_MASK32(_BitShift, _BitWidth),   \
+  _BitShift,  \
+  _Value)
+
+#define __SET_BIT_FIELD32(_Container, _BitMask, _BitShift, _Value)  \
+  do {  \
+(_Container) &= ~(_BitMask);\
+if ((_Value) != 0) {\
+  ASSERT(((UINT32)(_Value) << (_BitShift)) <= (_BitMask));  \
+  (_Container) |=   \
+  ((UINT32)(_Value) << (_BitShift)) & (_BitMask);   \
+}   \
+  } while (0)
+
+/*
+ * Extracts the value for a given bit field in 32-bit _Container
+ */
+
+#define GET_BIT_FIELD32(_Container, _BitShift, _BitWidth) \
+  __GET_BIT_FIELD32(_Container,   \
+  __GEN_BIT_FIELD_MASK32(_BitShift, _BitWidth),   \
+  _BitShift)
+
+#define __GET_BIT_FIELD32(_Container, _BitMask, _BitShift)  \
+  (((UINT32)(_Container) & (_BitMask)) >> (_BitShift))
+
+#define __GEN_BIT_FIELD_MASK32(_BitShift, _BitWidth)\
+  ((_BitWidth) < 32 ?   \
+   (((UINT32)1 << (_BitWidth)) - 1) << (_BitShift) :\
+   ~(UINT32)0)
+
+/*
+ *Stores a value for a given bit field in 64-bit '_Container'
+ */
+#define SET_BIT_FIELD64(_Container, _BitShift, _BitWidth, _Value) \
+  __SET_BIT_FIELD64(_Container,   \
+  __GEN_BIT_FIELD_MASK64(_BitShift, _BitWidth),   \
+  _BitShift,  \
+  _Value)
+
+#define __SET_BIT_FIELD64(_Container, _BitMask, _BitShift, _Value)  \
+  do {  \
+(_Container) &= ~(_BitMask);   

[edk2] [PATCH 07/10] Platform/NXP : Add support for DS1307 RTC library

[Meenakshi Aggarwal]

Real time clock Apis on top of I2C Apis

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h  |  40 
 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c   | 226 +
 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf |  40 
 3 files changed, 306 insertions(+)
 create mode 100644 Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h
 create mode 100644 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c
 create mode 100644 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf

diff --git a/Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h 
b/Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h
new file mode 100644
index 000..952933f
--- /dev/null
+++ b/Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h
@@ -0,0 +1,40 @@
+/** Ds1307Rtc.h
+*
+*  Copyright 2017 NXP
+*
+*  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 __DS1307RTC_H__
+#define __DS1307RTC_H__
+
+#define Bin(Bcd) ((Bcd) & 0x0f) + ((Bcd) >> 4) * 10
+#define Bcd(Bin) (((Bin / 10) << 4) | (Bin % 10))
+
+/*
+ * RTC register addresses
+ */
+#define DS1307_SEC_REG_ADDR0x00
+#define DS1307_MIN_REG_ADDR0x01
+#define DS1307_HR_REG_ADDR 0x02
+#define DS1307_DAY_REG_ADDR0x03
+#define DS1307_DATE_REG_ADDR   0x04
+#define DS1307_MON_REG_ADDR0x05
+#define DS1307_YR_REG_ADDR 0x06
+#define DS1307_CTL_REG_ADDR0x07
+
+#define DS1307_SEC_BIT_CH  0x80  /* Clock Halt (in Register 0)   */
+
+#define DS1307_CTL_BIT_RS0 0x01  /* Rate select 0*/
+#define DS1307_CTL_BIT_RS1 0x02  /* Rate select 1*/
+#define DS1307_CTL_BIT_SQWE0x10  /* Square Wave Enable   */
+#define DS1307_CTL_BIT_OUT 0x80  /* Output Control   */
+
+#endif // __DS1307RTC_H__
diff --git a/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c 
b/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c
new file mode 100644
index 000..5f620a3
--- /dev/null
+++ b/Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c
@@ -0,0 +1,226 @@
+/** Ds1307RtcLib.c
+  Implement EFI RealTimeClock with runtime services via RTC Lib for DS1307 RTC.
+
+  Based on RTC implementation available in
+  EmbeddedPkg/Library/TemplateRealTimeClockLib/RealTimeClockLib.c
+
+  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
+  Copyright 2017 NXP
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "Ds1307Rtc.h"
+
+/**
+  Read RTC register.
+
+  @param  RtcRegAddr   Register offset of RTC to be read.
+
+  @retval  Register Value read
+
+**/
+
+UINT8
+RtcRead (
+  IN  UINT8  RtcRegAddr
+  )
+{
+  INT32 Status;
+  UINT8 Val;
+
+  Val = 0;
+  Status = I2cDataRead(PcdGet32(PcdRtcI2cBus), PcdGet32(PcdDs1307I2cAddress),
+   RtcRegAddr, 0x1, , sizeof(Val));
+  if (EFI_ERROR(Status))
+DEBUG((DEBUG_ERROR, "RTC read error at Addr:0x%x\n", RtcRegAddr));
+
+  return Val;
+}
+/**
+  Write RTC register.
+
+  @param  RtcRegAddr   Register offset of RTC to write.
+  @param  Val  Value to be written
+
+**/
+
+VOID
+RtcWrite(
+  IN  UINT8  RtcRegAddr,
+  IN  UINT8  Val
+  )
+{
+  EFI_STATUS Status;
+
+  Status = I2cDataWrite(PcdGet32(PcdRtcI2cBus), PcdGet32(PcdDs1307I2cAddress),
+RtcRegAddr, 0x1, , sizeof(Val));
+  if (EFI_ERROR(Status))
+DEBUG((DEBUG_ERROR, "RTC write error at Addr:0x%x\n", RtcRegAddr));
+}
+
+/**
+  Returns the current time and date information, and the time-keeping 
capabilities
+  of the hardware platform.
+
+  @param  Time  A pointer to storage to receive a snapshot of 
the current time.
+  @param  Capabilities  An optional pointer to a buffer to receive the 
real time clock
+device's capabilities.
+
+  @retval EFI_SUCCESS   The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER Time is NULL.
+  @retval EFI_DEVICE_ERROR  The time could not be retrieved due to 
hardware error.
+
+**/
+
+EFI_STATUS
+EFIAPI
+LibGetTime (
+  OUT  EFI_TIME*Time,
+  OUT  

[edk2] [PATCH 08/10] Platform/NXP: Add support for ArmPlatformLib

[Meenakshi Aggarwal]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 .../Library/PlatformLib/ArmPlatformLib.c   | 105 
 .../Library/PlatformLib/ArmPlatformLib.inf |  70 
 .../Library/PlatformLib/NxpQoriqLsHelper.S |  38 +
 .../Library/PlatformLib/NxpQoriqLsMem.c| 184 +
 4 files changed, 397 insertions(+)
 create mode 100644 
Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
 create mode 100644 
Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
 create mode 100644 
Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
 create mode 100644 
Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c

diff --git a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c 
b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
new file mode 100644
index 000..21deb41
--- /dev/null
+++ b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
@@ -0,0 +1,105 @@
+/** ArmPlatformLib.c
+*
+*  Contains board initialization functions.
+*
+*  Based on BeagleBoardPkg/Library/BeagleBoardLib/BeagleBoard.c
+*
+*  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
+*  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
+*  Copyright 2017 NXP
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+#include 
+
+extern VOID SocInit(VOID);
+
+/**
+  Return the current Boot Mode
+
+  This function returns the boot reason on the platform
+
+**/
+EFI_BOOT_MODE
+ArmPlatformGetBootMode (
+  VOID
+  )
+{
+  return BOOT_WITH_FULL_CONFIGURATION;
+}
+
+/**
+ Placeholder for Platform Initialization
+**/
+EFI_STATUS
+ArmPlatformInitialize (
+  IN  UINTN   MpId
+  )
+{
+ SocInit();
+
+ return EFI_SUCCESS;
+}
+
+ARM_CORE_INFO LS1043aMpCoreInfoCTA53x4[] = {
+  {
+// Cluster 0, Core 0
+0x0, 0x0,
+
+// MP Core MailBox Set/Get/Clear Addresses and Clear Value
+(EFI_PHYSICAL_ADDRESS)0,
+(EFI_PHYSICAL_ADDRESS)0,
+(EFI_PHYSICAL_ADDRESS)0,
+(UINT64)0x
+  },
+};
+
+EFI_STATUS
+PrePeiCoreGetMpCoreInfo (
+  OUT UINTN   *CoreCount,
+  OUT ARM_CORE_INFO   **ArmCoreTable
+  )
+{
+  *CoreCount= sizeof(LS1043aMpCoreInfoCTA53x4) / sizeof(ARM_CORE_INFO);
+  *ArmCoreTable = LS1043aMpCoreInfoCTA53x4;
+
+  return EFI_SUCCESS;
+}
+
+ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };
+
+EFI_PEI_PPI_DESCRIPTOR  gPlatformPpiTable[] = {
+  {
+EFI_PEI_PPI_DESCRIPTOR_PPI,
+,
+
+  }
+};
+
+VOID
+ArmPlatformGetPlatformPpiList (
+  OUT UINTN   *PpiListSize,
+  OUT EFI_PEI_PPI_DESCRIPTOR  **PpiList
+  )
+{
+  *PpiListSize = sizeof(gPlatformPpiTable);
+  *PpiList = gPlatformPpiTable;
+}
+
+
+UINTN
+ArmPlatformGetCorePosition (
+  IN UINTN MpId
+  )
+{
+  return 1;
+}
diff --git a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf 
b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
new file mode 100644
index 000..43956f7
--- /dev/null
+++ b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
@@ -0,0 +1,70 @@
+#  @file
+#
+#  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
+#  Copyright 2017 NXP
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = PlatformLib
+  FILE_GUID  = 736343a0-1d96-11e0--0002a5d5c51b
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmPlatformLib
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  edk2-platforms/Platform/NXP/NxpQoriqLs.dec
+
+[LibraryClasses]
+  ArmLib
+  SocLib
+
+[Sources.common]
+  NxpQoriqLsHelper.S| GCC
+  NxpQoriqLsMem.c
+  ArmPlatformLib.c
+
+[Ppis]
+  gArmMpCoreInfoPpiGuid
+
+[FeaturePcd]
+  gEmbeddedTokenSpaceGuid.PcdCacheEnable
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdArmPrimaryCore
+  gNxpQoriqLsTokenSpaceGuid.PcdCcsrBaseAddr
+  gNxpQoriqLsTokenSpaceGuid.PcdCcsrSize
+  

[edk2] [PATCH 04/10] Platform/NXP : Add support for Watchdog driver

[Meenakshi Aggarwal]

Installs watchdog timer arch protocol

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Drivers/WatchDog/WatchDog.c  | 386 ++
 Platform/NXP/Drivers/WatchDog/WatchDog.h  |  37 +++
 Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf |  47 
 3 files changed, 470 insertions(+)
 create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.c
 create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.h
 create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf

diff --git a/Platform/NXP/Drivers/WatchDog/WatchDog.c 
b/Platform/NXP/Drivers/WatchDog/WatchDog.c
new file mode 100644
index 000..f257a1d
--- /dev/null
+++ b/Platform/NXP/Drivers/WatchDog/WatchDog.c
@@ -0,0 +1,386 @@
+/** WatchDog.c
+*
+*  Based on Watchdog driver implemenation available in
+*  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
+*
+*  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+*  Copyright 2017 NXP
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "WatchDog.h"
+
+EFI_EVENT  EfiExitBootServicesEvent = (EFI_EVENT)NULL;
+
+UINT16
+EFIAPI
+WdogRead (
+  IN  UINTN Address
+  )
+{
+  if (FixedPcdGetBool(PcdWdogBigEndian)) {
+return BeMmioRead16(Address);
+  } else {
+return MmioRead16(Address);
+  }
+}
+
+UINT16
+EFIAPI
+WdogWrite (
+  IN  UINTN Address,
+  IN  UINT16Value
+  )
+{
+  if (FixedPcdGetBool(PcdWdogBigEndian)) {
+return BeMmioWrite16(Address, Value);
+  } else {
+return MmioWrite16(Address, Value);
+  }
+}
+
+STATIC
+VOID
+WdogPing (
+  VOID
+  )
+{
+  WdogWrite(PcdGet64(PcdWdog1BaseAddr) + WDOG_WSR_OFFSET,
+ WDOG_SERVICE_SEQ1);
+  WdogWrite(PcdGet64(PcdWdog1BaseAddr) + WDOG_WSR_OFFSET,
+ WDOG_SERVICE_SEQ2);
+}
+
+/**
+  Stop the Wdog watchdog timer from counting down.
+**/
+STATIC
+VOID
+WdogStop (
+  VOID
+  )
+{
+  //  Watchdog cannot be disabled by software once started.
+  // At best, we can keep pinging the watchdog
+  WdogPing();
+}
+
+/**
+  Starts the Wdog counting down by feeding Service register with
+  desired pattern.
+  The count down will start from the value stored in the Load register,
+  not from the value where it was previously stopped.
+**/
+STATIC
+VOID
+WdogStart (
+  VOID
+  )
+{
+  /* Reload the timeout value */
+  WdogPing();
+}
+
+/**
+On exiting boot services we must make sure the Wdog Watchdog Timer
+is stopped.
+**/
+VOID
+EFIAPI
+ExitBootServicesEvent (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  WdogStop();
+}
+
+/**
+  This function registers the handler NotifyFunction so it is called every time
+  the watchdog timer expires.  It also passes the amount of time since the last
+  handler call to the NotifyFunction.
+  If NotifyFunction is not NULL and a handler is not already registered,
+  then the new handler is registered and EFI_SUCCESS is returned.
+  If NotifyFunction is NULL, and a handler is already registered,
+  then that handler is unregistered.
+  If an attempt is made to register a handler when a handler is already 
registered,
+  then EFI_ALREADY_STARTED is returned.
+  If an attempt is made to unregister a handler when a handler is not 
registered,
+  then EFI_INVALID_PARAMETER is returned.
+
+  @param  This The EFI_TIMER_ARCH_PROTOCOL instance.
+  @param  NotifyFunction   The function to call when a timer interrupt fires. 
This
+   function executes at TPL_HIGH_LEVEL. The DXE Core 
will
+   register a handler for the timer interrupt, so it 
can know
+   how much time has passed. This information is used 
to
+   signal timer based events. NULL will unregister the 
handler.
+
+  @retval EFI_SUCCESS   The watchdog timer handler was registered.
+  @retval EFI_ALREADY_STARTED   NotifyFunction is not NULL, and a handler is 
already
+registered.
+  @retval EFI_INVALID_PARAMETER NotifyFunction is NULL, and a handler was not
+previously registered.
+
+**/
+EFI_STATUS
+EFIAPI
+WdogRegisterHandler (
+  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
+  )
+{
+  // ERROR: This function is not supported.
+  // The hardware watchdog will reset the board
+  return EFI_INVALID_PARAMETER;
+}
+
+/**
+
+  This function adjusts the period of 

[edk2] [PATCH 03/10] Platform/NXP: Add support for Big Endian Mmio APIs

[Meenakshi Aggarwal]

This library add supports for BE read/write and other
MMIO helper function.
In this data swapped after reading from MMIO and before
write using MMIO.
It can be used by any module with BE address space.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Include/Library/BeIoLib.h   | 332 +
 Platform/NXP/Library/BeIoLib/BeIoLib.c   | 400 +++
 Platform/NXP/Library/BeIoLib/BeIoLib.inf |  31 +++
 3 files changed, 763 insertions(+)
 create mode 100644 Platform/NXP/Include/Library/BeIoLib.h
 create mode 100644 Platform/NXP/Library/BeIoLib/BeIoLib.c
 create mode 100644 Platform/NXP/Library/BeIoLib/BeIoLib.inf

diff --git a/Platform/NXP/Include/Library/BeIoLib.h 
b/Platform/NXP/Include/Library/BeIoLib.h
new file mode 100644
index 000..209262d
--- /dev/null
+++ b/Platform/NXP/Include/Library/BeIoLib.h
@@ -0,0 +1,332 @@
+/** BeIoLib.h
+ *
+ *  Copyright 2017 NXP
+ *
+ *  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
+ *
+ *  HE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+ *
+ **/
+
+#ifndef __BE_IOLIB_H__
+#define __BE_IOLIB_H__
+
+#include 
+
+/**
+  MmioRead8 for Big-Endian modules.
+
+  @param  Address The MMIO register to read.
+
+  @return The value read.
+
+**/
+UINT8
+EFIAPI
+BeMmioRead8 (
+  IN  UINTN Address
+  );
+
+/**
+  MmioRead16 for Big-Endian modules.
+
+  @param  Address The MMIO register to read.
+
+  @return The value read.
+
+**/
+UINT16
+EFIAPI
+BeMmioRead16 (
+  IN  UINTN Address
+  );
+
+/**
+  MmioRead32 for Big-Endian modules.
+
+  @param  Address The MMIO register to read.
+
+  @return The value read.
+
+**/
+UINT32
+EFIAPI
+BeMmioRead32 (
+  IN  UINTN Address
+  );
+
+/**
+  MmioRead64 for Big-Endian modules.
+
+  @param  Address The MMIO register to read.
+
+  @return The value read.
+
+**/
+UINT64
+EFIAPI
+BeMmioRead64 (
+  IN  UINTN Address
+  );
+
+/**
+  MmioWrite8 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  Value   The value to write to the MMIO register.
+
+**/
+UINT8
+EFIAPI
+BeMmioWrite8 (
+  IN  UINTN Address,
+  IN  UINT8 Value
+  );
+
+/**
+  MmioWrite16 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  Value   The value to write to the MMIO register.
+
+**/
+UINT16
+EFIAPI
+BeMmioWrite16 (
+  IN  UINTN Address,
+  IN  UINT16Value
+  );
+
+/**
+  MmioWrite32 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  Value   The value to write to the MMIO register.
+
+**/
+UINT32
+EFIAPI
+BeMmioWrite32 (
+  IN  UINTN Address,
+  IN  UINT32Value
+  );
+
+/**
+  MmioWrite64 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  Value   The value to write to the MMIO register.
+
+**/
+UINT64
+EFIAPI
+BeMmioWrite64 (
+  IN  UINTN Address,
+  IN  UINT64Value
+  );
+
+/**
+  MmioAndThenOr8 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  AndData The value to AND with the read value from the MMIO register.
+  @param  OrData  The value to OR with the result of the AND operation.
+
+  @return The value written back to the MMIO register.
+
+**/
+UINT8
+EFIAPI
+BeMmioAndThenOr8 (
+  IN  UINTN Address,
+  IN  UINT8 AndData,
+  IN  UINT8 OrData
+  );
+
+/**
+  MmioAndThenOr16 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  AndData The value to AND with the read value from the MMIO register.
+  @param  OrData  The value to OR with the result of the AND operation.
+
+  @return The value written back to the MMIO register.
+
+**/
+UINT16
+EFIAPI
+BeMmioAndThenOr16 (
+  IN  UINTN Address,
+  IN  UINT16AndData,
+  IN  UINT16OrData
+  );
+
+/**
+  MmioAndThenOr32 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  AndData The value to AND with the read value from the MMIO register.
+  @param  OrData  The value to OR with the result of the AND operation.
+
+  @return The value written back to the MMIO register.
+
+**/
+UINT32
+EFIAPI
+BeMmioAndThenOr32 (
+  IN  UINTN Address,
+  IN  UINT32AndData,
+  IN  UINT32OrData
+  );
+
+/**
+  MmioAndThenOr64 for Big-Endian modules.
+
+  @param  Address The MMIO register to write.
+  @param  AndData The value to AND with the read value from the MMIO register.
+  @param  OrData  The value to OR with the result of the AND operation.
+
+  @return The value written back to the MMIO register.
+
+**/
+UINT64
+EFIAPI
+BeMmioAndThenOr64 (
+  IN  UINTN Address,
+  IN  UINT64AndData,
+  

[edk2] [PATCH 06/10] Platform/NXP: Add support for I2c operations library

[Meenakshi Aggarwal]

I2C bus initialization and I2c read/write APIs added.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Include/Library/I2c.h | 125 
 Platform/NXP/Library/I2cLib/I2cLib.c   | 549 +
 Platform/NXP/Library/I2cLib/I2cLib.h   | 109 +++
 Platform/NXP/Library/I2cLib/I2cLib.inf |  43 +++
 4 files changed, 826 insertions(+)
 create mode 100644 Platform/NXP/Include/Library/I2c.h
 create mode 100644 Platform/NXP/Library/I2cLib/I2cLib.c
 create mode 100644 Platform/NXP/Library/I2cLib/I2cLib.h
 create mode 100644 Platform/NXP/Library/I2cLib/I2cLib.inf

diff --git a/Platform/NXP/Include/Library/I2c.h 
b/Platform/NXP/Include/Library/I2c.h
new file mode 100644
index 000..c7195ab
--- /dev/null
+++ b/Platform/NXP/Include/Library/I2c.h
@@ -0,0 +1,125 @@
+/** I2c.h
+  Header defining the constant, base address amd function for I2C controller
+
+  Copyright 2017 NXP
+
+  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 __I2C_H___
+#define __I2C_H__
+
+#include 
+
+#define I2C1  0
+#define I2C2  1
+#define I2C3  2
+#define I2C4  3
+
+///
+/// Define the I2C flags
+///
+#define I2C_READ_FLAG 0x1
+#define I2C_WRITE_FLAG0x2
+
+/**
+  Function to initialize i2c bus
+
+  @param   I2cBus I2c Controller number
+  @param   Speed  Value to be set
+
+  @retval  EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+I2cBusInit (
+  IN   UINT32   I2cBus,
+  IN   UINT32Speed
+  );
+
+/**
+  Function to read data usin i2c
+
+  @param   I2cBus I2c Controller number
+  @param   Chip   Address of slave device from where data to be read
+  @param   Offset Offset of slave memory
+  @param   Alen   Address length of slave
+  @param   Buffer A pointer to the destination buffer for the data
+  @param   LenLength of data to be read
+
+  @retval  EFI_NOT_READYArbitration lost
+  @retval  EFI_TIMEOUT  Failed to initialize data transfer in predefined 
time
+  @retval  EFI_NOT_FOUNDACK was not recieved
+  @retval  EFI_SUCCESS  Read was successful
+
+**/
+EFI_STATUS
+I2cDataRead (
+  IN   UINT32  I2cBus,
+  IN   UINT8Chip,
+  IN   UINT32  Offset,
+  IN   UINT32  Alen,
+  OUT  UINT8  *Buffer,
+  IN   UINT32  Len
+  );
+
+/**
+  Function to write data using i2c bus
+
+  @param   I2cBus  I2c Controller number
+  @param   ChipAddress of slave device where data to be 
written
+  @param   Offset  Offset of slave memory
+  @param   AlenAddress length of slave
+  @param   Buffer  A pointer to the source buffer for the data
+  @param   Len Length of data to be write
+
+  @retval  EFI_NOT_READY   Arbitration lost
+  @retval  EFI_TIMEOUT Failed to initialize data transfer in 
predefined time
+  @retval  EFI_NOT_FOUND   ACK was not recieved
+  @retval  EFI_SUCCESS Read was successful
+
+**/
+EFI_STATUS
+I2cDataWrite (
+  IN   UINT32I2cBus,
+  IN   UINT8 Chip,
+  IN   UINT32Offset,
+  IN   INT32 Alen,
+  OUT  UINT8 *Buffer,
+  IN   INT32 Len
+  );
+
+/**
+  Function to reset I2c
+  @param   I2cBusI2c Controller number
+
+  @return  VOID
+**/
+VOID
+I2cReset (
+  UINT32 I2cBus
+  );
+
+/**
+  Function to Probe I2c slave device
+  @paramI2cBusI2c Controller number
+
+  @retval   EFI_INVALID_PARAMETER Invalid I2c Controller number
+  @retval   EFI_SUCCESS   I2c device probed successfully
+
+**/
+EFI_STATUS
+EFIAPI
+I2cProbeDevices (
+  IN   INT16I2cBus,
+  IN   UINT8Chip
+  );
+
+#endif
diff --git a/Platform/NXP/Library/I2cLib/I2cLib.c 
b/Platform/NXP/Library/I2cLib/I2cLib.c
new file mode 100644
index 000..574b899
--- /dev/null
+++ b/Platform/NXP/Library/I2cLib/I2cLib.c
@@ -0,0 +1,549 @@
+/** I2cLib.c
+  I2c Library containing functions for read, write, initialize, set speed etc
+
+  Copyright 2017 NXP
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "I2cLib.h"
+

[edk2] [PATCH 01/10] Platform/NXP: Library to provide helper functions.

[Meenakshi Aggarwal]

UtilsLib provide helper functions which will be needed
by NXP SoCs Library and Drivers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Include/Library/Utils.h| 137 
 Platform/NXP/Library/UtilsLib/Utils.c   |  97 ++
 Platform/NXP/Library/UtilsLib/Utils.inf |  30 +++
 3 files changed, 264 insertions(+)
 create mode 100644 Platform/NXP/Include/Library/Utils.h
 create mode 100644 Platform/NXP/Library/UtilsLib/Utils.c
 create mode 100644 Platform/NXP/Library/UtilsLib/Utils.inf

diff --git a/Platform/NXP/Include/Library/Utils.h 
b/Platform/NXP/Include/Library/Utils.h
new file mode 100644
index 000..8920e4d
--- /dev/null
+++ b/Platform/NXP/Include/Library/Utils.h
@@ -0,0 +1,137 @@
+/** Utils.h
+  Header defining the General Purpose Utilities
+
+  Copyright 2017 NXP
+
+  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 __UTILS_H__
+#define __UTILS_H__
+
+/*
+ * Divide positive or negative dividend by positive divisor and round
+ * to closest UINTNeger. Result is undefined for negative divisors and
+ * for negative dividends if the divisor variable type is unsigned.
+ */
+#define DIV_ROUND_CLOSEST(X, Divisor)(   \
+{\
+  typeof(X) __X = X; \
+  typeof(Divisor) __D = Divisor; \
+  (((typeof(X))-1) > 0 ||\
+((typeof(Divisor))-1) > 0 || (__X) > 0) ?\
+  (((__X) + ((__D) / 2)) / (__D)) :  \
+  (((__X) - ((__D) / 2)) / (__D));   \
+}\
+)
+
+/*
+ * HammingWeight32: returns the hamming weight (i.e. the number
+ * of bits set) of a 32-bit word
+ */
+STATIC
+inline
+UINTN
+HammingWeight32 (
+  IN  UINTN  W
+  )
+{
+  UINTN Res;
+
+  Res = (W & 0x) + ((W >> 1) & 0x);
+  Res = (Res & 0x) + ((Res >> 2) & 0x);
+  Res = (Res & 0x0F0F0F0F) + ((Res >> 4) & 0x0F0F0F0F);
+  Res = (Res & 0x00FF00FF) + ((Res >> 8) & 0x00FF00FF);
+
+  return (Res & 0x) + ((Res >> 16) & 0x);
+}
+
+STATIC
+inline
+UINTN
+CpuMaskNext (
+  IN  UINTN  Cpu,
+  IN  UINTN  Mask
+  )
+{
+  for (Cpu++; !((1 << Cpu) & Mask); Cpu++)
+;
+
+  return Cpu;
+}
+
+#define ForEachCpu(Iter, Cpu, NumCpus, Mask) \
+  for (Iter = 0, Cpu = CpuMaskNext(-1, Mask); \
+Iter < NumCpus; \
+Iter++, Cpu = CpuMaskNext(Cpu, Mask)) \
+
+/**
+  Find last (most-significant) bit set
+
+  @param   X :the word to search
+
+  Note Fls(0) = 0, Fls(1) = 1, Fls(0x8000) = 32.
+
+**/
+STATIC
+inline
+INT32
+GenericFls (
+  IN  INT32  X
+  )
+{
+  INT32 R = 32;
+
+  if (!X)
+return 0;
+
+  if (!(X & 0xu)) {
+X <<= 16;
+R -= 16;
+  }
+  if (!(X & 0xff00u)) {
+X <<= 8;
+R -= 8;
+  }
+  if (!(X & 0xf000u)) {
+X <<= 4;
+R -= 4;
+  }
+  if (!(X & 0xc000u)) {
+X <<= 2;
+R -= 2;
+  }
+  if (!(X & 0x8000u)) {
+X <<= 1;
+R -= 1;
+  }
+
+  return R;
+}
+
+/*
+ * PrINT32 Sizes As "Xxx KiB", "Xxx.Y KiB", "Xxx MiB", "Xxx.Y MiB",
+ * Xxx GiB, Xxx.Y GiB, Etc As Needed; Allow for Optional Trailing String
+ * (Like "\n")
+ */
+VOID
+PrintSize (
+  IN  UINT64 Size,
+  IN  CONST INT8 *S
+  );
+
+/* Function to convert a frequency to MHz */
+CHAR8 *StringToMHz (
+  CHAR8   *Buf,
+  UINT32  Size,
+  UINT64  Hz
+  );
+
+#endif
diff --git a/Platform/NXP/Library/UtilsLib/Utils.c 
b/Platform/NXP/Library/UtilsLib/Utils.c
new file mode 100644
index 000..4f5a15c
--- /dev/null
+++ b/Platform/NXP/Library/UtilsLib/Utils.c
@@ -0,0 +1,97 @@
+/** Utils.c
+
+  Copyright 2017 NXP
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+
+/* Function to convert a frequency to MHz */
+CHAR8 *
+StringToMHz (
+  IN  CHAR8   *Buf,
+  IN  UINT32  Size,
+  IN  UINT64  Hz
+  )
+{
+  UINT64 L;
+  UINT64 M;
+  UINT64 N;
+
+  N = DIV_ROUND_CLOSEST(Hz, 1000) / 1000L;
+  L = AsciiSPrint (Buf, Size, "%ld", N);
+
+  Hz -= N * 100L;
+  M = DIV_ROUND_CLOSEST(Hz, 1000L);
+
+  if (M != 0) {
+AsciiSPrint (Buf + L, Size, ".%03ld", M);
+  }
+
+  return (Buf);
+}

[edk2] [PATCH 00/10] edk2-platforms/Platform/NXP

[Meenakshi Aggarwal]

Hi,

Following patches will add support of NXP SoCs in edk2-platforms.

Our directory structure will be:

edk2-platforms/
|-- Platform
|   |-- NXP
|   |   |-- build.sh
|   |   |-- Drivers
|   |   |-- Env.cshrc
|   |   |-- Include
|   |   |   `-- Library
|   |   |   `-- Drivers
|   |   |-- Library
|   |   |-- LS1043aRdbPkg
|   |   |   |-- Drivers
|   |   |   |-- Include
|   |   |   |   `-- Library
|   |   |   |   `-- Drivers
|   |   |   |-- Library
|   |   |   |-- LS1043aRdbPkg.dec
|   |   |   |-- LS1043aRdbPkg.dsc
|   |   |   `-- LS1043aRdbPkg.fdf
|   |   |-- NxpQoriqLs.dec
|   |   |-- NxpQoriqLs.dsc
|   |   `-- Readme.md
`-- Silicon
|-- NXP
|-- Chassis
|   |-- Chassis2
`-- LS1043A
|-- Include
|-- LS1043A.dec
`-- LS1043A.dsc

In Silicon/NXP, we are keeping our SoC specific information and remaining code 
will be kept in Platform/NXP.

Platform/NXP/LS1043aRdbPkg will host .dsc and .fdf files to support compilation 
for LS1043A RDB board.

In next series of patches we will be adding support for LS2088 and LS1046 board.


Looking forward for your kind support in upstreaming our board in 
edk2-platforms.


Meenakshi Aggarwal (10):
  Platform/NXP: Library to provide helper functions.
  Platform/NXP: Add support for system reset library
  Platform/NXP: Add support for Big Endian Mmio APIs
  Platform/NXP : Add support for Watchdog driver
  Platform/NXP : Add support for DUART library
  Platform/NXP: Add support for I2c operations library
  Platform/NXP : Add support for DS1307 RTC library
  Platform/NXP: Add support for ArmPlatformLib
  SocLib : Add support for initialization of peripherals
  Compilation : Add the fdf, dsc and dec files.

 Platform/NXP/Drivers/WatchDog/WatchDog.c   | 386 +++
 Platform/NXP/Drivers/WatchDog/WatchDog.h   |  37 ++
 Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf  |  47 ++
 Platform/NXP/Env.cshrc |  75 +++
 Platform/NXP/Include/Bitops.h  | 179 +++
 Platform/NXP/Include/Library/BeIoLib.h | 332 +
 Platform/NXP/Include/Library/I2c.h | 125 +
 Platform/NXP/Include/Library/Utils.h   | 137 +
 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec   |  29 ++
 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc   |  74 +++
 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf   | 279 +++
 .../Library/PlatformLib/ArmPlatformLib.c   | 105 
 .../Library/PlatformLib/ArmPlatformLib.inf |  70 +++
 .../Library/PlatformLib/NxpQoriqLsHelper.S |  38 ++
 .../Library/PlatformLib/NxpQoriqLsMem.c| 184 +++
 Platform/NXP/Library/BeIoLib/BeIoLib.c | 400 +++
 Platform/NXP/Library/BeIoLib/BeIoLib.inf   |  31 ++
 Platform/NXP/Library/DUartPortLib/DUart.h  | 128 +
 Platform/NXP/Library/DUartPortLib/DUartPortLib.c   | 334 +
 Platform/NXP/Library/DUartPortLib/DUartPortLib.inf |  39 ++
 Platform/NXP/Library/Ds1307RtcLib/Ds1307Rtc.h  |  40 ++
 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.c   | 226 +
 Platform/NXP/Library/Ds1307RtcLib/Ds1307RtcLib.inf |  40 ++
 Platform/NXP/Library/I2cLib/I2cLib.c   | 549 +
 Platform/NXP/Library/I2cLib/I2cLib.h   | 109 
 Platform/NXP/Library/I2cLib/I2cLib.inf |  43 ++
 .../NXP/Library/ResetSystemLib/ResetSystemLib.c|  96 
 .../NXP/Library/ResetSystemLib/ResetSystemLib.inf  |  33 ++
 Platform/NXP/Library/UtilsLib/Utils.c  |  97 
 Platform/NXP/Library/UtilsLib/Utils.inf|  30 ++
 Platform/NXP/NxpQoriqLs.dec| 257 ++
 Platform/NXP/NxpQoriqLs.dsc| 453 +
 Platform/NXP/Readme.md |  14 +
 Platform/NXP/build.sh  | 100 
 Silicon/NXP/Chassis/Chassis.c  | 393 +++
 Silicon/NXP/Chassis/Chassis.h  | 123 +
 Silicon/NXP/Chassis/Chassis2/Chassis2.dec  |  19 +
 Silicon/NXP/Chassis/Chassis2/SerDes.h  |  82 +++
 Silicon/NXP/Chassis/Chassis2/Soc.c | 146 ++
 Silicon/NXP/Chassis/Chassis2/Soc.h | 376 ++
 Silicon/NXP/Chassis/LS1043aSocLib.inf  |  48 ++
 Silicon/NXP/Chassis/SerDes.c   | 253 ++
 Silicon/NXP/LS1043A/Include/SocSerDes.h|  55 +++
 Silicon/NXP/LS1043A/LS1043A.dec|  22 +
 Silicon/NXP/LS1043A/LS1043A.dsc|  82 +++
 45 files changed, 6715 insertions(+)
 create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.c
 create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDog.h
 create mode 100644 Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
 create mode 100644 Platform/NXP/Env.cshrc
 create mode 100644 Platform/NXP/Include/Bitops.h
 create mode 100644 

[edk2] [PATCH 05/10] Platform/NXP : Add support for DUART library

[Meenakshi Aggarwal]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Library/DUartPortLib/DUart.h  | 128 
 Platform/NXP/Library/DUartPortLib/DUartPortLib.c   | 334 +
 Platform/NXP/Library/DUartPortLib/DUartPortLib.inf |  39 +++
 3 files changed, 501 insertions(+)
 create mode 100644 Platform/NXP/Library/DUartPortLib/DUart.h
 create mode 100644 Platform/NXP/Library/DUartPortLib/DUartPortLib.c
 create mode 100644 Platform/NXP/Library/DUartPortLib/DUartPortLib.inf

diff --git a/Platform/NXP/Library/DUartPortLib/DUart.h 
b/Platform/NXP/Library/DUartPortLib/DUart.h
new file mode 100644
index 000..907790b
--- /dev/null
+++ b/Platform/NXP/Library/DUartPortLib/DUart.h
@@ -0,0 +1,128 @@
+/** DUart.h
+*  Header defining the DUART constants (Base addresses, sizes, flags)
+*
+*  Based on Serial I/O Port library headers available in PL011Uart.h
+*
+*  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
+*  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
+*  Copyright 2017 NXP
+*
+*  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 __DUART_H__
+#define __DUART_H__
+
+// FIFO Control Register
+#define DUART_FCR_FIFO_EN  0x01 /* Fifo enable */
+#define DUART_FCR_CLEAR_RCVR   0x02 /* Clear the RCVR FIFO */
+#define DUART_FCR_CLEAR_XMIT   0x04 /* Clear the XMIT FIFO */
+#define DUART_FCR_DMA_SELECT   0x08 /* For DMA applications */
+#define DUART_FCR_TRIGGER_MASK 0xC0 /* Mask for the FIFO trigger range */
+#define DUART_FCR_TRIGGER_10x00 /* Mask for trigger set at 1 */
+#define DUART_FCR_TRIGGER_40x40 /* Mask for trigger set at 4 */
+#define DUART_FCR_TRIGGER_80x80 /* Mask for trigger set at 8 */
+#define DUART_FCR_TRIGGER_14   0xC0 /* Mask for trigger set at 14 */
+#define DUART_FCR_RXSR 0x02 /* Receiver soft reset */
+#define DUART_FCR_TXSR 0x04 /* Transmitter soft reset */
+
+// Modem Control Register
+#define DUART_MCR_DTR  0x01 /* Reserved  */
+#define DUART_MCR_RTS  0x02 /* RTS   */
+#define DUART_MCR_OUT1 0x04 /* Reserved */
+#define DUART_MCR_OUT2 0x08 /* Reserved */
+#define DUART_MCR_LOOP 0x10 /* Enable loopback test mode */
+#define DUART_MCR_AFE  0x20 /* AFE (Auto Flow Control) */
+#define DUART_MCR_DMA_EN   0x04
+#define DUART_MCR_TX_DFR   0x08
+
+// Line Control Register
+/*
+* Note: if the word length is 5 bits (DUART_LCR_WLEN5), then setting
+* DUART_LCR_STOP will select 1.5 stop bits, not 2 stop bits.
+*/
+#define DUART_LCR_WLS_MSK  0x03 /* character length select mask */
+#define DUART_LCR_WLS_50x00 /* 5 bit character length */
+#define DUART_LCR_WLS_60x01 /* 6 bit character length */
+#define DUART_LCR_WLS_70x02 /* 7 bit character length */
+#define DUART_LCR_WLS_80x03 /* 8 bit character length */
+#define DUART_LCR_STB  0x04 /* # stop Bits, off=1, on=1.5 or 2) */
+#define DUART_LCR_PEN  0x08 /* Parity eneble */
+#define DUART_LCR_EPS  0x10 /* Even Parity Select */
+#define DUART_LCR_STKP 0x20 /* Stick Parity */
+#define DUART_LCR_SBRK 0x40 /* Set Break */
+#define DUART_LCR_BKSE 0x80 /* Bank select enable */
+#define DUART_LCR_DLAB 0x80 /* Divisor latch access bit */
+
+// Line Status Register
+#define DUART_LSR_DR   0x01 /* Data ready */
+#define DUART_LSR_OE   0x02 /* Overrun */
+#define DUART_LSR_PE   0x04 /* Parity error */
+#define DUART_LSR_FE   0x08 /* Framing error */
+#define DUART_LSR_BI   0x10 /* Break */
+#define DUART_LSR_THRE 0x20 /* Xmit holding register empty */
+#define DUART_LSR_TEMT 0x40 /* Xmitter empty */
+#define DUART_LSR_ERR  0x80 /* Error */
+
+// Modem Status Register
+#define DUART_MSR_DCTS 0x01 /* Delta CTS */
+#define DUART_MSR_DDSR 0x02 /* Reserved */
+#define DUART_MSR_TERI 0x04 /* Reserved */
+#define DUART_MSR_DDCD 0x08 /* Reserved */
+#define DUART_MSR_CTS  0x10 /* Clear to Send */
+#define DUART_MSR_DSR  0x20 /* Reserved */
+#define DUART_MSR_RI   0x40 /* Reserved */
+#define DUART_MSR_DCD  0x80 /* Reserved */
+
+// Interrupt Identification Register
+#define DUART_IIR_NO_INT   0x01 /* No interrupts pending */
+#define DUART_IIR_ID   

[edk2] [PATCH 02/10] Platform/NXP: Add support for system reset library

[Meenakshi Aggarwal]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 .../NXP/Library/ResetSystemLib/ResetSystemLib.c| 96 ++
 .../NXP/Library/ResetSystemLib/ResetSystemLib.inf  | 33 
 2 files changed, 129 insertions(+)
 create mode 100644 Platform/NXP/Library/ResetSystemLib/ResetSystemLib.c
 create mode 100644 Platform/NXP/Library/ResetSystemLib/ResetSystemLib.inf

diff --git a/Platform/NXP/Library/ResetSystemLib/ResetSystemLib.c 
b/Platform/NXP/Library/ResetSystemLib/ResetSystemLib.c
new file mode 100644
index 000..897324a
--- /dev/null
+++ b/Platform/NXP/Library/ResetSystemLib/ResetSystemLib.c
@@ -0,0 +1,96 @@
+/** ResetSystemLib.c
+  Do a generic Cold Reset
+
+  Based on Reset system library implementation in
+  BeagleBoardPkg/Library/ResetSystemLib/ResetSystemLib.c
+
+  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
+  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
+  Copyright 2017 NXP
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Resets the entire platform.
+
+  @param  ResetType : The type of reset to perform.
+  @param  ResetStatus   : The status code for the reset.
+  @param  DataSize  : The size, in bytes, of WatchdogData.
+  @param  ResetData : For a ResetType of EfiResetCold, EfiResetWarm, or
+  EfiResetShutdown the data buffer starts with a 
Null-terminated
+  Unicode string, optionally followed by 
additional binary data.
+**/
+EFI_STATUS
+EFIAPI
+LibResetSystem (
+  IN EFI_RESET_TYPE   ResetType,
+  IN EFI_STATUS   ResetStatus,
+  IN UINTNDataSize,
+  IN CHAR16   *ResetData OPTIONAL
+  )
+{
+  ARM_SMC_ARGS ArmSmcArgs;
+
+  switch (ResetType) {
+  case EfiResetPlatformSpecific:
+  case EfiResetWarm:
+// Map a warm reset into a cold reset
+  case EfiResetCold:
+// Send a PSCI 0.2 SYSTEM_RESET command
+ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_RESET;
+break;
+  case EfiResetShutdown:
+// Send a PSCI 0.2 SYSTEM_OFF command
+ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF;
+break;
+  default:
+ASSERT (FALSE);
+return EFI_UNSUPPORTED;
+  }
+
+  ArmCallSmc ();
+
+  // We should never be here
+  DEBUG ((DEBUG_VERBOSE, "%a: PSCI failed in performing %d\n",
+__FUNCTION__, ArmSmcArgs.Arg0));
+
+  CpuDeadLoop ();
+  return EFI_UNSUPPORTED;
+}
+
+/**
+  Initialize any infrastructure required for LibResetSystem () to function.
+
+  @param  ImageHandle  :  The firmware allocated handle for the EFI image.
+  @param  SystemTable  :  A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS  :  The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+LibInitializeResetSystem (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  return EFI_SUCCESS;
+}
diff --git a/Platform/NXP/Library/ResetSystemLib/ResetSystemLib.inf 
b/Platform/NXP/Library/ResetSystemLib/ResetSystemLib.inf
new file mode 100644
index 000..c57fff8
--- /dev/null
+++ b/Platform/NXP/Library/ResetSystemLib/ResetSystemLib.inf
@@ -0,0 +1,33 @@
+#  @ResetSystemLib.inf
+#  Reset System lib to make it easy to port new platforms
+#
+#  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
+#  Copyright 2017 NXP
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution. The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = ResetSystemLib
+  FILE_GUID  = 781371a2-3fdd-41d4-96a1-7b34cbc9e895
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = EfiResetSystemLib
+
+[Sources.common]
+  ResetSystemLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmSmcLib
+  BaseLib
-- 
1.9.1

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

Re: [edk2] [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctly

[Zeng, Star]

Some minor comments.

1. How about update structure field name 'OriginalAttribute' to be 
'OriginalAttributes'?
2. The comments below in Stop() need to be updated accordingly.
  //
  // Get supported PCI attributes
  //
3. Add the bugzilla link to the commit log.


With that fixed, Reviewed-by: Star Zeng 


Thanks,
Star
-Original Message-
From: Ni, Ruiyu 
Sent: Friday, November 3, 2017 4:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Laszlo Ersek 
Subject: [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctly

The original code enables some BITs in PCI attributes in Start(), but wrongly 
to disable these BITs in Stop().

The correct behavior is to save the original PCI attributes before enables some 
BITs in Start(), and restore to original value in Stop().

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Star Zeng 
Cc: Laszlo Ersek 
---
 PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c | 44 + 
 PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h |  3 ++-
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c 
b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
index 32381b112d..60d2fb5a5b 100644
--- a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
+++ b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
@@ -172,6 +172,7 @@ PcatIsaAcpiDriverBindingStart (
   EFI_PCI_IO_PROTOCOL  *PciIo;
   PCAT_ISA_ACPI_DEV*PcatIsaAcpiDev;
   UINT64   Supports;
+  UINT64   OriginalAttributes;
   BOOLEAN  Enabled;
 
   Enabled = FALSE;
@@ -210,9 +211,18 @@ PcatIsaAcpiDriverBindingStart (
   if (Supports == 0 || Supports == (EFI_PCI_IO_ATTRIBUTE_ISA_IO | 
EFI_PCI_IO_ATTRIBUTE_ISA_IO_16)) {
 Status = EFI_UNSUPPORTED;
 goto Done;
-  }  
+  }
+
+  Status = PciIo->Attributes (
+PciIo,
+EfiPciIoAttributeOperationGet,
+0,
+
+);
+  if (EFI_ERROR (Status)) {
+goto Done;
+  }
 
-  Enabled = TRUE;
   Status = PciIo->Attributes (
 PciIo, 
 EfiPciIoAttributeOperationEnable, @@ -222,7 +232,8 @@ 
PcatIsaAcpiDriverBindingStart (
   if (EFI_ERROR (Status)) {
 goto Done;
   }
-  
+
+  Enabled = TRUE;
   //
   // Allocate memory for the PCAT ISA ACPI Device structure
   //
@@ -239,9 +250,10 @@ PcatIsaAcpiDriverBindingStart (
   //
   // Initialize the PCAT ISA ACPI Device structure
   //
-  PcatIsaAcpiDev->Signature = PCAT_ISA_ACPI_DEV_SIGNATURE;
-  PcatIsaAcpiDev->Handle= Controller;
-  PcatIsaAcpiDev->PciIo = PciIo;
+  PcatIsaAcpiDev->Signature = PCAT_ISA_ACPI_DEV_SIGNATURE;
+  PcatIsaAcpiDev->Handle= Controller;
+  PcatIsaAcpiDev->PciIo = PciIo;
+  PcatIsaAcpiDev->OriginalAttribute = OriginalAttributes;
 
   //
   // Initialize PcatIsaAcpiDeviceList
@@ -274,8 +286,8 @@ Done:
 if (PciIo != NULL && Enabled) {
   PciIo->Attributes (
PciIo, 
-   EfiPciIoAttributeOperationDisable, 
-   EFI_PCI_DEVICE_ENABLE | Supports | 
EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO,
+   EfiPciIoAttributeOperationSet, 
+   OriginalAttributes,
NULL 
);
 }
@@ -321,7 +333,6 @@ PcatIsaAcpiDriverBindingStop (
   EFI_STATUS Status;
   EFI_ISA_ACPI_PROTOCOL  *IsaAcpi;
   PCAT_ISA_ACPI_DEV  *PcatIsaAcpiDev;
-  UINT64 Supports;
   
   //
   // Get the ISA ACPI Protocol Interface @@ -348,23 +359,14 @@ 
PcatIsaAcpiDriverBindingStop (
   //
   Status = PcatIsaAcpiDev->PciIo->Attributes (
 PcatIsaAcpiDev->PciIo,
-EfiPciIoAttributeOperationSupported,
-0,
-
+EfiPciIoAttributeOperationSet,
+PcatIsaAcpiDev->OriginalAttribute,
+0
 );
   if (EFI_ERROR (Status)) {
 return Status;
   }
 
-  Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_ISA_IO | 
EFI_PCI_IO_ATTRIBUTE_ISA_IO_16);
-
-  PcatIsaAcpiDev->PciIo->Attributes (
-   PcatIsaAcpiDev->PciIo, 
-   EfiPciIoAttributeOperationDisable, 
-   EFI_PCI_DEVICE_ENABLE | Supports | 
EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO,
-   NULL 
-   );
- 
   //
   // Uninstall protocol interface: EFI_ISA_ACPI_PROTOCOL
   //
diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h 
b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h
index 0671127644..3ad3a3f313 100644
--- a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h
+++ b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h
@@ -1,7 +1,7 @@
 /** @file
   

Re: [edk2] [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctly

[Shi, Steven]

This patch can fix the Bug 405 issue 
(https://bugzilla.tianocore.org/show_bug.cgi?id=405). Thank you!



Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, November 7, 2017 1:36 PM
> To: Laszlo Ersek ; Shi, Steven 
> Cc: Zeng, Star ; edk2-devel@lists.01.org
> Subject: RE: [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes
> correctly
> 
> Laszlo,
> Sure I will add the Bugzilla url in the commit message.
> 
> Steven,
> Could you please check whether this patch can fix your "reconnect -r" hang?
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Tuesday, November 7, 2017 7:23 AM
> > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Shi, Steven 
> > Subject: Re: [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes
> > correctly
> >
> > Hi Ray,
> >
> > On 11/03/17 09:28, Ruiyu Ni wrote:
> > > The original code enables some BITs in PCI attributes in Start(), but
> > > wrongly to disable these BITs in Stop().
> > >
> > > The correct behavior is to save the original PCI attributes before
> > > enables some BITs in Start(), and restore to original value in Stop().
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ruiyu Ni 
> > > Cc: Star Zeng 
> > > Cc: Laszlo Ersek 
> > > ---
> > >  PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c | 44
> > > +
> > > PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h |  3 ++-
> > >  2 files changed, 25 insertions(+), 22 deletions(-)
> >
> > Is this for ?
> >
> > If so, can you please add the reference to the commit message?
> >
> > Also, I think we should ask Steven to test the patch. (CC'd.)
> >
> > I'll try to comment more later.
> >
> > Thanks
> > Laszlo
> >
> >
> > >
> > > diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
> > > b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
> > > index 32381b112d..60d2fb5a5b 100644
> > > --- a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
> > > +++ b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c
> > > @@ -172,6 +172,7 @@ PcatIsaAcpiDriverBindingStart (
> > >EFI_PCI_IO_PROTOCOL  *PciIo;
> > >PCAT_ISA_ACPI_DEV*PcatIsaAcpiDev;
> > >UINT64   Supports;
> > > +  UINT64   OriginalAttributes;
> > >BOOLEAN  Enabled;
> > >
> > >Enabled = FALSE;
> > > @@ -210,9 +211,18 @@ PcatIsaAcpiDriverBindingStart (
> > >if (Supports == 0 || Supports == (EFI_PCI_IO_ATTRIBUTE_ISA_IO |
> > EFI_PCI_IO_ATTRIBUTE_ISA_IO_16)) {
> > >  Status = EFI_UNSUPPORTED;
> > >  goto Done;
> > > -  }
> > > +  }
> > > +
> > > +  Status = PciIo->Attributes (
> > > +PciIo,
> > > +EfiPciIoAttributeOperationGet,
> > > +0,
> > > +
> > > +);
> > > +  if (EFI_ERROR (Status)) {
> > > +goto Done;
> > > +  }
> > >
> > > -  Enabled = TRUE;
> > >Status = PciIo->Attributes (
> > >  PciIo,
> > >  EfiPciIoAttributeOperationEnable, @@ -222,7
> > > +232,8 @@ PcatIsaAcpiDriverBindingStart (
> > >if (EFI_ERROR (Status)) {
> > >  goto Done;
> > >}
> > > -
> > > +
> > > +  Enabled = TRUE;
> > >//
> > >// Allocate memory for the PCAT ISA ACPI Device structure
> > >//
> > > @@ -239,9 +250,10 @@ PcatIsaAcpiDriverBindingStart (
> > >//
> > >// Initialize the PCAT ISA ACPI Device structure
> > >//
> > > -  PcatIsaAcpiDev->Signature = PCAT_ISA_ACPI_DEV_SIGNATURE;
> > > -  PcatIsaAcpiDev->Handle= Controller;
> > > -  PcatIsaAcpiDev->PciIo = PciIo;
> > > +  PcatIsaAcpiDev->Signature = PCAT_ISA_ACPI_DEV_SIGNATURE;
> > > +  PcatIsaAcpiDev->Handle= Controller;
> > > +  PcatIsaAcpiDev->PciIo = PciIo;
> > > +  PcatIsaAcpiDev->OriginalAttribute = OriginalAttributes;
> > >
> > >//
> > >// Initialize PcatIsaAcpiDeviceList @@ -274,8 +286,8 @@ Done:
> > >  if (PciIo != NULL && Enabled) {
> > >PciIo->Attributes (
> > > PciIo,
> > > -   EfiPciIoAttributeOperationDisable,
> > > -   EFI_PCI_DEVICE_ENABLE | Supports |
> > EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO,
> > > +   EfiPciIoAttributeOperationSet,
> > > +   OriginalAttributes,
> > > NULL
> > > );
> > >  }
> > > @@ -321,7 +333,6 @@ PcatIsaAcpiDriverBindingStop (
> > >EFI_STATUS Status;
> > >EFI_ISA_ACPI_PROTOCOL  *IsaAcpi;
> > >PCAT_ISA_ACPI_DEV  *PcatIsaAcpiDev;
> > > -  UINT64 Supports;
> > >
> > >//
> > >// Get the ISA ACPI Protocol Interface @@ -348,23 +359,14 @@
> > > 

Re: [edk2] [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT C4255 warning

[Yao, Jiewen]

Got it. That is better. Agree.

I think we can check in the real C code update at first.

Thank you
Yao Jiewen

> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, November 7, 2017 3:58 PM
> To: Yao, Jiewen ; Song, BinX ;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT C4255 warning
> 
> Jiewen:
>   We plan to enable this option in new VS tool chain VS2017. So, we can keep
> DSC without the change, and enable this option after VS2017 tool chain is 
> added.
> 
> Thanks
> Liming
> >-Original Message-
> >From: Yao, Jiewen
> >Sent: Tuesday, November 07, 2017 2:00 PM
> >To: Yao, Jiewen ; Song, BinX ;
> >edk2-devel@lists.01.org
> >Cc: Gao, Liming 
> >Subject: RE: [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT C4255 warning
> >
> >Sorry, let me clarify, if we believe C4255 can help catch more problem, and 
> >we
> >fixed all problem, why not enable it in BaseTool directly.
> >
> >
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >Yao,
> >> Jiewen
> >> Sent: Tuesday, November 7, 2017 1:44 PM
> >> To: Song, BinX ; edk2-devel@lists.01.org
> >> Cc: Gao, Liming 
> >> Subject: Re: [edk2] [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT
> >C4255
> >> warning
> >>
> >> Yes, I suggest we fix it.
> >>
> >> > -Original Message-
> >> > From: Song, BinX
> >> > Sent: Tuesday, November 7, 2017 1:43 PM
> >> > To: Yao, Jiewen ; edk2-devel@lists.01.org
> >> > Cc: Gao, Liming 
> >> > Subject: RE: [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT C4255
> >warning
> >> >
> >> > Hi Jiewen,
> >> >
> >> > Do you mean we fix the problem after enable MSFT C4255 warning?
> >> > If yes, I have fix them in related patch, such as MdeModulePkg.
> >> >
> >> > Best Regards,
> >> > Bell Song
> >> >
> >> >
> >> > > -Original Message-
> >> > > From: Yao, Jiewen
> >> > > Sent: Tuesday, November 7, 2017 1:39 PM
> >> > > To: Song, BinX ; edk2-devel@lists.01.org
> >> > > Cc: Gao, Liming 
> >> > > Subject: RE: [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT C4255
> >> > > warning
> >> > >
> >> > > Hi
> >> > > I suggest we fix the problem.
> >> > >
> >> > > > -Original Message-
> >> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> >Of
> >> > > Song,
> >> > > > BinX
> >> > > > Sent: Tuesday, November 7, 2017 1:35 PM
> >> > > > To: edk2-devel@lists.01.org
> >> > > > Cc: Gao, Liming 
> >> > > > Subject: [edk2] [PATCH 05/14] IntelFsp2WrapperPkg: Enable MSFT
> >C4255
> >> > > warning
> >> > > >
> >> > > > Enable MSFT C4255 warning
> >> > > >
> >> > > > From MSDN:
> >> > > > Compiler Warning (level 4) C4255
> >> > > > function' : no function prototype given: converting '()' to '(void)'
> >> > > > The compiler did not find an explicit list of arguments to a 
> >> > > > function.
> >> > > > This warning is for the C compiler only.
> >> > > >
> >> > > > Cc: Liming Gao 
> >> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> >> > > > Signed-off-by: Bell Song 
> >> > > > ---
> >> > > >  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 +
> >> > > >  1 file changed, 1 insertion(+)
> >> > > >
> >> > > > diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
> >> > > > b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
> >> > > > index 6496dad..4b4d5b2 100644
> >> > > > --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
> >> > > > +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
> >> > > > @@ -95,3 +95,4 @@
> >> > > >
> >> > > >  [BuildOptions]
> >> > > >*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> >> > > > +  MSFT:*_*_*_CC_FLAGS = /we4255
> >> > > > --
> >> > > > 2.10.2.windows.1
> >> > > >
> >> > > > ___
> >> > > > edk2-devel mailing list
> >> > > > edk2-devel@lists.01.org
> >> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel