Re: [edk2] [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine ConfigurePixelBitMaskFormat

[Tian, Feng]

Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, January 23, 2017 2:12 PM
To: edk2-devel@lists.01.org
Cc: Tian, Feng 
Subject: [PATCH 1/3] MdeModulePkg/FrameBufferBltLib: Refine 
ConfigurePixelBitMaskFormat

https://bugzilla.tianocore.org/show_bug.cgi?id=339
The patch refines ConfigurePixelBitMaskFormat() to prepare the enhancement in 
next commit: Enhance this library to use dynamic allocated line buffer to 
reduce memory usage of frame buffer configure.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 71 +-
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c 
b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
index c9bb206..5f6eddc 100644
--- a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
+++ b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
@@ -1,7 +1,7 @@
 /** @file
   FrameBufferBltLib - Library to perform blt operations on a frame buffer.
 
-  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2007 - 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
   which accompanies this distribution.  The full text of the license may be 
found at @@ -30,8 +30,8 @@ struct FRAME_BUFFER_CONFIGURE {
   UINT8   *FrameBuffer;
   EFI_GRAPHICS_PIXEL_FORMAT   PixelFormat;
   EFI_PIXEL_BITMASK   PixelMasks;
-  INTNPixelShl[4]; // R-G-B-Rsvd
-  INTNPixelShr[4]; // R-G-B-Rsvd
+  INT8PixelShl[4]; // R-G-B-Rsvd
+  INT8PixelShr[4]; // R-G-B-Rsvd
 };
 
 CONST EFI_PIXEL_BITMASK mRgbPixelMasks = { @@ -45,43 +45,47 @@ CONST 
EFI_PIXEL_BITMASK mBgrPixelMasks = {
 /**
   Initialize the bit mask in frame buffer configure.
 
-  @param Configure  The frame buffer configure.
-  @param BitMaskThe bit mask of pixel.
+  @param BitMask   The bit mask of pixel.
+  @param BytesPerPixel Size in bytes of pixel.
+  @param PixelShl  Left shift array.
+  @param PixelShr  Right shift array.
 **/
 VOID
-ConfigurePixelBitMaskFormat (
-  IN FRAME_BUFFER_CONFIGURE *Configure,
-  IN CONST EFI_PIXEL_BITMASK*BitMask
+FrameBufferBltLibConfigurePixelFormat (
+  IN CONST EFI_PIXEL_BITMASK*BitMask,
+  OUT UINTN *BytesPerPixel,
+  OUT INT8  *PixelShl,
+  OUT INT8  *PixelShr
   )
 {
-  UINTN   Loop;
+  UINT8   Index;
   UINT32  *Masks;
   UINT32  MergedMasks;
 
+  ASSERT (BytesPerPixel != NULL);
+
   MergedMasks = 0;
   Masks = (UINT32*) BitMask;
-  for (Loop = 0; Loop < 3; Loop++) {
-ASSERT ((Loop == 3) || (Masks[Loop] != 0));
-ASSERT ((MergedMasks & Masks[Loop]) == 0);
-Configure->PixelShl[Loop] = HighBitSet32 (Masks[Loop]) - 23 + (Loop * 8);
-if (Configure->PixelShl[Loop] < 0) {
-  Configure->PixelShr[Loop] = -Configure->PixelShl[Loop];
-  Configure->PixelShl[Loop] = 0;
+  for (Index = 0; Index < 3; Index++) {
+ASSERT ((MergedMasks & Masks[Index]) == 0);
+
+PixelShl[Index] = (INT8) HighBitSet32 (Masks[Index]) - 23 + (Index * 8);
+if (PixelShl[Index] < 0) {
+  PixelShr[Index] = -PixelShl[Index];
+  PixelShl[Index] = 0;
 } else {
-  Configure->PixelShr[Loop] = 0;
+  PixelShr[Index] = 0;
 }
-MergedMasks = (UINT32) (MergedMasks | Masks[Loop]);
-DEBUG ((EFI_D_VERBOSE, "%d: shl:%d shr:%d mask:%x\n", Loop,
-Configure->PixelShl[Loop], Configure->PixelShr[Loop], 
Masks[Loop]));
+DEBUG ((DEBUG_INFO, "%d: shl:%d shr:%d mask:%x\n", Index,
+PixelShl[Index], PixelShr[Index], Masks[Index]));
+
+MergedMasks = (UINT32) (MergedMasks | Masks[Index]);
   }
   MergedMasks = (UINT32) (MergedMasks | Masks[3]);
 
   ASSERT (MergedMasks != 0);
-  Configure->BytesPerPixel = (UINTN) ((HighBitSet32 (MergedMasks) + 7) / 8);
-
-  DEBUG ((EFI_D_VERBOSE, "Bytes per pixel: %d\n", Configure->BytesPerPixel));
-
-  CopyMem (>PixelMasks, BitMask, sizeof (*BitMask));
+  *BytesPerPixel = (UINTN) ((HighBitSet32 (MergedMasks) + 7) / 8);  
+ DEBUG ((DEBUG_INFO, "Bytes per pixel: %d\n", *BytesPerPixel));
 }
 
 /**
@@ -110,6 +114,11 @@ FrameBufferBltConfigure (
   IN OUT UINTN *ConfigureSize
   )
 {
+  CONST EFI_PIXEL_BITMASK  *BitMask;
+  UINTNBytesPerPixel;
+  INT8 PixelShl[4];
+  INT8 PixelShr[4];
+
   if (ConfigureSize == NULL) {
 return RETURN_INVALID_PARAMETER;
   }
@@ 

Re: [edk2] [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line buffer

[Tian, Feng]

Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, January 23, 2017 2:12 PM
To: edk2-devel@lists.01.org
Cc: Tian, Feng 
Subject: [PATCH 2/3] MdeModulePkg/FrameBufferBltLib: Use dynamic allocated line 
buffer

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

The patch uses dynamic allocated line buffer to reduce memory usage of frame 
buffer configure. (Original implementation uses 0x4000 bytes for line buffer.)

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 24 +++---
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c 
b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
index 5f6eddc..011d9c5 100644
--- a/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
+++ b/MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
@@ -26,12 +26,12 @@ struct FRAME_BUFFER_CONFIGURE {
   UINTN   BytesPerPixel;
   UINTN   WidthInPixels;
   UINTN   Height;
-  UINT8   LineBuffer[SIZE_4KB * sizeof 
(EFI_GRAPHICS_OUTPUT_BLT_PIXEL)];
   UINT8   *FrameBuffer;
   EFI_GRAPHICS_PIXEL_FORMAT   PixelFormat;
   EFI_PIXEL_BITMASK   PixelMasks;
   INT8PixelShl[4]; // R-G-B-Rsvd
   INT8PixelShr[4]; // R-G-B-Rsvd
+  UINT8   LineBuffer[0];
 };
 
 CONST EFI_PIXEL_BITMASK mRgbPixelMasks = { @@ -123,15 +123,6 @@ 
FrameBufferBltConfigure (
 return RETURN_INVALID_PARAMETER;
   }
 
-  if (*ConfigureSize < sizeof (FRAME_BUFFER_CONFIGURE)) {
-*ConfigureSize = sizeof (FRAME_BUFFER_CONFIGURE);
-return RETURN_BUFFER_TOO_SMALL;
-  }
-
-  if (Configure == NULL) {
-return RETURN_INVALID_PARAMETER;
-  }
-
   switch (FrameBufferInfo->PixelFormat) {
   case PixelRedGreenBlueReserved8BitPerColor:
 BitMask = 
@@ -156,6 +147,17 @@ FrameBufferBltConfigure (
 
   FrameBufferBltLibConfigurePixelFormat (BitMask, , PixelShl, 
PixelShr);
 
+  if (*ConfigureSize < sizeof (FRAME_BUFFER_CONFIGURE)
+ + FrameBufferInfo->HorizontalResolution * BytesPerPixel) {
+*ConfigureSize = sizeof (FRAME_BUFFER_CONFIGURE)
+   + FrameBufferInfo->HorizontalResolution * BytesPerPixel;
+return RETURN_BUFFER_TOO_SMALL;
+  }
+
+  if (Configure == NULL) {
+return RETURN_INVALID_PARAMETER;
+  }
+
   CopyMem (>PixelMasks, BitMask,  sizeof (*BitMask));
   CopyMem (Configure->PixelShl,PixelShl, sizeof (PixelShl));
   CopyMem (Configure->PixelShr,PixelShr, sizeof (PixelShr));
@@ -166,8 +168,6 @@ FrameBufferBltConfigure (
   Configure->Height= (UINTN) FrameBufferInfo->VerticalResolution;
   Configure->WidthInBytes  = Configure->WidthInPixels * 
Configure->BytesPerPixel;
 
-  ASSERT (Configure->WidthInBytes < sizeof (Configure->LineBuffer));
-
   return RETURN_SUCCESS;
 }
 
--
2.9.0.windows.1

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

[edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event

[Zhang, Chao B]

Log Startup Locality Event according to TCG PC Client PFP 00.21.
Event should be placed before any extend to PCR[0]
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf

Cc: Star Zeng 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 48 ++-
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c 
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 3534fd1..2658944 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -1381,11 +1381,12 @@ SetupEventLog (
   UINT32  HashAlgorithmMaskCopied;
   TCG_EfiSpecIDEventStruct*TcgEfiSpecIdEventStruct;
   UINT8   TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + 
sizeof(UINT32) + (HASH_COUNT * sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + 
sizeof(UINT8)];
-  TCG_PCR_EVENT_HDR   FirstPcrEvent;
+  TCG_PCR_EVENT_HDR   NoActionEvent;
   TCG_EfiSpecIdEventAlgorithmSize *DigestSize;
   TCG_EfiSpecIdEventAlgorithmSize *TempDigestSize;
   UINT8   *VendorInfoSize;
   UINT32  NumberOfAlgorithms;
+  TCG_EfiStartupLocalityEvent StartupLocalityEvent;
 
   DEBUG ((EFI_D_INFO, "SetupEventLog\n"));
 
@@ -1468,24 +1469,49 @@ SetupEventLog (
 VendorInfoSize = (UINT8 *)TempDigestSize;
 *VendorInfoSize = 0;
 
+NoActionEvent.PCRIndex = 0;
+NoActionEvent.EventType = EV_NO_ACTION;
+ZeroMem (, sizeof(NoActionEvent.Digest));
+NoActionEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize 
(TcgEfiSpecIdEventStruct);
+
 //
-// FirstPcrEvent
+// Log TcgEfiSpecIdEventStruct as the first Event
+//   TCG PC Client PFP spec. Section 9.2 Measurement Event Entries and 
Log
 //
-FirstPcrEvent.PCRIndex = 0;
-FirstPcrEvent.EventType = EV_NO_ACTION;
-ZeroMem (, sizeof(FirstPcrEvent.Digest));
-FirstPcrEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize 
(TcgEfiSpecIdEventStruct);
+Status = TcgDxeLogEvent (
+   mTcg2EventInfo[Index].LogFormat,
+   ,
+   sizeof(NoActionEvent),
+   (UINT8 *)TcgEfiSpecIdEventStruct,
+   NoActionEvent.EventSize
+   );
 
 //
-// Record
+// EfiStartupLocalityEvent
+//
+CopyMem (StartupLocalityEvent.Signature, 
TCG_EfiStartupLocalityEvent_SIGNATURE, sizeof(StartupLocalityEvent.Signature));
+//
+// SRTM uses Locality 0 to access the TPM according to PC Client PFP 
spec 2.2.1
+//
+StartupLocalityEvent.StartupLocality = LOCALITY_0_INDICATOR;
+
+NoActionEvent.PCRIndex = 0;
+NoActionEvent.EventType = EV_NO_ACTION;
+ZeroMem (, sizeof(NoActionEvent.Digest));
+NoActionEvent.EventSize = sizeof(StartupLocalityEvent);
+
+//
+// Log EfiStartupLocalityEvent as the second Event
+//   TCG PC Client PFP spec. Section 9.3.4.3 Startup Locality Event
 //
 Status = TcgDxeLogEvent (
mTcg2EventInfo[Index].LogFormat,
-   ,
-   sizeof(FirstPcrEvent),
-   (UINT8 *)TcgEfiSpecIdEventStruct,
-   FirstPcrEvent.EventSize
+   ,
+   sizeof(NoActionEvent),
+   (UINT8 *),
+   NoActionEvent.EventSize
);
+
   }
 }
   }
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH 2/2] MdePkg : UefiTcgPlatform.h: Define Startup Locality Event & Indicator

[Zhang, Chao B]

Add Startup Locality Event definition according to PC Client PFP 00.21
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Add Locality Indicator definition according to PC Client PTP 00.43
https://www.trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2-0-v43-150126.pdf

Cc: Star Zeng 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 29 ++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h 
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 23eaa53..6ce808e 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -1,7 +1,7 @@
 /** @file
   TCG EFI Platform Definition in TCG_EFI_Platform_1_20_Final
 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -268,6 +268,33 @@ typedef struct {
 //UINT8   vendorInfo[vendorInfoSize];
 } TCG_EfiSpecIDEventStruct;
 
+
+
+#define TCG_EfiStartupLocalityEvent_SIGNATURE  "StartupLocality"
+
+
+//
+// PC Client PTP spec Table 8 Relationship between Locality and Locality 
Attribute
+//
+#define LOCALITY_0_INDICATOR0x01
+#define LOCALITY_1_INDICATOR0x02
+#define LOCALITY_2_INDICATOR0x03
+#define LOCALITY_3_INDICATOR0x04
+#define LOCALITY_4_INDICATOR0x05
+
+
+//
+// Startup Locality Event
+//
+typedef struct tdTCG_EfiStartupLocalityEvent{
+  UINT8   Signature[16];
+  //
+  // The Locality Indicator which sent the TPM2_Startup command
+  //
+  UINT8   StartupLocality;
+} TCG_EfiStartupLocalityEvent;
+
+
 //
 // Restore original structure alignment
 //
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Fixed crash on Juno R0

[Ryan Harkin]

On 20 January 2017 at 20:57, Daniil Egranov  wrote:
> Hi Ryan,
>
>
> On 01/20/2017 04:30 AM, Ryan Harkin wrote:
>
> On 20 January 2017 at 01:34, Daniil Egranov  wrote:
>
> Hi Leif, Ryan
>
>
> On 01/19/2017 09:13 AM, Leif Lindholm wrote:
>
> On Thu, Jan 19, 2017 at 01:49:04PM +, Ryan Harkin wrote:
>
> On 18 January 2017 at 23:27, Daniil Egranov  wrote:
>
> The Marvell Yukon MAC address load supported only on Juno R1 and R2.
> It disabled for Juno R0 due to PCI issues on this board.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daniil Egranov 
>
> Tested-by: Ryan Harkin 
>
> ---
>  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> index 47ff587..e9e6990 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> @@ -378,6 +378,7 @@ OnEndOfDxe (
>EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
>EFI_HANDLEHandle;
>EFI_STATUSStatus;
> +  UINT32JunoRevision;
>
>//
>// PCI Root Complex initialization
> @@ -393,8 +394,12 @@ OnEndOfDxe (
>Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
> FALSE);
>ASSERT_EFI_ERROR (Status);
>
> -  Status = ArmJunoSetNicMacAddress ();
> -  ASSERT_EFI_ERROR (Status);
> +  GetJunoRevision (JunoRevision);
> +
> +  if (JunoRevision != JUNO_REVISION_R0) {
> +Status = ArmJunoSetNicMacAddress ();
> +ASSERT_EFI_ERROR (Status);
>
> This is just an FYI, but I stacked your patch on top of mainline, like this:
>
> 5f81f61  2017-01-18  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe:
> Fixed crash on Juno R0   [Daniil Egranov]
> 19ca06b  2017-01-19  OvmfPkg: Remove superfluous return statements.
>  [Thomas Huth]
>
> The first time I ran this, Juno R0 worked fine, but on R1 and R2, the
> assert triggered:
>
> UEFI firmware (version 5f81f61 built at 11:56:52 on Jan 19 2017)
> [snip]
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe]
> /linaro/platforms/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(401):
> !EFI_ERROR (Status)
>
> I worked out what is happening. And it's not to do with this patch.
> It's another fall-out from the re-work you did to the previous patch.
> It's also ultimately due to a bug the firmware.
>
> With the initial version of your "Set Marvell Yukon MAC address"
> patch, this hang didn't happen. I suspect that was because your error
> checking was weaker and certain PCIe failures didn't trigger the
> assert.
>
> To reproduce the error with this commit:
> 1) power on and boot R1 or R2 into Shell
>   I do this by interrupting the boot by pressing ESCAPE and using the boot
> menu
> 2) At the Shell prompt, run "reset -s" to shutdown
> 3) At the ARM Boot Loader "Cmd>" prompt, run "reboot"
> 4) the board will hang while booting UEFI, assuming the board firmware
> doesn't die with constant messages like this:
>
> ERROR: PCIe CSR read failed to respond
> ERROR: SMBus transaction not claimed
>
> Assuming the problem is firmware, not EDK2, what should we do about it?
>
> OK, so instinctively, my reaction was that "the reset -s bug is a
> system controller firmware bug and we shouldn't work around
> it". However, since it is actually disrupting Ryan's workflow, which
> frequently doesn't touch PCI at all, I think downgrading the ASSERT to
> an error message is a good idea short-term.
>
> Daniil - could you make that change please?
>
> /
> Leif
>
>
> I've been able to reproduce "PCIe CSR read failed to respond" and "SMBus
> transaction not claimed" errors on my Juno R2. I disabled Marvell Yukon
> driver (.dsc/.fdf) and removed ArmJunoDxe patch but still see the same error
> messages during the initial boot.
>
> Testing motherboard interfaces (FPGA build 118)...
> SRAM 32MB test: PASSED
> LAN9118   test: PASSED
> KMI1/2test: PASSED
> MMC   test: PASSED
> PB/LEDs   test: PASSED
> FPGA UART test: PASSED
> ERROR: PCIe CSR read failed to respond
> ERROR: SMBus transaction not claimed
> ERROR: PCIe CSR read failed to respond
> ...
>
> Once it went through reporting these errors, the UEFI starts loading but
> still fails in OnEndOfDxe():
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe]
> /home/user/workspace/juno/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(110):
> !EFI_ERROR (Status)
>
> This is the original ArmJunoDxe code:
>   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
> FALSE);
>   ASSERT_EFI_ERROR (Status); < line 110
>
> I'm confused at which point in the commit history this is line 110.
>
> Just before your MAC 

Re: [edk2] [PATCH 3/3] OvmfPkg/QemuVideoDxe: Frame buffer config size may change in new mode

[Laszlo Ersek]

On 01/23/17 07:11, Ruiyu Ni wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=339
> 
> The patch removes the assumption in QemuVideoDxe driver that it
> wrongly assumes the frame buffer configure size is the same in
> different video modes.
> The assumption is true in old FrameBufferBltLib but is false in
> new implementation.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  OvmfPkg/QemuVideoDxe/Gop.c | 47 
> +-
>  1 file changed, 26 insertions(+), 21 deletions(-)

Reviewed-by: Laszlo Ersek 

Thanks!
Laszlo

> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> index 5485ba3..359e921 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -1,7 +1,7 @@
>  /** @file
>Graphics Output Protocol functions for the QEMU video controller.
>  
> -  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2007 - 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
> @@ -189,30 +189,35 @@ Routine Description:
>QemuVideoCompleteModeData (Private, This->Mode);
>  
>//
> -  // Allocate when using first time.
> +  // Re-initialize the frame buffer configure when mode changes.
>//
> -  if (Private->FrameBufferBltConfigure == NULL) {
> -Status = FrameBufferBltConfigure (
> -   (VOID*) (UINTN) This->Mode->FrameBufferBase,
> -   This->Mode->Info,
> -   Private->FrameBufferBltConfigure,
> -   >FrameBufferBltConfigureSize
> -   );
> -ASSERT (Status == RETURN_BUFFER_TOO_SMALL);
> +  Status = FrameBufferBltConfigure (
> + (VOID*) (UINTN) This->Mode->FrameBufferBase,
> + This->Mode->Info,
> + Private->FrameBufferBltConfigure,
> + >FrameBufferBltConfigureSize
> + );
> +  if (Status == RETURN_BUFFER_TOO_SMALL) {
> +//
> +// Frame buffer configure may be larger in new mode.
> +//
> +if (Private->FrameBufferBltConfigure != NULL) {
> +  FreePool (Private->FrameBufferBltConfigure);
> +}
>  Private->FrameBufferBltConfigure =
>AllocatePool (Private->FrameBufferBltConfigureSize);
> -  }
> +ASSERT (Private->FrameBufferBltConfigure != NULL);
>  
> -  //
> -  // Create the configuration for FrameBufferBltLib
> -  //
> -  ASSERT (Private->FrameBufferBltConfigure != NULL);
> -  Status = FrameBufferBltConfigure (
> -  (VOID*) (UINTN) This->Mode->FrameBufferBase,
> -  This->Mode->Info,
> -  Private->FrameBufferBltConfigure,
> -  >FrameBufferBltConfigureSize
> -  );
> +//
> +// Create the configuration for FrameBufferBltLib
> +//
> +Status = FrameBufferBltConfigure (
> +(VOID*) (UINTN) This->Mode->FrameBufferBase,
> +This->Mode->Info,
> +Private->FrameBufferBltConfigure,
> +>FrameBufferBltConfigureSize
> +);
> +  }
>ASSERT (Status == RETURN_SUCCESS);
>  
>return EFI_SUCCESS;
> 

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

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Fixed crash on Juno R0

[Ryan Harkin]

On 20 January 2017 at 20:57, Daniil Egranov  wrote:
> Hi Ryan,
>
>
> On 01/20/2017 04:30 AM, Ryan Harkin wrote:
>
> On 20 January 2017 at 01:34, Daniil Egranov  wrote:
>
> Hi Leif, Ryan
>
>
> On 01/19/2017 09:13 AM, Leif Lindholm wrote:
>
> On Thu, Jan 19, 2017 at 01:49:04PM +, Ryan Harkin wrote:
>
> On 18 January 2017 at 23:27, Daniil Egranov  wrote:
>
> The Marvell Yukon MAC address load supported only on Juno R1 and R2.
> It disabled for Juno R0 due to PCI issues on this board.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daniil Egranov 
>
> Tested-by: Ryan Harkin 
>
> ---
>  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> index 47ff587..e9e6990 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
> @@ -378,6 +378,7 @@ OnEndOfDxe (
>EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
>EFI_HANDLEHandle;
>EFI_STATUSStatus;
> +  UINT32JunoRevision;
>
>//
>// PCI Root Complex initialization
> @@ -393,8 +394,12 @@ OnEndOfDxe (
>Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
> FALSE);
>ASSERT_EFI_ERROR (Status);
>
> -  Status = ArmJunoSetNicMacAddress ();
> -  ASSERT_EFI_ERROR (Status);
> +  GetJunoRevision (JunoRevision);
> +
> +  if (JunoRevision != JUNO_REVISION_R0) {
> +Status = ArmJunoSetNicMacAddress ();
> +ASSERT_EFI_ERROR (Status);
>
> This is just an FYI, but I stacked your patch on top of mainline, like this:
>
> 5f81f61  2017-01-18  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe:
> Fixed crash on Juno R0   [Daniil Egranov]
> 19ca06b  2017-01-19  OvmfPkg: Remove superfluous return statements.
>  [Thomas Huth]
>
> The first time I ran this, Juno R0 worked fine, but on R1 and R2, the
> assert triggered:
>
> UEFI firmware (version 5f81f61 built at 11:56:52 on Jan 19 2017)
> [snip]
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe]
> /linaro/platforms/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(401):
> !EFI_ERROR (Status)
>
> I worked out what is happening. And it's not to do with this patch.
> It's another fall-out from the re-work you did to the previous patch.
> It's also ultimately due to a bug the firmware.
>
> With the initial version of your "Set Marvell Yukon MAC address"
> patch, this hang didn't happen. I suspect that was because your error
> checking was weaker and certain PCIe failures didn't trigger the
> assert.
>
> To reproduce the error with this commit:
> 1) power on and boot R1 or R2 into Shell
>   I do this by interrupting the boot by pressing ESCAPE and using the boot
> menu
> 2) At the Shell prompt, run "reset -s" to shutdown
> 3) At the ARM Boot Loader "Cmd>" prompt, run "reboot"
> 4) the board will hang while booting UEFI, assuming the board firmware
> doesn't die with constant messages like this:
>
> ERROR: PCIe CSR read failed to respond
> ERROR: SMBus transaction not claimed
>
> Assuming the problem is firmware, not EDK2, what should we do about it?
>
> OK, so instinctively, my reaction was that "the reset -s bug is a
> system controller firmware bug and we shouldn't work around
> it". However, since it is actually disrupting Ryan's workflow, which
> frequently doesn't touch PCI at all, I think downgrading the ASSERT to
> an error message is a good idea short-term.
>
> Daniil - could you make that change please?
>
> /
> Leif
>
>
> I've been able to reproduce "PCIe CSR read failed to respond" and "SMBus
> transaction not claimed" errors on my Juno R2. I disabled Marvell Yukon
> driver (.dsc/.fdf) and removed ArmJunoDxe patch but still see the same error
> messages during the initial boot.
>
> Testing motherboard interfaces (FPGA build 118)...
> SRAM 32MB test: PASSED
> LAN9118   test: PASSED
> KMI1/2test: PASSED
> MMC   test: PASSED
> PB/LEDs   test: PASSED
> FPGA UART test: PASSED
> ERROR: PCIe CSR read failed to respond
> ERROR: SMBus transaction not claimed
> ERROR: PCIe CSR read failed to respond
> ...
>
> Once it went through reporting these errors, the UEFI starts loading but
> still fails in OnEndOfDxe():
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe]
> /home/user/workspace/juno/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(110):
> !EFI_ERROR (Status)
>
> This is the original ArmJunoDxe code:
>   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
> FALSE);
>   ASSERT_EFI_ERROR (Status); < line 110
>
> I'm confused at which point in the commit history this is line 110.
>
> Just before your MAC 

Re: [edk2] [PATCH v3 0/3] Enable the HTTP connections switch

[Laszlo Ersek]

On 01/23/17 03:40, Wu, Jiaxin wrote:
>> Subject: [PATCH v3 0/3] Enable the HTTP connections switch
>>
>> v3:
>> * Append patch for OVMF.
>>
>> Cc: Laszlo Ersek 
>> Cc: Justen Jordan L 
>> Cc: Gary Lin 
>> Cc: Ye Ting 
>> Cc: Fu Siyuan 
>> Cc: Kinney Michael D 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Wu Jiaxin 
>>
>> Jiaxin Wu (3):
>>   NetworkPkg: Add PCD to enable the HTTP connections switch
> 
> Commit:
> https://github.com/tianocore/edk2/commit/221463c2b337072532ed4ab8ffe3b566574724d8
> 
> 
>>   Nt32Pkg.dsc: Add flag to control HTTP connections
> 
> Commit:
> https://github.com/tianocore/edk2/commit/7c3c53e5e8ee066904e56d9e6dd85dad4933f29e
> 
> 
>>   OvmfPkg: Allow HTTP connections if HTTP Boot enabled
> 
> Commit:
> https://github.com/tianocore/edk2/commit/4b2fb7986d571827a6e4885377e531002d806681

Looks good, thanks!
Laszlo

> 
> 
> Thank you!
> Jiaxin
> 
> 
>>
>>  NetworkPkg/HttpBootDxe/HttpBootClient.c  | 20 +++-
>>  NetworkPkg/HttpBootDxe/HttpBootConfig.c  | 81 --
>> --
>>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |  5 +-
>>  NetworkPkg/HttpBootDxe/HttpBootSupport.c | 53 -
>>  NetworkPkg/HttpBootDxe/HttpBootSupport.h | 17 ++-
>>  NetworkPkg/HttpDxe/HttpDxe.inf   |  5 +-
>>  NetworkPkg/HttpDxe/HttpImpl.c| 12 -
>>  NetworkPkg/NetworkPkg.dec|  8 +++-
>>  Nt32Pkg/Nt32Pkg.dsc  | 18 ++-
>>  OvmfPkg/OvmfPkgIa32.dsc  |  6 ++-
>>  OvmfPkg/OvmfPkgIa32X64.dsc   |  6 ++-
>>  OvmfPkg/OvmfPkgX64.dsc   |  6 ++-
>>  12 files changed, 195 insertions(+), 42 deletions(-)
>>
>> --
>> 1.9.5.msysgit.1
> 

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

Re: [edk2] [PATCH 0/1] Refine casting expression result to bigger size

[Laszlo Ersek]

On 01/22/17 07:43, Hao Wu wrote:
> Please note that this patch is maily for feedback collection and the patch
> only covers MdePkg. We are working on patches for other packages.
> 
> 
> There are cases that the operands of an expression are all with rank less
> than UINT64/INT64 and the result of the expression is casted to
> UINT64/INT64 to fit the target size.
> 
> An example will be:
> UINT32 a,b;
> // a and b can be any unsigned int type with rank less than UINT64, like
> // UINT8, UINT16, etc.
> UINT64 c;
> c = (UINT64) (a + b);
> 
> Some static code checkers may warn that the expression result might
> overflow within the rank of int (integer promotions) and the result is
> then cast to a bigger size.
> 
> For the consideration of generated binaries size, the commit will keep the
> size of the operands as the size of int, and explitly add a type cast
> before converting the result to UINT64/INT64.
> 
> 1). When there is no operand with type UINTN
> (UINTN)  (a + b) -> (UINTN)(UINT32)  (a + b) or
> (UINT64) (a + b) -> (UINT64)(UINT32) (a + b)
> 
> 2). Otherwise
> (UINT64) (a + b) -> (UINT64)(UINTN)  (a + b)
> 
> Hao Wu (1):
>   MdePkg: Refine casting expression result to bigger size
> 
>  MdePkg/Library/BaseLib/String.c  | 4 ++--
>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c| 4 ++--
>  MdePkg/Library/BaseS3PciLib/S3PciLib.c   | 4 ++--
>  MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c  | 4 ++--
>  MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c | 4 ++--
>  5 files changed, 10 insertions(+), 10 deletions(-)
> 

- If it is justified to do the addition in 64-bits (or in UINTN), then
the addition should be modified accordingly. This might incur a code
size increase, but if that's necessary for correctness, then it should
be done.

- However, if the addition is just fine as-is, because we know for sure
that the sum will never overflow "int" or "unsigned int" (as selected by
the integer promotions and the usual arithmetic conversions), then we're
addressing the issue in the wrong direction. Namely, in this case, the
solution is to simply drop the outermost cast, which is already useless.
(Because, it would automatically happen as part of the assignment or the
"return" statement.)

I mean... Those (useless) outermost casts were probably introduced to
appease the Visual Studio compiler. Apparently, they cause various
static code checkers to complain, so now we introduce yet more casts to
keep them quiet as well. When will it end?

For example, the 2nd return statement of the InternalHexCharToUintn()
function is proposed as

  return (UINTN)(UINT32) (10 + InternalCharToUpper (Char) - L'A');

in reality it should be just

  return 10 + InternalCharToUpper (Char) - L'A';

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

Re: [edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event

[Yao, Jiewen]

Hi
StartupLocality is a platform policy. We should not hardcode it.

We may use one of below ways:
1) Define a new PCD.
2) Detect if there is an startuplocality event hob reported in PEI phase.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang,
> Chao B
> Sent: Monday, January 23, 2017 4:52 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zhang, Chao B
> ; Zeng, Star 
> Subject: [edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event
>
> Log Startup Locality Event according to TCG PC Client PFP 00.21.
> Event should be placed before any extend to PCR[0]
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
>
> Cc: Star Zeng 
> Cc: Yao Jiewen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 48
> ++-
>  1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index 3534fd1..2658944 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -1381,11 +1381,12 @@ SetupEventLog (
>UINT32  HashAlgorithmMaskCopied;
>TCG_EfiSpecIDEventStruct*TcgEfiSpecIdEventStruct;
>UINT8
> TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + sizeof(UINT32) + (HASH_COUNT *
> sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + sizeof(UINT8)];
> -  TCG_PCR_EVENT_HDR   FirstPcrEvent;
> +  TCG_PCR_EVENT_HDR   NoActionEvent;
>TCG_EfiSpecIdEventAlgorithmSize *DigestSize;
>TCG_EfiSpecIdEventAlgorithmSize *TempDigestSize;
>UINT8   *VendorInfoSize;
>UINT32  NumberOfAlgorithms;
> +  TCG_EfiStartupLocalityEvent StartupLocalityEvent;
>
>DEBUG ((EFI_D_INFO, "SetupEventLog\n"));
>
> @@ -1468,24 +1469,49 @@ SetupEventLog (
>  VendorInfoSize = (UINT8 *)TempDigestSize;
>  *VendorInfoSize = 0;
>
> +NoActionEvent.PCRIndex = 0;
> +NoActionEvent.EventType = EV_NO_ACTION;
> +ZeroMem (, sizeof(NoActionEvent.Digest));
> +NoActionEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize
> (TcgEfiSpecIdEventStruct);
> +
>  //
> -// FirstPcrEvent
> +// Log TcgEfiSpecIdEventStruct as the first Event
> +//   TCG PC Client PFP spec. Section 9.2 Measurement Event Entries
> and Log
>  //
> -FirstPcrEvent.PCRIndex = 0;
> -FirstPcrEvent.EventType = EV_NO_ACTION;
> -ZeroMem (, sizeof(FirstPcrEvent.Digest));
> -FirstPcrEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize
> (TcgEfiSpecIdEventStruct);
> +Status = TcgDxeLogEvent (
> +   mTcg2EventInfo[Index].LogFormat,
> +   ,
> +   sizeof(NoActionEvent),
> +   (UINT8 *)TcgEfiSpecIdEventStruct,
> +   NoActionEvent.EventSize
> +   );
>
>  //
> -// Record
> +// EfiStartupLocalityEvent
> +//
> +CopyMem (StartupLocalityEvent.Signature,
> TCG_EfiStartupLocalityEvent_SIGNATURE,
> sizeof(StartupLocalityEvent.Signature));
> +//
> +// SRTM uses Locality 0 to access the TPM according to PC Client PFP
> spec 2.2.1
> +//
> +StartupLocalityEvent.StartupLocality = LOCALITY_0_INDICATOR;
> +
> +NoActionEvent.PCRIndex = 0;
> +NoActionEvent.EventType = EV_NO_ACTION;
> +ZeroMem (, sizeof(NoActionEvent.Digest));
> +NoActionEvent.EventSize = sizeof(StartupLocalityEvent);
> +
> +//
> +// Log EfiStartupLocalityEvent as the second Event
> +//   TCG PC Client PFP spec. Section 9.3.4.3 Startup Locality Event
>  //
>  Status = TcgDxeLogEvent (
> mTcg2EventInfo[Index].LogFormat,
> -   ,
> -   sizeof(FirstPcrEvent),
> -   (UINT8 *)TcgEfiSpecIdEventStruct,
> -   FirstPcrEvent.EventSize
> +   ,
> +   sizeof(NoActionEvent),
> +   (UINT8 *),
> +   NoActionEvent.EventSize
> );
> +
>}
>  }
>}
> --
> 1.9.5.msysgit.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Carsey, Jaben]

Tim,

I meant some customers do localize the shell and we don't want to arbitrarily 
restrict that.

Do you have any ideas on how to remove the overhead when localization is not 
required?

-Jaben

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Monday, January 23, 2017 1:25 PM
> To: Carsey, Jaben ; Ni, Ruiyu ;
> Zeng, Star ; edk2-devel@lists.01.org
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec
> 3.1.1
> Importance: High
> 
> Jaben --
> 
> I don't know exactly what you mean by each "compiler of the shell". I do know
> that HII strings and HII string handling add the largest discretionary, 
> non-spec-
> required overhead to the Shell executable (based on our production mini-
> shell vs. the EDK2 shell) even without any second language.  I understand it 
> as
> a design goal of the EDK2 shell to allow localization (although it does not
> actually do any itself). I also understand that it is easier to enforce the 
> strings-
> in-uni rule as an absolute. But it does come with a cost for those who want to
> use a fully functional shell in a much more restricted environment.
> 
> 
> Tim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Carsey, Jaben
> Sent: Monday, January 23, 2017 1:15 PM
> To: Tim Lewis ; Ni, Ruiyu ; Zeng,
> Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> SMBIOS spec 3.1.1
> 
> Tim,
> 
> I agree that some strings are directly from SMBIOS or UEFI specifications and
> are not needed to be localized.
> 
> Is there a disadvantage to allowing each compiler of the shell to make that an
> individual decision?
> 
> In either case, it might be worthwhile to verify that all of the embedded 
> strings
> meet that criterion.
> 
> -Jaben
> 
> > -Original Message-
> > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > Sent: Monday, January 23, 2017 12:52 PM
> > To: Carsey, Jaben ; Ni, Ruiyu
> > ; Zeng, Star ; edk2-
> > de...@lists.01.org
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec 3.1.1
> > Importance: High
> >
> > Also, in some cases, the text is taken directly from the specification.
> > Introducing HII strings in order to make these translatable when the
> > source material is normative doesn't help, IMO.
> >
> > Tim
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Carsey, Jaben
> > Sent: Monday, January 23, 2017 9:41 AM
> > To: Ni, Ruiyu ; Zeng, Star ;
> > edk2- de...@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> > SMBIOS spec 3.1.1
> >
> > Reviewed-by: Jaben Carsey 
> >
> > I think that string mixed use existed in the EDK version of the
> > command and was just never removed.
> >
> > -Jaben
> >
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Sunday, January 22, 2017 1:49 AM
> > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > > spec 3.1.1
> > > Importance: High
> > >
> > > Reviewed-by: Ruiyu Ni 
> > >
> > > Thanks/Ray
> > >
> > > > -Original Message-
> > > > From: Zeng, Star
> > > > Sent: Sunday, January 22, 2017 5:25 PM
> > > > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > > > Cc: Carsey, Jaben ; Zeng, Star
> > > > 
> > > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> > > > SMBIOS spec 3.1.1
> > > >
> > > > Ray & Jaben,
> > > >
> > > > I am not so sure about the rule.
> > > > The mixed using of strings in c code and strings in uni file has
> > > > been there since it was created.
> > > >
> > > >
> > > > Thanks,
> > > > Star
> > > > -Original Message-
> > > > From: Ni, Ruiyu
> > > > Sent: Sunday, January 22, 2017 4:56 PM
> > > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > > Cc: Carsey, Jaben 
> > > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> > > > SMBIOS spec 3.1.1
> > > >
> > > > Star,
> > > > Why some strings are hardcoded in C file while some are defined in
> > > > UNI
> > > file?
> > > > What's the rule?
> > > >
> > > > Thanks/Ray
> > > >
> > > > > -Original Message-
> > > > > From: Zeng, Star
> > > > > Sent: Sunday, January 22, 2017 4:18 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Zeng, Star ; Ni, Ruiyu
> > > > > ; Carsey, 

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Tim Lewis]

We have not found a good solution for handling this automatically, although we 
have toyed with alternate ways to process the .uni file to create string 
literals with the StrDefs.h #defines and alternate versions of the ShellLib 
functions that use them. 

Tim

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com] 
Sent: Monday, January 23, 2017 1:48 PM
To: Tim Lewis ; Ni, Ruiyu ; Zeng, 
Star ; edk2-devel@lists.01.org
Cc: Carsey, Jaben 
Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

Tim,

I meant some customers do localize the shell and we don't want to arbitrarily 
restrict that.

Do you have any ideas on how to remove the overhead when localization is not 
required?

-Jaben

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Monday, January 23, 2017 1:25 PM
> To: Carsey, Jaben ; Ni, Ruiyu 
> ; Zeng, Star ; 
> edk2-devel@lists.01.org
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> spec
> 3.1.1
> Importance: High
> 
> Jaben --
> 
> I don't know exactly what you mean by each "compiler of the shell". I 
> do know that HII strings and HII string handling add the largest 
> discretionary, non-spec- required overhead to the Shell executable 
> (based on our production mini- shell vs. the EDK2 shell) even without 
> any second language.  I understand it as a design goal of the EDK2 
> shell to allow localization (although it does not actually do any 
> itself). I also understand that it is easier to enforce the strings- 
> in-uni rule as an absolute. But it does come with a cost for those who want 
> to use a fully functional shell in a much more restricted environment.
> 
> 
> Tim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Carsey, Jaben
> Sent: Monday, January 23, 2017 1:15 PM
> To: Tim Lewis ; Ni, Ruiyu ; 
> Zeng, Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> SMBIOS spec 3.1.1
> 
> Tim,
> 
> I agree that some strings are directly from SMBIOS or UEFI 
> specifications and are not needed to be localized.
> 
> Is there a disadvantage to allowing each compiler of the shell to make 
> that an individual decision?
> 
> In either case, it might be worthwhile to verify that all of the 
> embedded strings meet that criterion.
> 
> -Jaben
> 
> > -Original Message-
> > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > Sent: Monday, January 23, 2017 12:52 PM
> > To: Carsey, Jaben ; Ni, Ruiyu 
> > ; Zeng, Star ; edk2- 
> > de...@lists.01.org
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> > spec 3.1.1
> > Importance: High
> >
> > Also, in some cases, the text is taken directly from the specification.
> > Introducing HII strings in order to make these translatable when the 
> > source material is normative doesn't help, IMO.
> >
> > Tim
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Carsey, Jaben
> > Sent: Monday, January 23, 2017 9:41 AM
> > To: Ni, Ruiyu ; Zeng, Star 
> > ;
> > edk2- de...@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > SMBIOS spec 3.1.1
> >
> > Reviewed-by: Jaben Carsey 
> >
> > I think that string mixed use existed in the EDK version of the 
> > command and was just never removed.
> >
> > -Jaben
> >
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Sunday, January 22, 2017 1:49 AM
> > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > SMBIOS spec 3.1.1
> > > Importance: High
> > >
> > > Reviewed-by: Ruiyu Ni 
> > >
> > > Thanks/Ray
> > >
> > > > -Original Message-
> > > > From: Zeng, Star
> > > > Sent: Sunday, January 22, 2017 5:25 PM
> > > > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > > > Cc: Carsey, Jaben ; Zeng, Star 
> > > > 
> > > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > > SMBIOS spec 3.1.1
> > > >
> > > > Ray & Jaben,
> > > >
> > > > I am not so sure about the rule.
> > > > The mixed using of strings in c code and strings in uni file has 
> > > > been there since it was created.
> > > >
> > > >
> > > > Thanks,
> > > > Star
> > > > -Original Message-
> > > > From: Ni, Ruiyu
> > > > Sent: Sunday, January 

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Tim Lewis]

In our case, the main firmware is multi-lingual, but the shell is mono-lingual, 
and they are built together. The reason is: setup users require more 
user-friendly than shell users in our experience.

Tim

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com] 
Sent: Monday, January 23, 2017 1:59 PM
To: Tim Lewis ; Ni, Ruiyu ; Zeng, 
Star ; edk2-devel@lists.01.org
Cc: Carsey, Jaben 
Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

I was wondering if some replaced version of HiiLib could use the StrDefs.h 
differently and save the overhead if only one language is present...

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Tim Lewis
> Sent: Monday, January 23, 2017 1:57 PM
> To: Carsey, Jaben ; Ni, Ruiyu 
> ; Zeng, Star ; 
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> SMBIOS spec 3.1.1
> Importance: High
> 
> We have not found a good solution for handling this automatically, 
> although we have toyed with alternate ways to process the .uni file to 
> create string literals with the StrDefs.h #defines and alternate 
> versions of the ShellLib functions that use them.
> 
> Tim
> 
> -Original Message-
> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> Sent: Monday, January 23, 2017 1:48 PM
> To: Tim Lewis ; Ni, Ruiyu ; 
> Zeng, Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> spec
> 3.1.1
> 
> Tim,
> 
> I meant some customers do localize the shell and we don't want to 
> arbitrarily restrict that.
> 
> Do you have any ideas on how to remove the overhead when localization 
> is not required?
> 
> -Jaben
> 
> > -Original Message-
> > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > Sent: Monday, January 23, 2017 1:25 PM
> > To: Carsey, Jaben ; Ni, Ruiyu 
> > ; Zeng, Star ; 
> > edk2-devel@lists.01.org
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> > spec
> > 3.1.1
> > Importance: High
> >
> > Jaben --
> >
> > I don't know exactly what you mean by each "compiler of the shell". 
> > I do know that HII strings and HII string handling add the largest 
> > discretionary, non-spec- required overhead to the Shell executable 
> > (based on our production mini- shell vs. the EDK2 shell) even 
> > without any second language.  I understand it as a design goal of 
> > the EDK2 shell to allow localization (although it does not actually 
> > do any itself). I also understand that it is easier to enforce the 
> > strings- in-uni rule as an absolute. But it does come with a cost 
> > for those who want
> to use a fully functional shell in a much more restricted environment.
> >
> >
> > Tim
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Carsey, Jaben
> > Sent: Monday, January 23, 2017 1:15 PM
> > To: Tim Lewis ; Ni, Ruiyu 
> > ; Zeng, Star ; 
> > edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > SMBIOS spec 3.1.1
> >
> > Tim,
> >
> > I agree that some strings are directly from SMBIOS or UEFI 
> > specifications and are not needed to be localized.
> >
> > Is there a disadvantage to allowing each compiler of the shell to 
> > make that an individual decision?
> >
> > In either case, it might be worthwhile to verify that all of the 
> > embedded strings meet that criterion.
> >
> > -Jaben
> >
> > > -Original Message-
> > > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > > Sent: Monday, January 23, 2017 12:52 PM
> > > To: Carsey, Jaben ; Ni, Ruiyu 
> > > ; Zeng, Star ; edk2- 
> > > de...@lists.01.org
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > SMBIOS spec 3.1.1
> > > Importance: High
> > >
> > > Also, in some cases, the text is taken directly from the specification.
> > > Introducing HII strings in order to make these translatable when 
> > > the source material is normative doesn't help, IMO.
> > >
> > > Tim
> > >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On 
> > > Behalf Of Carsey, Jaben
> > > Sent: Monday, January 23, 2017 9:41 AM
> > > To: Ni, Ruiyu ; Zeng, Star 
> > > ;
> > > edk2- de...@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: 

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Carsey, Jaben]

I was wondering if some replaced version of HiiLib could use the StrDefs.h 
differently and save the overhead if only one language is present...

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tim
> Lewis
> Sent: Monday, January 23, 2017 1:57 PM
> To: Carsey, Jaben ; Ni, Ruiyu ;
> Zeng, Star ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> SMBIOS spec 3.1.1
> Importance: High
> 
> We have not found a good solution for handling this automatically, although
> we have toyed with alternate ways to process the .uni file to create string
> literals with the StrDefs.h #defines and alternate versions of the ShellLib
> functions that use them.
> 
> Tim
> 
> -Original Message-
> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> Sent: Monday, January 23, 2017 1:48 PM
> To: Tim Lewis ; Ni, Ruiyu ; Zeng,
> Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec
> 3.1.1
> 
> Tim,
> 
> I meant some customers do localize the shell and we don't want to arbitrarily
> restrict that.
> 
> Do you have any ideas on how to remove the overhead when localization is
> not required?
> 
> -Jaben
> 
> > -Original Message-
> > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > Sent: Monday, January 23, 2017 1:25 PM
> > To: Carsey, Jaben ; Ni, Ruiyu
> > ; Zeng, Star ;
> > edk2-devel@lists.01.org
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec
> > 3.1.1
> > Importance: High
> >
> > Jaben --
> >
> > I don't know exactly what you mean by each "compiler of the shell". I
> > do know that HII strings and HII string handling add the largest
> > discretionary, non-spec- required overhead to the Shell executable
> > (based on our production mini- shell vs. the EDK2 shell) even without
> > any second language.  I understand it as a design goal of the EDK2
> > shell to allow localization (although it does not actually do any
> > itself). I also understand that it is easier to enforce the strings-
> > in-uni rule as an absolute. But it does come with a cost for those who want
> to use a fully functional shell in a much more restricted environment.
> >
> >
> > Tim
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Carsey, Jaben
> > Sent: Monday, January 23, 2017 1:15 PM
> > To: Tim Lewis ; Ni, Ruiyu ;
> > Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> > SMBIOS spec 3.1.1
> >
> > Tim,
> >
> > I agree that some strings are directly from SMBIOS or UEFI
> > specifications and are not needed to be localized.
> >
> > Is there a disadvantage to allowing each compiler of the shell to make
> > that an individual decision?
> >
> > In either case, it might be worthwhile to verify that all of the
> > embedded strings meet that criterion.
> >
> > -Jaben
> >
> > > -Original Message-
> > > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > > Sent: Monday, January 23, 2017 12:52 PM
> > > To: Carsey, Jaben ; Ni, Ruiyu
> > > ; Zeng, Star ; edk2-
> > > de...@lists.01.org
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > > spec 3.1.1
> > > Importance: High
> > >
> > > Also, in some cases, the text is taken directly from the specification.
> > > Introducing HII strings in order to make these translatable when the
> > > source material is normative doesn't help, IMO.
> > >
> > > Tim
> > >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Carsey, Jaben
> > > Sent: Monday, January 23, 2017 9:41 AM
> > > To: Ni, Ruiyu ; Zeng, Star
> > > ;
> > > edk2- de...@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> > > SMBIOS spec 3.1.1
> > >
> > > Reviewed-by: Jaben Carsey 
> > >
> > > I think that string mixed use existed in the EDK version of the
> > > command and was just never removed.
> > >
> > > -Jaben
> > >
> > > > -Original Message-
> > > > From: Ni, Ruiyu
> > > > Sent: Sunday, January 22, 2017 1:49 AM
> > > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > > Cc: Carsey, Jaben 
> > > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> > > > SMBIOS spec 3.1.1
> > > > Importance: High
> > > >
> > > > Reviewed-by: 

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Tim Lewis]

Jaben --

I don't know exactly what you mean by each "compiler of the shell". I do know 
that HII strings and HII string handling add the largest discretionary, 
non-spec-required overhead to the Shell executable (based on our production 
mini-shell vs. the EDK2 shell) even without any second language.  I understand 
it as a design goal of the EDK2 shell to allow localization (although it does 
not actually do any itself). I also understand that it is easier to enforce the 
strings-in-uni rule as an absolute. But it does come with a cost for those who 
want to use a fully functional shell in a much more restricted environment.


Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Carsey, 
Jaben
Sent: Monday, January 23, 2017 1:15 PM
To: Tim Lewis ; Ni, Ruiyu ; Zeng, 
Star ; edk2-devel@lists.01.org
Cc: Carsey, Jaben 
Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
spec 3.1.1

Tim,

I agree that some strings are directly from SMBIOS or UEFI specifications and 
are not needed to be localized.  

Is there a disadvantage to allowing each compiler of the shell to make that an 
individual decision?

In either case, it might be worthwhile to verify that all of the embedded 
strings meet that criterion.

-Jaben

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Monday, January 23, 2017 12:52 PM
> To: Carsey, Jaben ; Ni, Ruiyu 
> ; Zeng, Star ; edk2- 
> de...@lists.01.org
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> spec 3.1.1
> Importance: High
> 
> Also, in some cases, the text is taken directly from the specification.
> Introducing HII strings in order to make these translatable when the 
> source material is normative doesn't help, IMO.
> 
> Tim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Carsey, Jaben
> Sent: Monday, January 23, 2017 9:41 AM
> To: Ni, Ruiyu ; Zeng, Star ; 
> edk2- de...@lists.01.org
> Cc: Carsey, Jaben 
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> SMBIOS spec 3.1.1
> 
> Reviewed-by: Jaben Carsey 
> 
> I think that string mixed use existed in the EDK version of the 
> command and was just never removed.
> 
> -Jaben
> 
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Sunday, January 22, 2017 1:49 AM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> > spec 3.1.1
> > Importance: High
> >
> > Reviewed-by: Ruiyu Ni 
> >
> > Thanks/Ray
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Sunday, January 22, 2017 5:25 PM
> > > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Zeng, Star 
> > > 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > SMBIOS spec 3.1.1
> > >
> > > Ray & Jaben,
> > >
> > > I am not so sure about the rule.
> > > The mixed using of strings in c code and strings in uni file has 
> > > been there since it was created.
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Sunday, January 22, 2017 4:56 PM
> > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > SMBIOS spec 3.1.1
> > >
> > > Star,
> > > Why some strings are hardcoded in C file while some are defined in 
> > > UNI
> > file?
> > > What's the rule?
> > >
> > > Thanks/Ray
> > >
> > > > -Original Message-
> > > > From: Zeng, Star
> > > > Sent: Sunday, January 22, 2017 4:18 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Zeng, Star ; Ni, Ruiyu 
> > > > ; Carsey, Jaben 
> > > > Subject: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec
> > > > 3.1.1
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
> > > >
> > > > Cc: Ruiyu Ni 
> > > > Cc: Jaben Carsey 
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Star Zeng 
> > > > ---
> > > >  .../SmbiosView/PrintInfo.c |  6 +++-
> > > >  .../SmbiosView/QueryTable.c| 37
> > ++
> > > >  .../SmbiosView/QueryTable.h| 14 +++-
> > > >  .../SmbiosView/SmbiosViewStrings.uni   |  3 +-
> > > >  4 files changed, 57 insertions(+), 3 

Re: [edk2] [PATCH 0/1] Refine casting expression result to bigger size

[Ard Biesheuvel]

On 23 January 2017 at 11:01, Laszlo Ersek  wrote:
> On 01/22/17 07:43, Hao Wu wrote:
>> Please note that this patch is maily for feedback collection and the patch
>> only covers MdePkg. We are working on patches for other packages.
>>
>>
>> There are cases that the operands of an expression are all with rank less
>> than UINT64/INT64 and the result of the expression is casted to
>> UINT64/INT64 to fit the target size.
>>
>> An example will be:
>> UINT32 a,b;
>> // a and b can be any unsigned int type with rank less than UINT64, like
>> // UINT8, UINT16, etc.
>> UINT64 c;
>> c = (UINT64) (a + b);
>>
>> Some static code checkers may warn that the expression result might
>> overflow within the rank of int (integer promotions) and the result is
>> then cast to a bigger size.
>>
>> For the consideration of generated binaries size, the commit will keep the
>> size of the operands as the size of int, and explitly add a type cast
>> before converting the result to UINT64/INT64.
>>
>> 1). When there is no operand with type UINTN
>> (UINTN)  (a + b) -> (UINTN)(UINT32)  (a + b) or
>> (UINT64) (a + b) -> (UINT64)(UINT32) (a + b)
>>
>> 2). Otherwise
>> (UINT64) (a + b) -> (UINT64)(UINTN)  (a + b)
>>
>> Hao Wu (1):
>>   MdePkg: Refine casting expression result to bigger size
>>
>>  MdePkg/Library/BaseLib/String.c  | 4 ++--
>>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c| 4 ++--
>>  MdePkg/Library/BaseS3PciLib/S3PciLib.c   | 4 ++--
>>  MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c  | 4 ++--
>>  MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c | 4 ++--
>>  5 files changed, 10 insertions(+), 10 deletions(-)
>>
>
> - If it is justified to do the addition in 64-bits (or in UINTN), then
> the addition should be modified accordingly. This might incur a code
> size increase, but if that's necessary for correctness, then it should
> be done.
>

Indeed. The problem with

c = (UINT64) (a + b);

is the parens: this should be written as (UINT64)a + b if the sum may
overflow a UINT32

> - However, if the addition is just fine as-is, because we know for sure
> that the sum will never overflow "int" or "unsigned int" (as selected by
> the integer promotions and the usual arithmetic conversions), then we're
> addressing the issue in the wrong direction. Namely, in this case, the
> solution is to simply drop the outermost cast, which is already useless.
> (Because, it would automatically happen as part of the assignment or the
> "return" statement.)
>

Agreed.

> I mean... Those (useless) outermost casts were probably introduced to
> appease the Visual Studio compiler. Apparently, they cause various
> static code checkers to complain, so now we introduce yet more casts to
> keep them quiet as well. When will it end?
>
> For example, the 2nd return statement of the InternalHexCharToUintn()
> function is proposed as
>
>   return (UINTN)(UINT32) (10 + InternalCharToUpper (Char) - L'A');
>
> in reality it should be just
>
>   return 10 + InternalCharToUpper (Char) - L'A';
>

Indeed. IMO this is related to our warnings-as-errors policy, while in
reality, some warnings are simply warnings, and should be ignored if
it can be confirmed by visual inspection that the expression can never
overflow.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Carsey, Jaben]

Tim,

I agree that some strings are directly from SMBIOS or UEFI specifications and 
are not needed to be localized.  

Is there a disadvantage to allowing each compiler of the shell to make that an 
individual decision?

In either case, it might be worthwhile to verify that all of the embedded 
strings meet that criterion.

-Jaben

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Monday, January 23, 2017 12:52 PM
> To: Carsey, Jaben ; Ni, Ruiyu
> ; Zeng, Star ; edk2-
> de...@lists.01.org
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> spec 3.1.1
> Importance: High
> 
> Also, in some cases, the text is taken directly from the specification.
> Introducing HII strings in order to make these translatable when the source
> material is normative doesn't help, IMO.
> 
> Tim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Carsey, Jaben
> Sent: Monday, January 23, 2017 9:41 AM
> To: Ni, Ruiyu ; Zeng, Star ; edk2-
> de...@lists.01.org
> Cc: Carsey, Jaben 
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> SMBIOS spec 3.1.1
> 
> Reviewed-by: Jaben Carsey 
> 
> I think that string mixed use existed in the EDK version of the command and
> was just never removed.
> 
> -Jaben
> 
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Sunday, January 22, 2017 1:49 AM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec 3.1.1
> > Importance: High
> >
> > Reviewed-by: Ruiyu Ni 
> >
> > Thanks/Ray
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Sunday, January 22, 2017 5:25 PM
> > > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Zeng, Star
> > > 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > > spec 3.1.1
> > >
> > > Ray & Jaben,
> > >
> > > I am not so sure about the rule.
> > > The mixed using of strings in c code and strings in uni file has
> > > been there since it was created.
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Sunday, January 22, 2017 4:56 PM
> > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > > spec 3.1.1
> > >
> > > Star,
> > > Why some strings are hardcoded in C file while some are defined in
> > > UNI
> > file?
> > > What's the rule?
> > >
> > > Thanks/Ray
> > >
> > > > -Original Message-
> > > > From: Zeng, Star
> > > > Sent: Sunday, January 22, 2017 4:18 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Zeng, Star ; Ni, Ruiyu
> > > > ; Carsey, Jaben 
> > > > Subject: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec
> > > > 3.1.1
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
> > > >
> > > > Cc: Ruiyu Ni 
> > > > Cc: Jaben Carsey 
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Star Zeng 
> > > > ---
> > > >  .../SmbiosView/PrintInfo.c |  6 +++-
> > > >  .../SmbiosView/QueryTable.c| 37
> > ++
> > > >  .../SmbiosView/QueryTable.h| 14 +++-
> > > >  .../SmbiosView/SmbiosViewStrings.uni   |  3 +-
> > > >  4 files changed, 57 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > >
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > >
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > index ecb8e2492453..1d6002b92593 100644
> > > > ---
> > > >
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > +++
> > > >
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > @@ -1106,7 +1106,7 @@ SmbiosPrintStructure (
> > > >// Management Controller Host Interface (Type 42)
> > > >//
> > > >case 42:
> > > > -PRINT_STRUCT_VALUE_H (Struct, Type42, InterfaceType);
> > > > +DisplayMCHostInterfaceType (Struct->Type42->InterfaceType,
> > > > + Option);
> > > >  break;
> > > >
> > > >//
> > > > @@ -1818,6 +1818,10 @@ DisplayProcessorFamily (
> > > >  Print (L"AMD Opteron(TM) X3000 Series APU\n");
> > > >  break;
> > > >
> > > > +  case 0x6B:
> > > > +Print (L"AMD Zen Processor Family\n");
> > > > +break;
> > > > +
> > > >case 0x70:
> > > >  

Re: [edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Fixed crash on Juno R0

[Daniil Egranov]

Hi Ryan,


On 01/23/2017 06:56 AM, Ryan Harkin wrote:

On 20 January 2017 at 20:57, Daniil Egranov  wrote:

Hi Ryan,


On 01/20/2017 04:30 AM, Ryan Harkin wrote:

On 20 January 2017 at 01:34, Daniil Egranov  wrote:

Hi Leif, Ryan


On 01/19/2017 09:13 AM, Leif Lindholm wrote:

On Thu, Jan 19, 2017 at 01:49:04PM +, Ryan Harkin wrote:

On 18 January 2017 at 23:27, Daniil Egranov  wrote:

The Marvell Yukon MAC address load supported only on Juno R1 and R2.
It disabled for Juno R0 due to PCI issues on this board.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 

Tested-by: Ryan Harkin 

---
  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index 47ff587..e9e6990 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -378,6 +378,7 @@ OnEndOfDxe (
EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
EFI_HANDLEHandle;
EFI_STATUSStatus;
+  UINT32JunoRevision;

//
// PCI Root Complex initialization
@@ -393,8 +394,12 @@ OnEndOfDxe (
Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
FALSE);
ASSERT_EFI_ERROR (Status);

-  Status = ArmJunoSetNicMacAddress ();
-  ASSERT_EFI_ERROR (Status);
+  GetJunoRevision (JunoRevision);
+
+  if (JunoRevision != JUNO_REVISION_R0) {
+Status = ArmJunoSetNicMacAddress ();
+ASSERT_EFI_ERROR (Status);

This is just an FYI, but I stacked your patch on top of mainline, like this:

5f81f61  2017-01-18  ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe:
Fixed crash on Juno R0   [Daniil Egranov]
19ca06b  2017-01-19  OvmfPkg: Remove superfluous return statements.
  [Thomas Huth]

The first time I ran this, Juno R0 worked fine, but on R1 and R2, the
assert triggered:

UEFI firmware (version 5f81f61 built at 11:56:52 on Jan 19 2017)
[snip]
ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [ArmJunoDxe]
/linaro/platforms/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(401):
!EFI_ERROR (Status)

I worked out what is happening. And it's not to do with this patch.
It's another fall-out from the re-work you did to the previous patch.
It's also ultimately due to a bug the firmware.

With the initial version of your "Set Marvell Yukon MAC address"
patch, this hang didn't happen. I suspect that was because your error
checking was weaker and certain PCIe failures didn't trigger the
assert.

To reproduce the error with this commit:
1) power on and boot R1 or R2 into Shell
   I do this by interrupting the boot by pressing ESCAPE and using the boot
menu
2) At the Shell prompt, run "reset -s" to shutdown
3) At the ARM Boot Loader "Cmd>" prompt, run "reboot"
4) the board will hang while booting UEFI, assuming the board firmware
doesn't die with constant messages like this:

 ERROR: PCIe CSR read failed to respond
 ERROR: SMBus transaction not claimed

Assuming the problem is firmware, not EDK2, what should we do about it?

OK, so instinctively, my reaction was that "the reset -s bug is a
system controller firmware bug and we shouldn't work around
it". However, since it is actually disrupting Ryan's workflow, which
frequently doesn't touch PCI at all, I think downgrading the ASSERT to
an error message is a good idea short-term.

Daniil - could you make that change please?

/
 Leif


I've been able to reproduce "PCIe CSR read failed to respond" and "SMBus
transaction not claimed" errors on my Juno R2. I disabled Marvell Yukon
driver (.dsc/.fdf) and removed ArmJunoDxe patch but still see the same error
messages during the initial boot.

Testing motherboard interfaces (FPGA build 118)...
SRAM 32MB test: PASSED
LAN9118   test: PASSED
KMI1/2test: PASSED
MMC   test: PASSED
PB/LEDs   test: PASSED
FPGA UART test: PASSED
ERROR: PCIe CSR read failed to respond
ERROR: SMBus transaction not claimed
ERROR: PCIe CSR read failed to respond
...

Once it went through reporting these errors, the UEFI starts loading but
still fails in OnEndOfDxe():
ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [ArmJunoDxe]
/home/user/workspace/juno/uefi/edk2/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c(110):
!EFI_ERROR (Status)

This is the original ArmJunoDxe code:
   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath,
FALSE);
   ASSERT_EFI_ERROR (Status); < line 110

I'm confused at which point in the commit history this is line 110.

Just before your MAC commit, it's line 108. After your commit, and in
the current mainline, it's line 394.

Then, I have to go back to Jeremy's commit
58a4bff071832e587d18f97597b5d571dcebc9d7 

Re: [edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event

[Zhang, Chao B]

Jiewen:
   Actually I tried item 2 in PEI & produce hob for DXE to consume. But basing 
on my test
No TPM we have can support such feature defined in PTP00.47. PCR[0] after 
Startup(CLEAR) always
shows zero.
  I will choose option 1 first.

From: Yao, Jiewen
Sent: Monday, January 23, 2017 11:25 PM
To: Zhang, Chao B ; edk2-devel@lists.01.org
Cc: Zhang, Chao B ; Zeng, Star 
Subject: RE: [edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event

Hi
StartupLocality is a platform policy. We should not hardcode it.

We may use one of below ways:
1) Define a new PCD.
2) Detect if there is an startuplocality event hob reported in PEI phase.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang,
> Chao B
> Sent: Monday, January 23, 2017 4:52 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen >; Zhang, 
> Chao B
> >; Zeng, Star 
> >
> Subject: [edk2] [PATCH 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event
>
> Log Startup Locality Event according to TCG PC Client PFP 00.21.
> Event should be placed before any extend to PCR[0]
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
>
> Cc: Star Zeng >
> Cc: Yao Jiewen >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> >
> ---
>  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 48
> ++-
>  1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index 3534fd1..2658944 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -1381,11 +1381,12 @@ SetupEventLog (
>UINT32  HashAlgorithmMaskCopied;
>TCG_EfiSpecIDEventStruct*TcgEfiSpecIdEventStruct;
>UINT8
> TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + sizeof(UINT32) + (HASH_COUNT *
> sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + sizeof(UINT8)];
> -  TCG_PCR_EVENT_HDR   FirstPcrEvent;
> +  TCG_PCR_EVENT_HDR   NoActionEvent;
>TCG_EfiSpecIdEventAlgorithmSize *DigestSize;
>TCG_EfiSpecIdEventAlgorithmSize *TempDigestSize;
>UINT8   *VendorInfoSize;
>UINT32  NumberOfAlgorithms;
> +  TCG_EfiStartupLocalityEvent StartupLocalityEvent;
>
>DEBUG ((EFI_D_INFO, "SetupEventLog\n"));
>
> @@ -1468,24 +1469,49 @@ SetupEventLog (
>  VendorInfoSize = (UINT8 *)TempDigestSize;
>  *VendorInfoSize = 0;
>
> +NoActionEvent.PCRIndex = 0;
> +NoActionEvent.EventType = EV_NO_ACTION;
> +ZeroMem (, sizeof(NoActionEvent.Digest));
> +NoActionEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize
> (TcgEfiSpecIdEventStruct);
> +
>  //
> -// FirstPcrEvent
> +// Log TcgEfiSpecIdEventStruct as the first Event
> +//   TCG PC Client PFP spec. Section 9.2 Measurement Event Entries
> and Log
>  //
> -FirstPcrEvent.PCRIndex = 0;
> -FirstPcrEvent.EventType = EV_NO_ACTION;
> -ZeroMem (, sizeof(FirstPcrEvent.Digest));
> -FirstPcrEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize
> (TcgEfiSpecIdEventStruct);
> +Status = TcgDxeLogEvent (
> +   mTcg2EventInfo[Index].LogFormat,
> +   ,
> +   sizeof(NoActionEvent),
> +   (UINT8 *)TcgEfiSpecIdEventStruct,
> +   NoActionEvent.EventSize
> +   );
>
>  //
> -// Record
> +// EfiStartupLocalityEvent
> +//
> +CopyMem (StartupLocalityEvent.Signature,
> TCG_EfiStartupLocalityEvent_SIGNATURE,
> sizeof(StartupLocalityEvent.Signature));
> +//
> +// SRTM uses Locality 0 to access the TPM according to PC Client PFP
> spec 2.2.1
> +//
> +StartupLocalityEvent.StartupLocality = LOCALITY_0_INDICATOR;
> +
> +NoActionEvent.PCRIndex = 0;
> +NoActionEvent.EventType = EV_NO_ACTION;
> +ZeroMem (, sizeof(NoActionEvent.Digest));
> +NoActionEvent.EventSize = sizeof(StartupLocalityEvent);
> +
> +//
> +// Log EfiStartupLocalityEvent as the second Event
> +//   TCG PC Client PFP spec. Section 9.3.4.3 Startup Locality Event
>  //
>  Status = TcgDxeLogEvent (
> mTcg2EventInfo[Index].LogFormat,
> -   ,
> -   sizeof(FirstPcrEvent),
> -   (UINT8 

Re: [edk2] [PATCH V2 2/2] MdePkg : UefiTcgPlatform.h: Define Startup Locality Event & Indicator

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zhang, Chao B
> Sent: Tuesday, January 24, 2017 9:54 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star ;
> Zhang, Chao B 
> Subject: [PATCH V2 2/2] MdePkg : UefiTcgPlatform.h: Define Startup Locality
> Event & Indicator
> 
> Add Startup Locality Event definition according to PC Client PFP 00.21
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
> Add Locality Indicator definition according to PC Client PTP 00.43
> https://www.trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specifi
> c-Platform-TPM-Profile-for-TPM-2-0-v43-150126.pdf
> 
> Cc: Star Zeng 
> Cc: Yao Jiewen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 29
> ++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
> b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
> index 23eaa53..6ce808e 100644
> --- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
> +++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
> @@ -1,7 +1,7 @@
>  /** @file
>TCG EFI Platform Definition in TCG_EFI_Platform_1_20_Final
> 
> -  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2006 - 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
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -268,6 +268,33 @@ typedef struct {
>  //UINT8   vendorInfo[vendorInfoSize];
>  } TCG_EfiSpecIDEventStruct;
> 
> +
> +
> +#define TCG_EfiStartupLocalityEvent_SIGNATURE  "StartupLocality"
> +
> +
> +//
> +// PC Client PTP spec Table 8 Relationship between Locality and Locality
> Attribute
> +//
> +#define LOCALITY_0_INDICATOR0x01
> +#define LOCALITY_1_INDICATOR0x02
> +#define LOCALITY_2_INDICATOR0x03
> +#define LOCALITY_3_INDICATOR0x04
> +#define LOCALITY_4_INDICATOR0x05
> +
> +
> +//
> +// Startup Locality Event
> +//
> +typedef struct tdTCG_EfiStartupLocalityEvent{
> +  UINT8   Signature[16];
> +  //
> +  // The Locality Indicator which sent the TPM2_Startup command
> +  //
> +  UINT8   StartupLocality;
> +} TCG_EfiStartupLocalityEvent;
> +
> +
>  //
>  // Restore original structure alignment
>  //
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [PATCH V2 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zhang, Chao B
> Sent: Tuesday, January 24, 2017 9:54 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star ;
> Zhang, Chao B 
> Subject: [PATCH V2 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event
> 
> Log Startup Locality Event according to TCG PC Client PFP 00.21.
> Event should be placed before any extend to PCR[0]
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
> 
> Cc: Star Zeng 
> Cc: Yao Jiewen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  SecurityPkg/Include/Guid/TcgEventHob.h | 12 +++-
>  SecurityPkg/SecurityPkg.dec|  4 +++
>  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c  | 54
> ++
>  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf|  3 +-
>  4 files changed, 59 insertions(+), 14 deletions(-)
> 
> diff --git a/SecurityPkg/Include/Guid/TcgEventHob.h
> b/SecurityPkg/Include/Guid/TcgEventHob.h
> index 1082807..8be5cd0 100644
> --- a/SecurityPkg/Include/Guid/TcgEventHob.h
> +++ b/SecurityPkg/Include/Guid/TcgEventHob.h
> @@ -3,7 +3,7 @@
>a TPM DXE Driver. A GUIDed HOB is generated for each measurement
>made in the PEI Phase.
> 
> -Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2007 - 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
>  which accompanies this distribution.  The full text of the license may be 
> found
> at
> @@ -44,4 +44,14 @@ extern EFI_GUID gTcgEvent2EntryHobGuid;
> 
>  extern EFI_GUID gTpmErrorHobGuid;
> 
> +///
> +/// The Global ID of a GUIDed HOB used to record TPM2 Startup Locality.
> +///
> +#define EFI_TPM2_STARTUP_LOCALITY_HOB_GUID \
> +  { \
> +0xef598499, 0xb25e, 0x473a, { 0xbf, 0xaf, 0xe7, 0xe5, 0x7d, 0xce, 0x82, 
> 0xc4 }
> \
> +  }
> +
> +extern EFI_GUID gTpm2StartupLocalityHobGuid;
> +
>  #endif
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 0c64d25..b556fb6 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -134,6 +134,10 @@
>#  Include/Guid/TcgEventHob.h
>gTpmErrorHobGuid   = { 0xef598499, 0xb25e, 0x473a,
> { 0xbf, 0xaf, 0xe7, 0xe5, 0x7d, 0xce, 0x82, 0xc4 }}
> 
> +  ## HOB GUID used to record TPM2 startup locality
> +  ## Include/Guid/TcgEventHob.h
> +  gTpm2StartupLocalityHobGuid= { 0x397b0c9, 0x22e8, 0x459e, { 0xa4,
> 0xff, 0x99, 0xbc, 0x65, 0x27, 0x9, 0x29 }}
> +
>## HOB GUID used to pass all PEI measured FV info to DXE Driver.
>#  Include/Guid/MeasuredFvHob.h
>gMeasuredFvHobGuid = { 0xb2360b42, 0x7173, 0x420a,
> { 0x86, 0x96, 0x46, 0xca, 0x6b, 0xab, 0x10, 0x60 }}
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index 3534fd1..99e2c48 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -1381,11 +1381,12 @@ SetupEventLog (
>UINT32  HashAlgorithmMaskCopied;
>TCG_EfiSpecIDEventStruct*TcgEfiSpecIdEventStruct;
>UINT8
> TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + sizeof(UINT32) + (HASH_COUNT *
> sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + sizeof(UINT8)];
> -  TCG_PCR_EVENT_HDR   FirstPcrEvent;
> +  TCG_PCR_EVENT_HDR   NoActionEvent;
>TCG_EfiSpecIdEventAlgorithmSize *DigestSize;
>TCG_EfiSpecIdEventAlgorithmSize *TempDigestSize;
>UINT8   *VendorInfoSize;
>UINT32  NumberOfAlgorithms;
> +  TCG_EfiStartupLocalityEvent StartupLocalityEvent;
> 
>DEBUG ((EFI_D_INFO, "SetupEventLog\n"));
> 
> @@ -1468,24 +1469,53 @@ SetupEventLog (
>  VendorInfoSize = (UINT8 *)TempDigestSize;
>  *VendorInfoSize = 0;
> 
> -//
> -// FirstPcrEvent
> -//
> -FirstPcrEvent.PCRIndex = 0;
> -FirstPcrEvent.EventType = EV_NO_ACTION;
> -ZeroMem (, sizeof(FirstPcrEvent.Digest));
> -FirstPcrEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize
> (TcgEfiSpecIdEventStruct);
> +NoActionEvent.PCRIndex = 0;
> +NoActionEvent.EventType = EV_NO_ACTION;
> +ZeroMem (, sizeof(NoActionEvent.Digest));
> +NoActionEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize
> (TcgEfiSpecIdEventStruct);
> 
>  //
> -// Record
> +// Log TcgEfiSpecIdEventStruct as the first Event
> +//   TCG PC Client PFP spec. Section 9.2 Measurement Event Entries
> and Log
>  //
>  Status = TcgDxeLogEvent (
> mTcg2EventInfo[Index].LogFormat,
> -   ,
> -   

[edk2] [PATCH v2] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Fixed crash on Juno R0

[Daniil Egranov]

The Marvell Yukon MAC address load supported only on Juno R1 and R2.
It disabled for Juno R0 due to PCI issues on this board.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
Changelog:

v2
Replaced ASSERT with the error message in case Marvell MAC address
set has failed

 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index 47ff587..0193b98 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -378,6 +378,7 @@ OnEndOfDxe (
   EFI_DEVICE_PATH_PROTOCOL* PciRootComplexDevicePath;
   EFI_HANDLEHandle;
   EFI_STATUSStatus;
+  UINT32JunoRevision;
 
   //
   // PCI Root Complex initialization
@@ -393,8 +394,14 @@ OnEndOfDxe (
   Status = gBS->ConnectController (Handle, NULL, PciRootComplexDevicePath, 
FALSE);
   ASSERT_EFI_ERROR (Status);
 
-  Status = ArmJunoSetNicMacAddress ();
-  ASSERT_EFI_ERROR (Status);
+  GetJunoRevision (JunoRevision);
+
+  if (JunoRevision != JUNO_REVISION_R0) {
+Status = ArmJunoSetNicMacAddress ();
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "ArmJunoDxe: Failed to set Marvell Yukon NIC MAC 
address\n"));
+}
+  }
 }
 
 STATIC
-- 
2.7.4

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

[edk2] [PATCH V2 1/2] SecurityPkg: Tcg2Dxe: Log Startup Locality Event

[Zhang, Chao B]

Log Startup Locality Event according to TCG PC Client PFP 00.21.
Event should be placed before any extend to PCR[0]
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf

Cc: Star Zeng 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Include/Guid/TcgEventHob.h | 12 +++-
 SecurityPkg/SecurityPkg.dec|  4 +++
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c  | 54 ++
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf|  3 +-
 4 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/SecurityPkg/Include/Guid/TcgEventHob.h 
b/SecurityPkg/Include/Guid/TcgEventHob.h
index 1082807..8be5cd0 100644
--- a/SecurityPkg/Include/Guid/TcgEventHob.h
+++ b/SecurityPkg/Include/Guid/TcgEventHob.h
@@ -3,7 +3,7 @@
   a TPM DXE Driver. A GUIDed HOB is generated for each measurement 
   made in the PEI Phase.
 
-Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 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 
 which accompanies this distribution.  The full text of the license may be 
found at 
@@ -44,4 +44,14 @@ extern EFI_GUID gTcgEvent2EntryHobGuid;
 
 extern EFI_GUID gTpmErrorHobGuid;
 
+///
+/// The Global ID of a GUIDed HOB used to record TPM2 Startup Locality.
+///
+#define EFI_TPM2_STARTUP_LOCALITY_HOB_GUID \
+  { \
+0xef598499, 0xb25e, 0x473a, { 0xbf, 0xaf, 0xe7, 0xe5, 0x7d, 0xce, 0x82, 
0xc4 } \
+  }
+
+extern EFI_GUID gTpm2StartupLocalityHobGuid;
+
 #endif
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 0c64d25..b556fb6 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -134,6 +134,10 @@
   #  Include/Guid/TcgEventHob.h
   gTpmErrorHobGuid   = { 0xef598499, 0xb25e, 0x473a, { 0xbf, 
0xaf, 0xe7, 0xe5, 0x7d, 0xce, 0x82, 0xc4 }}
 
+  ## HOB GUID used to record TPM2 startup locality
+  ## Include/Guid/TcgEventHob.h
+  gTpm2StartupLocalityHobGuid= { 0x397b0c9, 0x22e8, 0x459e, { 0xa4, 
0xff, 0x99, 0xbc, 0x65, 0x27, 0x9, 0x29 }}
+
   ## HOB GUID used to pass all PEI measured FV info to DXE Driver.
   #  Include/Guid/MeasuredFvHob.h
   gMeasuredFvHobGuid = { 0xb2360b42, 0x7173, 0x420a, { 0x86, 
0x96, 0x46, 0xca, 0x6b, 0xab, 0x10, 0x60 }}
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c 
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 3534fd1..99e2c48 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -1381,11 +1381,12 @@ SetupEventLog (
   UINT32  HashAlgorithmMaskCopied;
   TCG_EfiSpecIDEventStruct*TcgEfiSpecIdEventStruct;
   UINT8   TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + 
sizeof(UINT32) + (HASH_COUNT * sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + 
sizeof(UINT8)];
-  TCG_PCR_EVENT_HDR   FirstPcrEvent;
+  TCG_PCR_EVENT_HDR   NoActionEvent;
   TCG_EfiSpecIdEventAlgorithmSize *DigestSize;
   TCG_EfiSpecIdEventAlgorithmSize *TempDigestSize;
   UINT8   *VendorInfoSize;
   UINT32  NumberOfAlgorithms;
+  TCG_EfiStartupLocalityEvent StartupLocalityEvent;
 
   DEBUG ((EFI_D_INFO, "SetupEventLog\n"));
 
@@ -1468,24 +1469,53 @@ SetupEventLog (
 VendorInfoSize = (UINT8 *)TempDigestSize;
 *VendorInfoSize = 0;
 
-//
-// FirstPcrEvent
-//
-FirstPcrEvent.PCRIndex = 0;
-FirstPcrEvent.EventType = EV_NO_ACTION;
-ZeroMem (, sizeof(FirstPcrEvent.Digest));
-FirstPcrEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize 
(TcgEfiSpecIdEventStruct);
+NoActionEvent.PCRIndex = 0;
+NoActionEvent.EventType = EV_NO_ACTION;
+ZeroMem (, sizeof(NoActionEvent.Digest));
+NoActionEvent.EventSize = (UINT32)GetTcgEfiSpecIdEventStructSize 
(TcgEfiSpecIdEventStruct);
 
 //
-// Record
+// Log TcgEfiSpecIdEventStruct as the first Event
+//   TCG PC Client PFP spec. Section 9.2 Measurement Event Entries and 
Log
 //
 Status = TcgDxeLogEvent (
mTcg2EventInfo[Index].LogFormat,
-   ,
-   sizeof(FirstPcrEvent),
+   ,
+   sizeof(NoActionEvent),
(UINT8 *)TcgEfiSpecIdEventStruct,
-   FirstPcrEvent.EventSize
+   NoActionEvent.EventSize
);
+
+//
+// EfiStartupLocalityEvent
+//
+GuidHob.Guid = GetFirstGuidHob ();
+if (GuidHob.Guid != NULL) {
+  //
+  // Get Locality Indicator from StartupLocality HOB
+  //
+  StartupLocalityEvent.StartupLocality = 

[edk2] [PATCH V2 2/2] MdePkg : UefiTcgPlatform.h: Define Startup Locality Event & Indicator

[Zhang, Chao B]

Add Startup Locality Event definition according to PC Client PFP 00.21
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Add Locality Indicator definition according to PC Client PTP 00.43
https://www.trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2-0-v43-150126.pdf

Cc: Star Zeng 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 29 ++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h 
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 23eaa53..6ce808e 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -1,7 +1,7 @@
 /** @file
   TCG EFI Platform Definition in TCG_EFI_Platform_1_20_Final
 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -268,6 +268,33 @@ typedef struct {
 //UINT8   vendorInfo[vendorInfoSize];
 } TCG_EfiSpecIDEventStruct;
 
+
+
+#define TCG_EfiStartupLocalityEvent_SIGNATURE  "StartupLocality"
+
+
+//
+// PC Client PTP spec Table 8 Relationship between Locality and Locality 
Attribute
+//
+#define LOCALITY_0_INDICATOR0x01
+#define LOCALITY_1_INDICATOR0x02
+#define LOCALITY_2_INDICATOR0x03
+#define LOCALITY_3_INDICATOR0x04
+#define LOCALITY_4_INDICATOR0x05
+
+
+//
+// Startup Locality Event
+//
+typedef struct tdTCG_EfiStartupLocalityEvent{
+  UINT8   Signature[16];
+  //
+  // The Locality Indicator which sent the TPM2_Startup command
+  //
+  UINT8   StartupLocality;
+} TCG_EfiStartupLocalityEvent;
+
+
 //
 // Restore original structure alignment
 //
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Tim Lewis]

Also, in some cases, the text is taken directly from the specification. 
Introducing HII strings in order to make these translatable when the source 
material is normative doesn't help, IMO.

Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Carsey, 
Jaben
Sent: Monday, January 23, 2017 9:41 AM
To: Ni, Ruiyu ; Zeng, Star ; 
edk2-devel@lists.01.org
Cc: Carsey, Jaben 
Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
spec 3.1.1

Reviewed-by: Jaben Carsey 

I think that string mixed use existed in the EDK version of the command and was 
just never removed.

-Jaben

> -Original Message-
> From: Ni, Ruiyu
> Sent: Sunday, January 22, 2017 1:49 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> spec 3.1.1
> Importance: High
> 
> Reviewed-by: Ruiyu Ni 
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Sunday, January 22, 2017 5:25 PM
> > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Zeng, Star 
> > 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> > spec 3.1.1
> >
> > Ray & Jaben,
> >
> > I am not so sure about the rule.
> > The mixed using of strings in c code and strings in uni file has 
> > been there since it was created.
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Sunday, January 22, 2017 4:56 PM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> > spec 3.1.1
> >
> > Star,
> > Why some strings are hardcoded in C file while some are defined in 
> > UNI
> file?
> > What's the rule?
> >
> > Thanks/Ray
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Sunday, January 22, 2017 4:18 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Zeng, Star ; Ni, Ruiyu 
> > > ; Carsey, Jaben 
> > > Subject: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> spec
> > > 3.1.1
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
> > >
> > > Cc: Ruiyu Ni 
> > > Cc: Jaben Carsey 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Star Zeng 
> > > ---
> > >  .../SmbiosView/PrintInfo.c |  6 +++-
> > >  .../SmbiosView/QueryTable.c| 37
> ++
> > >  .../SmbiosView/QueryTable.h| 14 +++-
> > >  .../SmbiosView/SmbiosViewStrings.uni   |  3 +-
> > >  4 files changed, 57 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > index ecb8e2492453..1d6002b92593 100644
> > > ---
> > >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > +++
> > >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > @@ -1106,7 +1106,7 @@ SmbiosPrintStructure (
> > >// Management Controller Host Interface (Type 42)
> > >//
> > >case 42:
> > > -PRINT_STRUCT_VALUE_H (Struct, Type42, InterfaceType);
> > > +DisplayMCHostInterfaceType (Struct->Type42->InterfaceType, 
> > > + Option);
> > >  break;
> > >
> > >//
> > > @@ -1818,6 +1818,10 @@ DisplayProcessorFamily (
> > >  Print (L"AMD Opteron(TM) X3000 Series APU\n");
> > >  break;
> > >
> > > +  case 0x6B:
> > > +Print (L"AMD Zen Processor Family\n");
> > > +break;
> > > +
> > >case 0x70:
> > >  ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN
> > > (STR_SMBIOSVIEW_PRINTINFO_HOBBIT_FAMILY),
> gShellDebug1HiiHandle);
> > >  break;
> > > diff --git
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > c
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > c
> > > index 4a06c12e3b2b..282ba584c8c9 100644
> > > ---
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > c
> > > +++
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > +++ c
> > > @@ -575,6 +575,10 @@ TABLE_ITEM  ProcessorUpgradeTable[] = {
> > >{
> > >  0x37,
> > >  L"Socket SP3"
> > > +  },
> > > +  {
> > > +0x38,
> > > +L"Socket SP3r2"
> > >}
> > >  };
> > >
> > > @@ -3156,6 +3160,22 @@ TABLE_ITEM  IPMIDIBMCInterfaceTypeTable[]
> =
> > {
> > >},
> > >  };
> > >
> > > +TABLE_ITEM  MCHostInterfaceTypeTable[] 

[edk2] [PATCH 1/6] MdeModulePkg: Refine type cast for pointer subtraction

[Hao Wu]

For pointer subtraction, the result is of type "ptrdiff_t". According to
the C11 spec, ptrdiff_t is a signed integer type but its size is
implementation-defined.

There are cases that the result of pointer subtraction is type casted to
UINTN, like:

UINT8  *Ptr1, *Ptr2;
UINTN  PtrDiff;
...
PtrDiff = (UINTN) (Ptr1 - Ptr2);

Some static code checkers may warn that the pointer subtraction might
overflow first and then the result is being cast to a bigger size.

The commit will cast each pointer to UINTN first and then perform the
subtraction:

PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2;

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  4 ++--
 MdeModulePkg/Include/Library/NetLib.h  |  6 +++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  |  4 ++--
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c  |  2 +-
 MdeModulePkg/Library/FileExplorerLib/FileExplorer.c|  6 +++---
 MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c |  4 ++--
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c |  4 ++--
 MdeModulePkg/Universal/DebugPortDxe/DebugPort.c|  4 ++--
 .../Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c   |  4 ++--
 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c  |  4 ++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c  | 10 +-
 11 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index 2bc4f8c..d2ad94e 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -1,7 +1,7 @@
 /** @file
   PCI Rom supporting funtions implementation for PCI Bus module.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -776,7 +776,7 @@ ProcessOpRomImage (
 NextImage:
 RomBarOffset += ImageSize;
 
-  } while (((Indicator & 0x80) == 0x00) && ((UINTN) (RomBarOffset - (UINT8 *) 
RomBar) < PciDevice->RomSize));
+  } while (((Indicator & 0x80) == 0x00) && (((UINTN) RomBarOffset - (UINTN) 
RomBar) < PciDevice->RomSize));
 
   return RetStatus;
 }
diff --git a/MdeModulePkg/Include/Library/NetLib.h 
b/MdeModulePkg/Include/Library/NetLib.h
index 09ead09..3b8ff1a 100644
--- a/MdeModulePkg/Include/Library/NetLib.h
+++ b/MdeModulePkg/Include/Library/NetLib.h
@@ -2,7 +2,7 @@
   This library is only intended to be used by UEFI network stack modules.
   It provides basic functions for the UEFI network stack.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -1610,10 +1610,10 @@ typedef struct {
   (sizeof (NET_BUF) + ((BlockOpNum) - 1) * sizeof (NET_BLOCK_OP))
 
 #define NET_HEADSPACE(BlockOp)  \
-  (UINTN)((BlockOp)->Head - (BlockOp)->BlockHead)
+  ((UINTN)((BlockOp)->Head) - (UINTN)((BlockOp)->BlockHead))
 
 #define NET_TAILSPACE(BlockOp)  \
-  (UINTN)((BlockOp)->BlockTail - (BlockOp)->Tail)
+  ((UINTN)((BlockOp)->BlockTail) - (UINTN)((BlockOp)->Tail))
 
 /**
   Allocate a single block NET_BUF. Upon allocation, all the
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 71e05bd..d7abcc8 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -10,7 +10,7 @@
   ValidateFmpCapsule(), DisplayCapsuleImage(), ConvertBmpToGopBlt() will
   receive untrusted input and do basic validation.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -547,7 +547,7 @@ ConvertBmpToGopBlt (
 
 }
 
-ImageIndex = (UINTN) (Image - ImageHeader);
+ImageIndex = (UINTN) Image - (UINTN) ImageHeader;
 if ((ImageIndex % 4) != 0) {
   //
   // Bmp Image starts each row on a 32-bit boundary!
diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c 
b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
index 0137868..7fda260 100644

[edk2] [PATCH 0/6] Refine type cast for pointer subtraction

[Hao Wu]

Please note that this patch is maily for feedback collection and the only
the MdeModulePkg part of the series is sent out firstly.


For pointer subtraction, the result is of type "ptrdiff_t". According to
the C11 spec, ptrdiff_t is a signed integer type but its size is
implementation-defined.

There are cases that the result of pointer subtraction is type casted to
UINTN, like:

UINT8  *Ptr1, *Ptr2;
UINTN  PtrDiff;
...
PtrDiff = (UINTN) (Ptr1 - Ptr2);

Some static code checkers may warn that the pointer subtraction might
overflow first and then the result is being cast to a bigger size.

The series will cast each pointer to UINTN first and then perform the
subtraction:

PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2;


Hao Wu (6):
  MdeModulePkg: Refine type cast for pointer subtraction
  CryptoPkg: Refine type cast for pointer subtraction
  IntelFrameworkModulePkg: Refine type cast for pointer subtraction
  NetworkPkg: Refine type cast for pointer subtraction
  SecurityPkg: Refine type cast for pointer subtraction
  ShellPkg: Refine type cast for pointer subtraction

 .../BaseCryptLib/SysCall/RuntimeMemAllocation.c|  6 +++---
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c  |  4 ++--
 .../Library/GenericBdsLib/BdsBoot.c|  4 ++--
 .../Library/GenericBdsLib/BdsConsole.c |  4 ++--
 .../Library/GenericBdsLib/BdsMisc.c|  4 ++--
 .../Library/LegacyBootManagerLib/LegacyBm.c|  4 ++--
 .../Universal/BdsDxe/BootMaint/BootOption.c|  6 +++---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  4 ++--
 MdeModulePkg/Include/Library/NetLib.h  |  6 +++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  |  4 ++--
 .../Library/DxePrintLibPrint2Protocol/PrintLib.c   |  2 +-
 MdeModulePkg/Library/FileExplorerLib/FileExplorer.c|  6 +++---
 .../Library/PiDxeS3BootScriptLib/BootScriptSave.c  |  4 ++--
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c |  4 ++--
 MdeModulePkg/Universal/DebugPortDxe/DebugPort.c|  4 ++--
 .../FaultTolerantWriteDxe/FaultTolerantWrite.c |  4 ++--
 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c  |  4 ++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c  | 10 +-
 NetworkPkg/HttpDxe/HttpImpl.c  |  6 +++---
 .../DxeImageVerificationLib/DxeImageVerificationLib.c  | 18 +-
 .../DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c| 10 +-
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c  |  6 +++---
 SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c| 10 +-
 SecurityPkg/Tcg/TrEEDxe/MeasureBootPeCoff.c| 10 +-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 +++---
 ShellPkg/Application/Shell/ShellParametersProtocol.c   |  6 +++---
 ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c |  4 ++--
 .../UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c |  4 ++--
 ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c  |  4 ++--
 ShellPkg/Library/UefiShellLevel2CommandsLib/Cd.c   |  6 +++---
 30 files changed, 91 insertions(+), 91 deletions(-)

-- 
1.9.5.msysgit.0

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

Re: [edk2] [PATCH 1/3] MdePkg: Add definitions for SMBIOS spec 3.1.1

[Zeng, Star]

Yes, good catch.

Thanks,
Star
-Original Message-
From: Gao, Liming 
Sent: Tuesday, January 24, 2017 3:25 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Kinney, Michael D 
Subject: RE: [PATCH 1/3] MdePkg: Add definitions for SMBIOS spec 3.1.1

Star:
  Please fix the incorrect character in comments. With this change, I add my 
Reviewed-by: Liming Gao 

+/// 00h  C 3Fh: MCTP Host Interfaces

Thanks
Liming
>-Original Message-
>From: Zeng, Star
>Sent: Sunday, January 22, 2017 4:18 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star ; Gao, Liming 
>; Kinney, Michael D 
>Subject: [PATCH 1/3] MdePkg: Add definitions for SMBIOS spec 3.1.1
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
>
>This patch is to add definitions for below items.
>Processor Information (Type 4):
>- add socket SP3r2
>- add AMD Zen Processor Family
>Management Controller Host Interface (Type 42):
>- include Host Interface Type and Protocol Identifier enumerations
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng 
>---
> MdePkg/Include/IndustryStandard/SmBios.h | 25
>+++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Include/IndustryStandard/SmBios.h
>b/MdePkg/Include/IndustryStandard/SmBios.h
>index f5c29bcbd0c8..6a8fa1b39d17 100644
>--- a/MdePkg/Include/IndustryStandard/SmBios.h
>+++ b/MdePkg/Include/IndustryStandard/SmBios.h
>@@ -611,6 +611,7 @@ typedef enum {
>   ProcessorFamilyAmdOpteronX2000Series  = 0x68,
>   ProcessorFamilyAmdOpteronASeries  = 0x69,
>   ProcessorFamilyAmdOpteronX3000Series  = 0x6A,
>+  ProcessorFamilyAmdZen = 0x6B,
>   ProcessorFamilyHobbit = 0x70,
>   ProcessorFamilyCrusoeTM5000   = 0x78,
>   ProcessorFamilyCrusoeTM3000   = 0x79,
>@@ -804,7 +805,8 @@ typedef enum {
>   ProcessorUpgradeSocketBGA1440   = 0x34,
>   ProcessorUpgradeSocketBGA1515   = 0x35,
>   ProcessorUpgradeSocketLGA3647_1 = 0x36,
>-  ProcessorUpgradeSocketSP3   = 0x37
>+  ProcessorUpgradeSocketSP3   = 0x37,
>+  ProcessorUpgradeSocketSP3r2 = 0x38
> } PROCESSOR_UPGRADE;
>
> ///
>@@ -2386,6 +2388,25 @@ typedef struct {  } SMBIOS_TABLE_TYPE41;
>
> ///
>+/// Management Controller Host Interface - Interface Types.
>+/// 00h  C 3Fh: MCTP Host Interfaces
>+///
>+typedef enum{
>+  MCHostInterfaceTypeNetworkHostInterface   = 0x40,
>+  MCHostInterfaceTypeOemDefined = 0xF0
>+} MC_HOST_INTERFACE_TYPE;
>+
>+///
>+/// Management Controller Host Interface - Protocol Types.
>+///
>+typedef enum{
>+  MCHostInterfaceProtocolTypeIPMI   = 0x02,
>+  MCHostInterfaceProtocolTypeMCTP   = 0x03,
>+  MCHostInterfaceProtocolTypeRedfishOverIP  = 0x04,
>+  MCHostInterfaceProtocolTypeOemDefined = 0xF0
>+} MC_HOST_INTERFACE_PROTOCOL_TYPE;
>+
>+///
> /// Management Controller Host Interface (Type 42).
> ///
> /// The information in this structure defines the attributes of a 
>Management @@ -2404,7 +2425,7 @@ typedef struct {  ///  typedef struct 
>{
>   SMBIOS_STRUCTURE  Hdr;
>-  UINT8 InterfaceType;
>+  UINT8 InterfaceType;  ///< The 
>enumeration value from
>MC_HOST_INTERFACE_TYPE
>   UINT8 MCHostInterfaceData[1]; ///< This field 
> has a minimum
>of four bytes
> } SMBIOS_TABLE_TYPE42;
>
>--
>2.7.0.windows.1

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

[edk2] [PATCH v2 1/1] MdePkg: Refine casting expression result to bigger size

[Hao Wu]

There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly casted to
UINT64/INT64 to fit the target size.

An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);

Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.

The commit refines codes by the following rules:
1). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;

2). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdePkg/Library/BaseLib/String.c  |  4 ++--
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c| 12 +---
 MdePkg/Library/BaseS3PciLib/S3PciLib.c   |  4 ++--
 MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c  |  4 ++--
 MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c |  4 ++--
 5 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
index e84bf50..4151e0e 100644
--- a/MdePkg/Library/BaseLib/String.c
+++ b/MdePkg/Library/BaseLib/String.c
@@ -586,7 +586,7 @@ InternalHexCharToUintn (
 return Char - L'0';
   }
 
-  return (UINTN) (10 + InternalCharToUpper (Char) - L'A');
+  return (10 + InternalCharToUpper (Char) - L'A');
 }
 
 /**
@@ -1211,7 +1211,7 @@ InternalAsciiHexCharToUintn (
 return Char - '0';
   }
 
-  return (UINTN) (10 + InternalBaseLibAsciiToUpper (Char) - 'A');
+  return (10 + InternalBaseLibAsciiToUpper (Char) - 'A');
 }
 
 
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 33cad23..8d1daba 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -15,7 +15,7 @@
   PeCoffLoaderGetPeHeader() routine will do basic check for PE/COFF header.
   PeCoffLoaderGetImageInfo() routine will do basic check for whole PE/COFF 
image.
 
-  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
   Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -703,12 +703,10 @@ PeCoffLoaderGetImageInfo (
   //
   DebugDirectoryEntryFileOffset = 0;
 
-  SectionHeaderOffset = (UINTN)(
-   ImageContext->PeCoffHeaderOffset +
-   sizeof (UINT32) +
-   sizeof (EFI_IMAGE_FILE_HEADER) +
-   Hdr.Pe32->FileHeader.SizeOfOptionalHeader
-   );
+  SectionHeaderOffset = ImageContext->PeCoffHeaderOffset +
+sizeof (UINT32) +
+sizeof (EFI_IMAGE_FILE_HEADER) +
+Hdr.Pe32->FileHeader.SizeOfOptionalHeader;
 
   for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
 //
diff --git a/MdePkg/Library/BaseS3PciLib/S3PciLib.c 
b/MdePkg/Library/BaseS3PciLib/S3PciLib.c
index e29f7fe..27342b0 100644
--- a/MdePkg/Library/BaseS3PciLib/S3PciLib.c
+++ b/MdePkg/Library/BaseS3PciLib/S3PciLib.c
@@ -3,7 +3,7 @@
   the PCI operations to be replayed during an S3 resume. This library class
   maps directly on top of the PciLib class. 
 
-  Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions
@@ -25,7 +25,7 @@
 #include 
 
 #define PCILIB_TO_COMMON_ADDRESS(Address) \
-((UINT64) UINTN) ((Address>>20) & 0xff)) << 24) + (((UINTN) 
((Address>>15) & 0x1f)) << 16) + (((UINTN) ((Address>>12) & 0x07)) << 8) + 
((UINTN) (Address & 0xfff 
+UINTN) ((Address>>20) & 0xff)) << 24) + (((UINTN) ((Address>>15) & 
0x1f)) << 16) + (((UINTN) ((Address>>12) & 0x07)) << 8) + ((UINTN) (Address & 
0xfff )))
 
 /**
   Saves a PCI configuration value to the boot script.
diff --git a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c 
b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
index 937165a..592cced 100644
--- a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
+++ b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
@@ -12,7 +12,7 @@
   allocation for the Reserved memory types are not supported and will always 
   return NULL.
 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights 

[edk2] [PATCH v2 0/1] Refine casting expression result to bigger size

[Hao Wu]

Please note that this patch is maily for feedback collection and the patch
only covers MdePkg. We are working on patches for other packages.


V2:
Follow the below rules to refine codes:
1). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;

2). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;

V1:
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is casted to
UINT64/INT64 to fit the target size.

An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);

Some static code checkers may warn that the expression result might
overflow within the rank of int (integer promotions) and the result is
then cast to a bigger size.

For the consideration of generated binaries size, the commit will keep the
size of the operands as the size of int, and explitly add a type cast
before converting the result to UINT64/INT64.

1). When there is no operand with type UINTN
(UINTN)  (a + b) -> (UINTN)(UINT32)  (a + b) or
(UINT64) (a + b) -> (UINT64)(UINT32) (a + b)

2). Otherwise
(UINT64) (a + b) -> (UINT64)(UINTN)  (a + b)

Cc: Michael Kinney 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 

Hao Wu (1):
  MdePkg: Refine casting expression result to bigger size

 MdePkg/Library/BaseLib/String.c  |  4 ++--
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c| 12 +---
 MdePkg/Library/BaseS3PciLib/S3PciLib.c   |  4 ++--
 MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c  |  4 ++--
 MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c |  4 ++--
 5 files changed, 13 insertions(+), 15 deletions(-)

-- 
1.9.5.msysgit.0

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

Re: [edk2] [PATCH 2/3] MdeModulePkg: Update PcdSmbiosDocRev to 0x1 for SMBIOS spec 3.1.1

[Tian, Feng]

reviewed-by: Feng Tian 

-Original Message-
From: Zeng, Star 
Sent: Sunday, January 22, 2017 4:18 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Tian, Feng ; Gao, 
Liming 
Subject: [PATCH 2/3] MdeModulePkg: Update PcdSmbiosDocRev to 0x1 for SMBIOS 
spec 3.1.1

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

Cc: Feng Tian 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdeModulePkg/MdeModulePkg.dec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index 7e67ddccbb85..273cd7e1716f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1519,7 +1519,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, 
PcdsDynamicEx]
 
   ## SMBIOS Docrev field in SMBIOS 3.0 (64-bit) Entry Point Structure.
   # @Prompt SMBIOS Docrev field in SMBIOS 3.0 (64-bit) Entry Point Structure.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0|UINT8|0x0001006A
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x1|UINT8|0x0001006A
 
   ## SMBIOS produce method.
   #  BIT0 set indicates 32-bit entry point and table are produced.
--
2.7.0.windows.1

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

Re: [edk2] [PATCH 1/3] MdePkg: Add definitions for SMBIOS spec 3.1.1

[Gao, Liming]

Star:
  Please fix the incorrect character in comments. With this change, I add my 
Reviewed-by: Liming Gao 

+/// 00h �C 3Fh: MCTP Host Interfaces

Thanks
Liming
>-Original Message-
>From: Zeng, Star
>Sent: Sunday, January 22, 2017 4:18 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star ; Gao, Liming ;
>Kinney, Michael D 
>Subject: [PATCH 1/3] MdePkg: Add definitions for SMBIOS spec 3.1.1
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
>
>This patch is to add definitions for below items.
>Processor Information (Type 4):
>- add socket SP3r2
>- add AMD Zen Processor Family
>Management Controller Host Interface (Type 42):
>- include Host Interface Type and Protocol Identifier enumerations
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng 
>---
> MdePkg/Include/IndustryStandard/SmBios.h | 25
>+++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Include/IndustryStandard/SmBios.h
>b/MdePkg/Include/IndustryStandard/SmBios.h
>index f5c29bcbd0c8..6a8fa1b39d17 100644
>--- a/MdePkg/Include/IndustryStandard/SmBios.h
>+++ b/MdePkg/Include/IndustryStandard/SmBios.h
>@@ -611,6 +611,7 @@ typedef enum {
>   ProcessorFamilyAmdOpteronX2000Series  = 0x68,
>   ProcessorFamilyAmdOpteronASeries  = 0x69,
>   ProcessorFamilyAmdOpteronX3000Series  = 0x6A,
>+  ProcessorFamilyAmdZen = 0x6B,
>   ProcessorFamilyHobbit = 0x70,
>   ProcessorFamilyCrusoeTM5000   = 0x78,
>   ProcessorFamilyCrusoeTM3000   = 0x79,
>@@ -804,7 +805,8 @@ typedef enum {
>   ProcessorUpgradeSocketBGA1440   = 0x34,
>   ProcessorUpgradeSocketBGA1515   = 0x35,
>   ProcessorUpgradeSocketLGA3647_1 = 0x36,
>-  ProcessorUpgradeSocketSP3   = 0x37
>+  ProcessorUpgradeSocketSP3   = 0x37,
>+  ProcessorUpgradeSocketSP3r2 = 0x38
> } PROCESSOR_UPGRADE;
>
> ///
>@@ -2386,6 +2388,25 @@ typedef struct {
> } SMBIOS_TABLE_TYPE41;
>
> ///
>+/// Management Controller Host Interface - Interface Types.
>+/// 00h �C 3Fh: MCTP Host Interfaces
>+///
>+typedef enum{
>+  MCHostInterfaceTypeNetworkHostInterface   = 0x40,
>+  MCHostInterfaceTypeOemDefined = 0xF0
>+} MC_HOST_INTERFACE_TYPE;
>+
>+///
>+/// Management Controller Host Interface - Protocol Types.
>+///
>+typedef enum{
>+  MCHostInterfaceProtocolTypeIPMI   = 0x02,
>+  MCHostInterfaceProtocolTypeMCTP   = 0x03,
>+  MCHostInterfaceProtocolTypeRedfishOverIP  = 0x04,
>+  MCHostInterfaceProtocolTypeOemDefined = 0xF0
>+} MC_HOST_INTERFACE_PROTOCOL_TYPE;
>+
>+///
> /// Management Controller Host Interface (Type 42).
> ///
> /// The information in this structure defines the attributes of a Management
>@@ -2404,7 +2425,7 @@ typedef struct {
> ///
> typedef struct {
>   SMBIOS_STRUCTURE  Hdr;
>-  UINT8 InterfaceType;
>+  UINT8 InterfaceType;  ///< The 
>enumeration value from
>MC_HOST_INTERFACE_TYPE
>   UINT8 MCHostInterfaceData[1]; ///< This field 
> has a minimum
>of four bytes
> } SMBIOS_TABLE_TYPE42;
>
>--
>2.7.0.windows.1

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

[edk2] SetUnicodeMem in DisplayEngineDxe seems unsafe

[wang xiaofeng]

Hi DisplayEngineDxe  Owner,
   SetUnicodeMem seems unsafe  since the buffer may overflow if the input Size 
is bigger than buffer size.Do we think about improve the function 


/**
  Set Buffer to Value for Size bytes.


  @param  Buffer Memory to set.
  @param  Size   Number of bytes to set
  @param  Value  Value of the set operation.


**/
VOID
SetUnicodeMem (
  IN VOID   *Buffer,
  IN UINTN  Size,
  IN CHAR16 Value
  )
{
  CHAR16  *Ptr;


  Ptr = Buffer;
  while ((Size--)  != 0) {
*(Ptr++) = Value;
  }
}
   
   The problem I meet is liking the following screen. Year in main page shows 
incorrect char randomly.

   If I turn off GetNumericInput optimize with #pragma optimize( "", off ) in 
InputHandler.c , or swtich to use StrCpyS like this. The problem disappear. 
This issue cannot be seen in OVMF ,but it can be reproduced in our own platform 
with a rate of 30%. 



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

[edk2] [PATCH 2/3] MdePkg: VariableDxe: Use UEFI_VARIABLE_DATA

[Zhang, Chao B]

Use UEFI_VARIABLE_DATA data structure according to TCG PC-Client PFP Spec
00.21.
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf

Cc: Star Zeng 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
index 309521f..b9febac 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
@@ -96,7 +96,7 @@ MeasureVariable (
 {
   EFI_STATUSStatus;
   UINTN VarNameLength;
-  EFI_VARIABLE_DATA_TREE*VarLog;
+  UEFI_VARIABLE_DATA*VarLog;
   UINT32VarLogSize;
 
   ASSERT ((VarSize == 0 && VarData == NULL) || (VarSize != 0 && VarData != 
NULL));
@@ -105,7 +105,7 @@ MeasureVariable (
   VarLogSize = (UINT32)(sizeof (*VarLog) + VarNameLength * sizeof (*VarName) + 
VarSize
 - sizeof (VarLog->UnicodeName) - sizeof 
(VarLog->VariableData));
 
-  VarLog = (EFI_VARIABLE_DATA_TREE *) AllocateZeroPool (VarLogSize);
+  VarLog = (UEFI_VARIABLE_DATA *) AllocateZeroPool (VarLogSize);
   if (VarLog == NULL) {
 return EFI_OUT_OF_RESOURCES;
   }
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH 1/3] MdePkg: UefiTcgPlatform.h: Add UEFI_VARIABLE_DATA

[Zhang, Chao B]

Add UEFI_VARIABLE_DATA according to TCG PC-Client PFP Spec 00.21.
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf

Cc: Star Zeng 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 MdePkg/Include/IndustryStandard/UefiTcgPlatform.h | 17 +
 1 file changed, 17 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h 
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index 6ce808e..8a3e170 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -151,6 +151,7 @@ typedef struct tdEFI_HANDOFF_TABLE_POINTERS {
 /// This structure serves as the header for measuring variables. The name of 
the
 /// variable (in Unicode format) should immediately follow, then the variable
 /// data.
+/// This is defined in TCG EFI Platform Spec for TPM1.1 or 1.2 V1.22
 ///
 typedef struct tdEFI_VARIABLE_DATA {
   EFI_GUID  VariableName;
@@ -160,6 +161,22 @@ typedef struct tdEFI_VARIABLE_DATA {
   INT8  VariableData[1];  ///< Driver or 
platform-specific data
 } EFI_VARIABLE_DATA;
 
+///
+/// UEFI_VARIABLE_DATA
+///
+/// This structure serves as the header for measuring variables. The name of 
the
+/// variable (in Unicode format) should immediately follow, then the variable
+/// data.
+/// This is defined in TCG PC Client Firmware Profile Spec 00.21
+///
+typedef struct tdUEFI_VARIABLE_DATA {
+  EFI_GUID  VariableName;
+  UINT64UnicodeNameLength;
+  UINT64VariableDataLength;
+  CHAR16UnicodeName[1];
+  INT8  VariableData[1];  ///< Driver or 
platform-specific data
+} UEFI_VARIABLE_DATA;
+
 //
 // For TrEE1.0 compatibility
 //
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH] ShellPkg/pci: Use PCI definitions defined in MdePkg

[Ruiyu Ni]

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

The patch removes the local PCI definitions and uses the definitions
defined in MdePkg/Include/IndustryStandard folder.
There is no functionality impact.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jaben Carsey 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 582 ++
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.h | 423 +---
 2 files changed, 284 insertions(+), 721 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
index 5302051..9cc4f5c 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
@@ -1,7 +1,7 @@
 /** @file
   Main file for Pci shell Debug1 function.
 
-  Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP  
   This program and the accompanying materials
@@ -1928,7 +1928,7 @@ PciExplainData (
 **/
 EFI_STATUS
 PciExplainDeviceData (
-  IN PCI_DEVICE_HEADER  *Device,
+  IN PCI_DEVICE_HEADER_TYPE_REGION  *Device,
   IN UINT64 Address,
   IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL*IoDev
   );
@@ -1944,7 +1944,7 @@ PciExplainDeviceData (
 **/
 EFI_STATUS
 PciExplainBridgeData (
-  IN  PCI_BRIDGE_HEADER *Bridge,
+  IN  PCI_BRIDGE_CONTROL_REGISTER   *Bridge,
   IN  UINT64Address,
   IN  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *IoDev
   );
@@ -1980,7 +1980,7 @@ PciExplainBar (
 **/
 EFI_STATUS
 PciExplainCardBusData (
-  IN PCI_CARDBUS_HEADER *CardBus,
+  IN PCI_CARDBUS_CONTROL_REGISTER   *CardBus,
   IN UINT64 Address,
   IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL*IoDev
   );
@@ -2075,7 +2075,7 @@ PciExplainPciExpress (
 **/
 EFI_STATUS
 ExplainPcieCapReg (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2087,7 +2087,7 @@ ExplainPcieCapReg (
 **/
 EFI_STATUS
 ExplainPcieDeviceCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2099,7 +2099,7 @@ ExplainPcieDeviceCap (
 **/
 EFI_STATUS
 ExplainPcieDeviceControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2111,7 +2111,7 @@ ExplainPcieDeviceControl (
 **/
 EFI_STATUS
 ExplainPcieDeviceStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2123,7 +2123,7 @@ ExplainPcieDeviceStatus (
 **/
 EFI_STATUS
 ExplainPcieLinkCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2135,7 +2135,7 @@ ExplainPcieLinkCap (
 **/
 EFI_STATUS
 ExplainPcieLinkControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2147,7 +2147,7 @@ ExplainPcieLinkControl (
 **/
 EFI_STATUS
 ExplainPcieLinkStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2159,7 +2159,7 @@ ExplainPcieLinkStatus (
 **/
 EFI_STATUS
 ExplainPcieSlotCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2171,7 +2171,7 @@ ExplainPcieSlotCap (
 **/
 EFI_STATUS
 ExplainPcieSlotControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2183,7 +2183,7 @@ ExplainPcieSlotControl (
 **/
 EFI_STATUS
 ExplainPcieSlotStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2195,7 +2195,7 @@ ExplainPcieSlotStatus (
 **/
 EFI_STATUS
 ExplainPcieRootControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2207,7 +2207,7 @@ ExplainPcieRootControl (
 **/
 EFI_STATUS
 ExplainPcieRootCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2219,10 +2219,10 @@ ExplainPcieRootCap (
 **/
 EFI_STATUS
 ExplainPcieRootStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
-typedef EFI_STATUS (*PCIE_EXPLAIN_FUNCTION) (IN PCIE_CAP_STRUCTURE 
*PciExpressCap);
+typedef EFI_STATUS (*PCIE_EXPLAIN_FUNCTION) (IN PCI_CAPABILITY_PCIEXP 
*PciExpressCap);
 
 typedef enum {
   FieldWidthUINT8,
@@ -2452,7 +2452,7 @@ ShellCommandRunPci (
   UINT64Address;
   EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *IoDev;
   EFI_STATUSStatus;
-  PCI_COMMON_HEADER PciHeader;
+  PCI_DEVICE_INDEPENDENT_REGION PciHeader;
   

Re: [edk2] [PATCH 0/1] Refine casting expression result to bigger size

[Wu, Hao A]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, January 24, 2017 4:58 AM
> To: Laszlo Ersek
> Cc: Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 0/1] Refine casting expression result to bigger 
> size
> 
> On 23 January 2017 at 11:01, Laszlo Ersek  wrote:
> > On 01/22/17 07:43, Hao Wu wrote:
> >> Please note that this patch is maily for feedback collection and the patch
> >> only covers MdePkg. We are working on patches for other packages.
> >>
> >>
> >> There are cases that the operands of an expression are all with rank less
> >> than UINT64/INT64 and the result of the expression is casted to
> >> UINT64/INT64 to fit the target size.
> >>
> >> An example will be:
> >> UINT32 a,b;
> >> // a and b can be any unsigned int type with rank less than UINT64, like
> >> // UINT8, UINT16, etc.
> >> UINT64 c;
> >> c = (UINT64) (a + b);
> >>
> >> Some static code checkers may warn that the expression result might
> >> overflow within the rank of int (integer promotions) and the result is
> >> then cast to a bigger size.
> >>
> >> For the consideration of generated binaries size, the commit will keep the
> >> size of the operands as the size of int, and explitly add a type cast
> >> before converting the result to UINT64/INT64.
> >>
> >> 1). When there is no operand with type UINTN
> >> (UINTN)  (a + b) -> (UINTN)(UINT32)  (a + b) or
> >> (UINT64) (a + b) -> (UINT64)(UINT32) (a + b)
> >>
> >> 2). Otherwise
> >> (UINT64) (a + b) -> (UINT64)(UINTN)  (a + b)
> >>
> >> Hao Wu (1):
> >>   MdePkg: Refine casting expression result to bigger size
> >>
> >>  MdePkg/Library/BaseLib/String.c  | 4 ++--
> >>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c| 4 ++--
> >>  MdePkg/Library/BaseS3PciLib/S3PciLib.c   | 4 ++--
> >>  MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c  | 4 ++-
> -
> >>  MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c | 4 ++--
> >>  5 files changed, 10 insertions(+), 10 deletions(-)
> >>
> >
> > - If it is justified to do the addition in 64-bits (or in UINTN), then
> > the addition should be modified accordingly. This might incur a code
> > size increase, but if that's necessary for correctness, then it should
> > be done.
> >
> 
> Indeed. The problem with
> 
> c = (UINT64) (a + b);
> 
> is the parens: this should be written as (UINT64)a + b if the sum may
> overflow a UINT32
> 
> > - However, if the addition is just fine as-is, because we know for sure
> > that the sum will never overflow "int" or "unsigned int" (as selected by
> > the integer promotions and the usual arithmetic conversions), then we're
> > addressing the issue in the wrong direction. Namely, in this case, the
> > solution is to simply drop the outermost cast, which is already useless.
> > (Because, it would automatically happen as part of the assignment or the
> > "return" statement.)
> >
> 
> Agreed.
> 
> > I mean... Those (useless) outermost casts were probably introduced to
> > appease the Visual Studio compiler. Apparently, they cause various
> > static code checkers to complain, so now we introduce yet more casts to
> > keep them quiet as well. When will it end?
> >
> > For example, the 2nd return statement of the InternalHexCharToUintn()
> > function is proposed as
> >
> >   return (UINTN)(UINT32) (10 + InternalCharToUpper (Char) - L'A');
> >
> > in reality it should be just
> >
> >   return 10 + InternalCharToUpper (Char) - L'A';
> >
> 

Hi Laszlo and Ard,

Thanks for the feedbacks. I agree that for the above case the type casts
"(UINT)(UINT32)" are unnecessary.

Also, I agree that for cases that overflow might happen, (UINT64)a + b
should be used.

So I will work out a V2 patch for MdePkg following the below rules:
1). If the expression will not overflow "(unsigned) int", remove the
unnecessary type casts.
2). If overflow is possible, explicitly type cast the first operand in the
expression to bigger size.

Best Regards,
Hao Wu

> Indeed. IMO this is related to our warnings-as-errors policy, while in
> reality, some warnings are simply warnings, and should be ignored if
> it can be confirmed by visual inspection that the expression can never
> overflow.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Ni, Ruiyu]

Tim,
I agree with you.
If the text to display has to be in English. Embedding them in C file makes 
code more readable.

Thanks/Ray

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Tuesday, January 24, 2017 4:52 AM
> To: Carsey, Jaben ; Ni, Ruiyu ;
> Zeng, Star ; edk2-devel@lists.01.org
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> spec 3.1.1
> 
> Also, in some cases, the text is taken directly from the specification.
> Introducing HII strings in order to make these translatable when the source
> material is normative doesn't help, IMO.
> 
> Tim
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Carsey, Jaben
> Sent: Monday, January 23, 2017 9:41 AM
> To: Ni, Ruiyu ; Zeng, Star ; edk2-
> de...@lists.01.org
> Cc: Carsey, Jaben 
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of
> SMBIOS spec 3.1.1
> 
> Reviewed-by: Jaben Carsey 
> 
> I think that string mixed use existed in the EDK version of the command and
> was just never removed.
> 
> -Jaben
> 
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Sunday, January 22, 2017 1:49 AM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec 3.1.1
> > Importance: High
> >
> > Reviewed-by: Ruiyu Ni 
> >
> > Thanks/Ray
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Sunday, January 22, 2017 5:25 PM
> > > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Zeng, Star
> > > 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > > spec 3.1.1
> > >
> > > Ray & Jaben,
> > >
> > > I am not so sure about the rule.
> > > The mixed using of strings in c code and strings in uni file has
> > > been there since it was created.
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Sunday, January 22, 2017 4:56 PM
> > > To: Zeng, Star ; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben 
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > > spec 3.1.1
> > >
> > > Star,
> > > Why some strings are hardcoded in C file while some are defined in
> > > UNI
> > file?
> > > What's the rule?
> > >
> > > Thanks/Ray
> > >
> > > > -Original Message-
> > > > From: Zeng, Star
> > > > Sent: Sunday, January 22, 2017 4:18 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Zeng, Star ; Ni, Ruiyu
> > > > ; Carsey, Jaben 
> > > > Subject: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec
> > > > 3.1.1
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
> > > >
> > > > Cc: Ruiyu Ni 
> > > > Cc: Jaben Carsey 
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Star Zeng 
> > > > ---
> > > >  .../SmbiosView/PrintInfo.c |  6 +++-
> > > >  .../SmbiosView/QueryTable.c| 37
> > ++
> > > >  .../SmbiosView/QueryTable.h| 14 +++-
> > > >  .../SmbiosView/SmbiosViewStrings.uni   |  3 +-
> > > >  4 files changed, 57 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > >
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > >
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > index ecb8e2492453..1d6002b92593 100644
> > > > ---
> > > >
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > +++
> > > >
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > @@ -1106,7 +1106,7 @@ SmbiosPrintStructure (
> > > >// Management Controller Host Interface (Type 42)
> > > >//
> > > >case 42:
> > > > -PRINT_STRUCT_VALUE_H (Struct, Type42, InterfaceType);
> > > > +DisplayMCHostInterfaceType (Struct->Type42->InterfaceType,
> > > > + Option);
> > > >  break;
> > > >
> > > >//
> > > > @@ -1818,6 +1818,10 @@ DisplayProcessorFamily (
> > > >  Print (L"AMD Opteron(TM) X3000 Series APU\n");
> > > >  break;
> > > >
> > > > +  case 0x6B:
> > > > +Print (L"AMD Zen Processor Family\n");
> > > > +break;
> > > > +
> > > >case 0x70:
> > > >  ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN
> > > > (STR_SMBIOSVIEW_PRINTINFO_HOBBIT_FAMILY),
> > gShellDebug1HiiHandle);
> > > >  break;
> > > > diff --git
> > > >
> > >
> >
> 

[edk2] [PATCH] ShellPkg/pci: Use PCI definitions defined in MdePkg

[Ruiyu Ni]

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

The patch removes the local PCI definitions and uses the definitions
defined in MdePkg/Include/IndustryStandard folder.
There is no functionality impact.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jaben Carsey 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 582 ++
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.h | 423 +---
 2 files changed, 284 insertions(+), 721 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
index 5302051..9cc4f5c 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
@@ -1,7 +1,7 @@
 /** @file
   Main file for Pci shell Debug1 function.
 
-  Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP  
   This program and the accompanying materials
@@ -1928,7 +1928,7 @@ PciExplainData (
 **/
 EFI_STATUS
 PciExplainDeviceData (
-  IN PCI_DEVICE_HEADER  *Device,
+  IN PCI_DEVICE_HEADER_TYPE_REGION  *Device,
   IN UINT64 Address,
   IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL*IoDev
   );
@@ -1944,7 +1944,7 @@ PciExplainDeviceData (
 **/
 EFI_STATUS
 PciExplainBridgeData (
-  IN  PCI_BRIDGE_HEADER *Bridge,
+  IN  PCI_BRIDGE_CONTROL_REGISTER   *Bridge,
   IN  UINT64Address,
   IN  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *IoDev
   );
@@ -1980,7 +1980,7 @@ PciExplainBar (
 **/
 EFI_STATUS
 PciExplainCardBusData (
-  IN PCI_CARDBUS_HEADER *CardBus,
+  IN PCI_CARDBUS_CONTROL_REGISTER   *CardBus,
   IN UINT64 Address,
   IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL*IoDev
   );
@@ -2075,7 +2075,7 @@ PciExplainPciExpress (
 **/
 EFI_STATUS
 ExplainPcieCapReg (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2087,7 +2087,7 @@ ExplainPcieCapReg (
 **/
 EFI_STATUS
 ExplainPcieDeviceCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2099,7 +2099,7 @@ ExplainPcieDeviceCap (
 **/
 EFI_STATUS
 ExplainPcieDeviceControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2111,7 +2111,7 @@ ExplainPcieDeviceControl (
 **/
 EFI_STATUS
 ExplainPcieDeviceStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2123,7 +2123,7 @@ ExplainPcieDeviceStatus (
 **/
 EFI_STATUS
 ExplainPcieLinkCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2135,7 +2135,7 @@ ExplainPcieLinkCap (
 **/
 EFI_STATUS
 ExplainPcieLinkControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2147,7 +2147,7 @@ ExplainPcieLinkControl (
 **/
 EFI_STATUS
 ExplainPcieLinkStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2159,7 +2159,7 @@ ExplainPcieLinkStatus (
 **/
 EFI_STATUS
 ExplainPcieSlotCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2171,7 +2171,7 @@ ExplainPcieSlotCap (
 **/
 EFI_STATUS
 ExplainPcieSlotControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2183,7 +2183,7 @@ ExplainPcieSlotControl (
 **/
 EFI_STATUS
 ExplainPcieSlotStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2195,7 +2195,7 @@ ExplainPcieSlotStatus (
 **/
 EFI_STATUS
 ExplainPcieRootControl (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2207,7 +2207,7 @@ ExplainPcieRootControl (
 **/
 EFI_STATUS
 ExplainPcieRootCap (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
 /**
@@ -2219,10 +2219,10 @@ ExplainPcieRootCap (
 **/
 EFI_STATUS
 ExplainPcieRootStatus (
-  IN PCIE_CAP_STRUCTURE *PciExpressCap
+  IN PCI_CAPABILITY_PCIEXP *PciExpressCap
   );
 
-typedef EFI_STATUS (*PCIE_EXPLAIN_FUNCTION) (IN PCIE_CAP_STRUCTURE 
*PciExpressCap);
+typedef EFI_STATUS (*PCIE_EXPLAIN_FUNCTION) (IN PCI_CAPABILITY_PCIEXP 
*PciExpressCap);
 
 typedef enum {
   FieldWidthUINT8,
@@ -2452,7 +2452,7 @@ ShellCommandRunPci (
   UINT64Address;
   EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *IoDev;
   EFI_STATUSStatus;
-  PCI_COMMON_HEADER PciHeader;
+  PCI_DEVICE_INDEPENDENT_REGION PciHeader;
   

[edk2] [PATCH] ShellPkg/pci: Support interpreting specific PCIE ext cap thru "-ec"

[Ruiyu Ni]

The implementation was already there but through a private flag
"-_e". The patch removes "-_e" support and add "-ec" support.
Removing old "-_e" support makes the pci command more clean.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jaben Carsey 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c  | 61 +-
 .../UefiShellDebug1CommandsLib.uni | 15 +++---
 2 files changed, 34 insertions(+), 42 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
index 9cc4f5c..99e6caf 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
@@ -2370,7 +2370,7 @@ PCI_CONFIG_SPACE  *mConfigSpace = NULL;
 STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {L"-s", TypeValue},
   {L"-i", TypeFlag},
-  {L"-_e", TypeValue},
+  {L"-ec", TypeValue},
   {NULL, TypeMax}
   };
 
@@ -2516,6 +2516,11 @@ ShellCommandRunPci (
   ShellStatus = SHELL_INVALID_PARAMETER;
   goto Done;
 }
+if (ShellCommandLineGetFlag(Package, L"-ec") && 
ShellCommandLineGetValue(Package, L"-ec") == NULL) {
+  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE), 
gShellDebug1HiiHandle,  L"pci", L"-ec");  
+  ShellStatus = SHELL_INVALID_PARAMETER;
+  goto Done;
+}
 if (ShellCommandLineGetFlag(Package, L"-s") && 
ShellCommandLineGetValue(Package, L"-s") == NULL) {
   ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE), 
gShellDebug1HiiHandle,  L"pci", L"-s");  
   ShellStatus = SHELL_INVALID_PARAMETER;
@@ -2878,13 +2883,11 @@ ShellCommandRunPci (
 // If "-i" appears in command line, interpret data in configuration space
 //
 if (ExplainData) {
-  EnhancedDump = 0;
-  if (ShellCommandLineGetFlag(Package, L"-_e")) {
-EnhancedDump = 0x;
-Temp = ShellCommandLineGetValue(Package, L"-_e");
-if (Temp != NULL) {
-  EnhancedDump = (UINT16) ShellHexStrToUintn (Temp);
-}
+  EnhancedDump = 0x;
+  if (ShellCommandLineGetFlag(Package, L"-ec")) {
+Temp = ShellCommandLineGetValue(Package, L"-ec");
+ASSERT (Temp != NULL);
+EnhancedDump = (UINT16) ShellHexStrToUintn (Temp);
   }
   Status = PciExplainData (, Address, IoDev, EnhancedDump);
 }
@@ -5829,39 +5832,25 @@ PciExplainPciExpress (
 return EFI_UNSUPPORTED;
   }
 
-  if (EnhancedDump == 0) {
+  ExtHdr = (PCI_EXP_EXT_HDR*)ExRegBuffer;
+  while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) {
 //
-// Print the PciEx extend space in raw bytes ( 0xFF-0xFFF)
+// Process this item
 //
-ShellPrintEx (-1, -1, L"\r\n%HStart dumping PCIex extended configuration 
space (0x100 - 0xFFF).%N\r\n\r\n");
-
-DumpHex (
-  2,
-  EFI_PCIE_CAPABILITY_BASE_OFFSET,
-  ExtendRegSize,
-  (VOID *) (ExRegBuffer)
-  );
-  } else {
-ExtHdr = (PCI_EXP_EXT_HDR*)ExRegBuffer;
-while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) {
+if (EnhancedDump == 0x || EnhancedDump == ExtHdr->CapabilityId) {
   //
-  // Process this item
+  // Print this item
   //
-  if (EnhancedDump == 0x || EnhancedDump == ExtHdr->CapabilityId) {
-//
-// Print this item
-//
-PrintPciExtendedCapabilityDetails((PCI_EXP_EXT_HDR*)ExRegBuffer, 
ExtHdr, );
-  }
+  PrintPciExtendedCapabilityDetails((PCI_EXP_EXT_HDR*)ExRegBuffer, ExtHdr, 
);
+}
 
-  //
-  // Advance to the next item if it exists
-  //
-  if (ExtHdr->NextCapabilityOffset != 0) {
-ExtHdr = (PCI_EXP_EXT_HDR*)((UINT8*)ExRegBuffer + 
ExtHdr->NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET);
-  } else {
-break;
-  }
+//
+// Advance to the next item if it exists
+//
+if (ExtHdr->NextCapabilityOffset != 0) {
+  ExtHdr = (PCI_EXP_EXT_HDR*)((UINT8*)ExRegBuffer + 
ExtHdr->NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET);
+} else {
+  break;
 }
   }
   SHELL_FREE_NON_NULL(ExRegBuffer);
diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
index 06865a4..324e233f 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
@@ -895,17 +895,20 @@
 #string STR_GET_HELP_PCI  #language en-US ""
 ".TH pci 0 "Displays PCI device information."\r\n"
 ".SH NAME\r\n"
-"Displays a PCI device list or PCI function configuration space of a 
device.\r\n"
+"Displays PCI device list or PCI function configuration space and PCIe 
extended\r\n"
+"configuration space.\r\n"
 ".SH SYNOPSIS\r\n"
 " \r\n"
-"PCI [Bus Dev [Func] [-s Seg] [-i]]\r\n"
+"PCI [Bus Dev [Func] 

[edk2] [PATCH] ShellPkg/pci: Fix extended register dumping for MFVC capability

[Ruiyu Ni]

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

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jaben Carsey 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
index 99e6caf..fb7561f 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
@@ -5505,7 +5505,8 @@ PrintInterpretedExtendedCompatibilityVirtualChannel (
   DumpHex (
 4,
 EFI_PCIE_CAPABILITY_BASE_OFFSET + ((UINT8*)HeaderAddress - 
(UINT8*)HeadersBaseAddress),
-sizeof(PCI_EXPRESS_EXTENDED_CAPABILITIES_VIRTUAL_CHANNEL_VC) + 
(Header->ExtendedVcCount - 1) * 
sizeof(PCI_EXPRESS_EXTENDED_CAPABILITIES_VIRTUAL_CHANNEL_CAPABILITY),
+sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_VIRTUAL_CHANNEL_CAPABILITY)
++ Header->ExtendedVcCount * sizeof 
(PCI_EXPRESS_EXTENDED_CAPABILITIES_VIRTUAL_CHANNEL_VC),
 (VOID *) (HeaderAddress)
 );
 
-- 
2.9.0.windows.1

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

Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

I think that string mixed use existed in the EDK version of the command and was 
just never removed.

-Jaben

> -Original Message-
> From: Ni, Ruiyu
> Sent: Sunday, January 22, 2017 1:49 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> spec 3.1.1
> Importance: High
> 
> Reviewed-by: Ruiyu Ni 
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Sunday, January 22, 2017 5:25 PM
> > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Zeng, Star
> > 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec 3.1.1
> >
> > Ray & Jaben,
> >
> > I am not so sure about the rule.
> > The mixed using of strings in c code and strings in uni file has been there
> > since it was created.
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Sunday, January 22, 2017 4:56 PM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben 
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec 3.1.1
> >
> > Star,
> > Why some strings are hardcoded in C file while some are defined in UNI
> file?
> > What's the rule?
> >
> > Thanks/Ray
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Sunday, January 22, 2017 4:18 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Zeng, Star ; Ni, Ruiyu ;
> > > Carsey, Jaben 
> > > Subject: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> spec
> > > 3.1.1
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
> > >
> > > Cc: Ruiyu Ni 
> > > Cc: Jaben Carsey 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Star Zeng 
> > > ---
> > >  .../SmbiosView/PrintInfo.c |  6 +++-
> > >  .../SmbiosView/QueryTable.c| 37
> ++
> > >  .../SmbiosView/QueryTable.h| 14 +++-
> > >  .../SmbiosView/SmbiosViewStrings.uni   |  3 +-
> > >  4 files changed, 57 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > index ecb8e2492453..1d6002b92593 100644
> > > ---
> > >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > +++
> > >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > @@ -1106,7 +1106,7 @@ SmbiosPrintStructure (
> > >// Management Controller Host Interface (Type 42)
> > >//
> > >case 42:
> > > -PRINT_STRUCT_VALUE_H (Struct, Type42, InterfaceType);
> > > +DisplayMCHostInterfaceType (Struct->Type42->InterfaceType,
> > > + Option);
> > >  break;
> > >
> > >//
> > > @@ -1818,6 +1818,10 @@ DisplayProcessorFamily (
> > >  Print (L"AMD Opteron(TM) X3000 Series APU\n");
> > >  break;
> > >
> > > +  case 0x6B:
> > > +Print (L"AMD Zen Processor Family\n");
> > > +break;
> > > +
> > >case 0x70:
> > >  ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN
> > > (STR_SMBIOSVIEW_PRINTINFO_HOBBIT_FAMILY),
> gShellDebug1HiiHandle);
> > >  break;
> > > diff --git
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > c
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > c
> > > index 4a06c12e3b2b..282ba584c8c9 100644
> > > ---
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > c
> > > +++
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > +++ c
> > > @@ -575,6 +575,10 @@ TABLE_ITEM  ProcessorUpgradeTable[] = {
> > >{
> > >  0x37,
> > >  L"Socket SP3"
> > > +  },
> > > +  {
> > > +0x38,
> > > +L"Socket SP3r2"
> > >}
> > >  };
> > >
> > > @@ -3156,6 +3160,22 @@ TABLE_ITEM  IPMIDIBMCInterfaceTypeTable[]
> =
> > {
> > >},
> > >  };
> > >
> > > +TABLE_ITEM  MCHostInterfaceTypeTable[] = {
> > > +  {
> > > +0x3F00,
> > > +L" MCTP Host Interface "
> > > +  },
> > > +  {
> > > +0x40,
> > > +L" Network Host Interface "
> > > +  },
> > > +  {
> > > +0xF0,
> > > +L" OEM defined "
> > > +  },
> > > +};
> > > +
> > > +
> > >  TABLE_ITEM  StructureTypeInfoTable[] = {
> > >{
> > >  0,
> > > @@ -4525,6 +4545,23 @@ DisplayIPMIDIBMCInterfaceType (  }
> > >
> > >  /**
> > > +  Display Management Controller Host Interface (Type 42) information.
> > > +
> > > +  @param[in] Key  The key of the structure.
> > > +  @param[in] Option   The