date:20200616

Re: [edk2-devel] [PATCH 5/5] UefiCpuPkg: Uninstall EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.

2020-06-16 Thread Zhiguang Liu

Hi Laszlo,
Thanks for the comments, I will take the first one.
But I can't find service to unregister protocol notify in EFI_SMM_SYSTEM_TABLE2.
Do you now how the unregister it in SMM driver?

Thanks
Zhiguang

> -Original Message-
> From: Laszlo Ersek 
> Sent: Tuesday, June 16, 2020 11:07 PM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul1 
> Subject: Re: [edk2-devel] [PATCH 5/5] UefiCpuPkg: Uninstall
> EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.
> 
> On 06/16/20 11:04, Zhiguang Liu wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> > To avoid leaking information from SMM, uninstall
> > EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.
> >
> > Cc: Eric Dong 
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Rahul Kumar 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 37
> +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  1 +
> >  2 files changed, 38 insertions(+)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > index db68e1316e..a1b209e125 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > @@ -520,6 +520,33 @@ SmmReadyToLockEventNotify (
> >return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  SMM End of Dxe event notification handler.
> > +
> > +  To avoid leaking information from SMM, uninstall
> > + EFI_SMM_CONFIGURATION_PROTOCOL  at end of Dxe.
> > +
> > +  @param[in] Protocol   Points to the protocol's unique identifier.
> > +  @param[in] Interface  Points to the interface instance.
> > +  @param[in] Handle The handle on which the interface was installed.
> > +
> > +  @retval EFI_SUCCESS   Notification handler runs successfully.
> > + **/
> > +EFI_STATUS
> > +EFIAPI
> > +SmmEndOfDxeNotify (
> > +  IN CONST EFI_GUID  *Protocol,
> > +  IN VOID*Interface,
> > +  IN EFI_HANDLE  Handle
> > +  )
> > +{
> > +  gBS->UninstallProtocolInterface (
> > + gSmmCpuPrivate->SmmCpuHandle,
> > + , 
> >SmmConfiguration
> > + );
> > +  return EFI_SUCCESS;
> > +}
> 
> (1) I suggest setting "gSmmCpuPrivate->SmmCpuHandle" to NULL here.
> 
> (2) I also suggest de-registering the gEfiSmmEndOfDxeProtocolGuid
> notification.
> 
> Thanks
> Laszlo
> 
> > +
> >  /**
> >The module Entry Point of the CPU SMM driver.
> >
> > @@ -1038,6 +1065,16 @@ PiCpuSmmEntry (
> >  );
> >ASSERT_EFI_ERROR (Status);
> >
> > +  //
> > +  // register SMM End of Dxe notification  //  Status =
> > + gSmst->SmmRegisterProtocolNotify (
> > +,
> > +SmmEndOfDxeNotify,
> > +
> > +);
> > +  ASSERT_EFI_ERROR (Status);
> > +
> >//
> >// Initialize SMM Profile feature
> >//
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > index 76b1462996..bb994814d6 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > @@ -105,6 +105,7 @@
> >gEfiSmmConfigurationProtocolGuid ## PRODUCES
> >gEfiSmmCpuProtocolGuid   ## PRODUCES
> >gEfiSmmReadyToLockProtocolGuid   ## NOTIFY
> > +  gEfiSmmEndOfDxeProtocolGuid  ## NOTIFY
> >gEfiSmmCpuServiceProtocolGuid## PRODUCES
> >gEdkiiSmmMemoryAttributeProtocolGuid ## PRODUCES
> >gEfiMmMpProtocolGuid## PRODUCES
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61374): https://edk2.groups.io/g/devel/message/61374
Mute This Topic: https://groups.io/mt/74912556/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/5] MdeModulePkg: avoid SMM pointers being leaked by not using CopyMem()

2020-06-16 Thread Zhiguang Liu

Hi Mike,
This code change is to avoid expose the SMM data and using CopyMem() to copy 
the whole structure will Copy the "next" filed which contain SMM address.
But the Guid is not private information and I think it is ok to use CopyMem() 
to copy Guid.
Maybe the title is confusing, I will change the patch title.

Thanks
Zhiguang

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, June 17, 2020 6:38 AM
> To: devel@edk2.groups.io; Liu, Zhiguang ; Kinney,
> Michael D 
> Cc: Zeng, Star ; Gao, Liming ;
> Wang, Jian J ; Wu, Hao A 
> Subject: RE: [edk2-devel] [PATCH 1/5] MdeModulePkg: avoid SMM pointers
> being leaked by not using CopyMem()
> 
> Zhiguang,
> 
> An implementation of CopyGuid() could use CopyMem().
> Does CopyGuid() also need to be avoided?
> 
> Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On
> > Behalf Of Zhiguang Liu
> > Sent: Tuesday, June 16, 2020 2:05 AM
> > To: devel@edk2.groups.io
> > Cc: Zeng, Star ; Gao, Liming
> > ; Wang, Jian J
> > ; Wu, Hao A 
> > Subject: [edk2-devel] [PATCH 1/5] MdeModulePkg: avoid
> > SMM pointers being leaked by not using CopyMem()
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2002
> >
> > This commit will update the logic in function
> > SmmVariableGetStatistics()
> > so that the pointer fields ('Next' and 'Name') in
> > structure
> > VARIABLE_INFO_ENTRY will not be copied into the SMM
> > communication buffer.
> >
> > Doing so will prevent SMM pointers address from being
> > leaked into non-SMM
> > environment.
> >
> > Please note that newly introduced internal function
> > CopyVarInfoEntry()
> > will not use CopyMem() to copy the whole
> > VARIABLE_INFO_ENTRY structure and
> > then zero out the 'Next' and 'Name' fields. This is for
> > preventing race
> > conditions where the pointers value might still be read.
> >
> > Cc: Star Zeng 
> > Cc: Liming Gao 
> > Cc: Jian J Wang 
> > Signed-off-by: Hao A Wu 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > | 33 +++--
> >  1 file changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > index caca5c3241..74e756bc00 100644
> > ---
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > @@ -315,6 +315,35 @@ GetFvbCountAndBuffer (
> >  }
> >
> >
> >
> >
> >
> > +/**
> >
> > +  Copy only the meaningful fields of the variable
> > statistics information from
> >
> > +  source buffer to the destination buffer. Other fields
> > are filled with zero.
> >
> > +
> >
> > +  @param[out]  DstInfoEntryA pointer to the buffer
> > of destination variable
> >
> > +   information entry.
> >
> > +  @param[in]   SrcInfoEntryA pointer to the buffer
> > of source variable
> >
> > +   information entry.
> >
> > +
> >
> > +**/
> >
> > +static
> >
> > +VOID
> >
> > +CopyVarInfoEntry (
> >
> > +  OUT VARIABLE_INFO_ENTRY*DstInfoEntry,
> >
> > +  IN  VARIABLE_INFO_ENTRY*SrcInfoEntry
> >
> > +  )
> >
> > +{
> >
> > +  DstInfoEntry->Next = NULL;
> >
> > +  DstInfoEntry->Name = NULL;
> >
> > +
> >
> > +  CopyGuid (>VendorGuid, 
> > >VendorGuid);
> >
> > +  DstInfoEntry->Attributes  = SrcInfoEntry->Attributes;
> >
> > +  DstInfoEntry->ReadCount   = SrcInfoEntry->ReadCount;
> >
> > +  DstInfoEntry->WriteCount  = SrcInfoEntry->WriteCount;
> >
> > +  DstInfoEntry->DeleteCount = SrcInfoEntry-
> > >DeleteCount;
> >
> > +  DstInfoEntry->CacheCount  = SrcInfoEntry->CacheCount;
> >
> > +  DstInfoEntry->Volatile= SrcInfoEntry->Volatile;
> >
> > +}
> >
> > +
> >
> >  /**
> >
> >Get the variable statistics information from the
> > information buffer pointed by gVariableInfo.
> >
> >
> >
> > @@ -377,7 +406,7 @@ SmmVariableGetStatistics (
> >*InfoSize = StatisticsInfoSize;
> >
> >return EFI_BUFFER_TOO_SMALL;
> >
> >  }
> >
> > -CopyMem (InfoEntry, VariableInfo, sizeof
> > (VARIABLE_INFO_ENTRY));
> >
> > +CopyVarInfoEntry (InfoEntry, VariableInfo);
> >
> >  CopyMem (InfoName, VariableInfo->Name, NameSize);
> >
> >  *InfoSize = StatisticsInfoSize;
> >
> >  return EFI_SUCCESS;
> >
> > @@ -417,7 +446,7 @@ SmmVariableGetStatistics (
> >  return EFI_BUFFER_TOO_SMALL;
> >
> >}
> >
> >
> >
> > -  CopyMem (InfoEntry, VariableInfo, sizeof
> > (VARIABLE_INFO_ENTRY));
> >
> > +  CopyVarInfoEntry (InfoEntry, VariableInfo);
> >
> >CopyMem (InfoName, VariableInfo->Name, NameSize);
> >
> >*InfoSize = StatisticsInfoSize;
> >
> >
> >
> > --
> > 2.25.1.windows.1
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this
> > group.
> >
> > View/Reply Online (#61324):
> > https://edk2.groups.io/g/devel/message/61324
> > Mute This Topic:

Re: [edk2-devel] [PATCH 5/5] Platform/Intel/Vlv2TbltDevicePkg: Change PCDs type about status code

2020-06-16 Thread Sun, Zailiang

Reviewed-by: Zailiang Sun 

-Original Message-
From: Tan, Ming  
Sent: Tuesday, June 9, 2020 7:24 PM
To: devel@edk2.groups.io
Cc: Sun, Zailiang ; Qian, Yi 
Subject: [PATCH 5/5] Platform/Intel/Vlv2TbltDevicePkg: Change PCDs type about 
status code

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

Since the type of PcdStatusCodeUseSerial and PcdStatusCodeUseMemory in 
MdeModulePkg.dec are changed, so change them from PcdsFeatureFlag to 
PcdsFixedAtBuild in dsc files.

Cc: Zailiang Sun 
Cc: Yi Qian 
Signed-off-by: Ming Tan 
---
 .../Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc| 14 +++---
 .../Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 14 +++---
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index 463b952e65..1cb0b9230a 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -1,7 +1,7 @@
 #/** @file # Platform description. #-# Copyright (c) 2012  - 2019, Intel 
Corporation. All rights reserved.+# Copyright (c) 2012  - 2020, Intel 
Corporation. All rights reserved. # # SPDX-License-Identifier: 
BSD-2-Clause-Patent #@@ -432,12 +432,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|FALSE !endif   
gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreImageLoaderSearchTeSectionFirst|FALSE-!if
 $(TARGET) == RELEASE-  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE-!else-  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE-!endif-  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE !if 
$(ISA_SERIAL_STATUS_CODE_ENABLE) == TRUE   
gEfiSerialPortTokenSpaceGuid.PcdStatusCodeUseIsaSerial|TRUE !else@@ -483,6 
+477,12 @@
 !else   
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE 
!endif+!if $(TARGET) == RELEASE+  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE+!else+  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE+!endif+  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE !if $(TARGET) == 
RELEASE   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0   
gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x3diff --git 
a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index ee18b45c97..62ff5f5c4d 100644
--- a/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -1,7 +1,7 @@
 #/** @file # Platform description. #-# Copyright (c) 2012  - 2019, Intel 
Corporation. All rights reserved.+# Copyright (c) 2012  - 2020, Intel 
Corporation. All rights reserved. # # SPDX-License-Identifier: 
BSD-2-Clause-Patent #@@ -434,12 +434,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|FALSE !endif   
gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreImageLoaderSearchTeSectionFirst|FALSE-!if
 $(TARGET) == RELEASE-  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE-!else-  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE-!endif-  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE !if 
$(ISA_SERIAL_STATUS_CODE_ENABLE) == TRUE   
gEfiSerialPortTokenSpaceGuid.PcdStatusCodeUseIsaSerial|TRUE !else@@ -485,6 
+479,12 @@
 !else   
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE 
!endif+!if $(TARGET) == RELEASE+  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE+!else+  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE+!endif+  
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE !if $(TARGET) == 
RELEASE   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0   
gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x3-- 
2.24.0.windows.2


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61372): https://edk2.groups.io/g/devel/message/61372
Mute This Topic: https://groups.io/mt/74771811/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1 v2] MdeModulePkg/StatusCodeHandler: do not output \n\r for string data

2020-06-16 Thread Wang, Jian J

Reviewed-by: Jian J Wang 

Regards,
Jian

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Tan, Ming
> Sent: Monday, June 15, 2020 10:04 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH 1/1 v2] MdeModulePkg/StatusCodeHandler: do
> not output \n\r for string data
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2787
> 
> When output string data through serial port, will not ouput \n\r now.
> Caller can output several data in one line, and output \n\r when needed.
> 
> Signed-off-by: Ming Tan 
> ---
> V2: Make it as a standalone patch.
>  .../Universal/StatusCodeHandler/Pei/SerialStatusCodeWorker.c| 2 +-
>  .../StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.c   | 2 +-
>  .../Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.c| 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/SerialStatusCodeWorker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/SerialStatusCodeWorker.c
> index 2455f8b2a908..3aa5642b64fb 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Pei/SerialStatusCodeWorker.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Pei/SerialStatusCodeWorker.c
> @@ -134,7 +134,7 @@ SerialStatusCodeReportWorker (
>  CharCount = AsciiSPrint (
>Buffer,
>sizeof (Buffer),
> -  "%a\n\r",
> +  "%a",
>((EFI_STATUS_CODE_STRING_DATA *) Data)->String.Ascii
>);
>} else {
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCode
> Worker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCode
> Worker.c
> index 2dc3ecfff52e..0b98e7ec6315 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCode
> Worker.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCode
> Worker.c
> @@ -129,7 +129,7 @@ SerialStatusCodeReportWorker (
>  CharCount = AsciiSPrint (
>Buffer,
>sizeof (Buffer),
> -  "%a\n\r",
> +  "%a",
>((EFI_STATUS_CODE_STRING_DATA *) Data)->String.Ascii
>);
>} else {
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.
> c
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.
> c
> index c0c907b32f5a..3df0a6712611 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.
> c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.
> c
> @@ -129,7 +129,7 @@ SerialStatusCodeReportWorker (
>  CharCount = AsciiSPrint (
>Buffer,
>sizeof (Buffer),
> -  "%a\n\r",
> +  "%a",
>((EFI_STATUS_CODE_STRING_DATA *) Data)->String.Ascii
>);
>} else {
> --
> 2.24.0.windows.2
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61371): https://edk2.groups.io/g/devel/message/61371
Mute This Topic: https://groups.io/mt/74887116/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 1/1] OvmfPkg: Introduce LSI 53C895A SCSI Controller Driver

2020-06-16 Thread Gary Lin

On Mon, Jun 15, 2020 at 01:27:22PM +0200, Laszlo Ersek wrote:
> Hello Gary,
> 
Hi Laszlo,

> On 06/12/20 12:04, Gary Lin wrote:
> > This commit introduces the driver for LSI 53C895A SCSI Controller
> > which, so that OVMF can access the devices attached to the emulated
> > "lsi" SCSI controller.
> >
> > Cc: Jordan Justen 
> > Cc: Laszlo Ersek 
> > Cc: Ard Biesheuvel 
> > Signed-off-by: Gary Lin 
> > ---
> > Although lsi is now considered obsolete in QEMU, I wrote this driver
> > mainly for learning the UEFI SCSI driver and ... FUN! The majority of
> > the code is actually borrowed from VirtioScsci and MptScsi, and I
> > mainly focus on LsiScsiPassThru().
> >
> > If it's fine to add this driver into OvmfPkg, I'll start to split this
> > patch into smaller pieces to make it easier to review.
> 
> (1) Do we have an official deprecation notice for this SCSI controller
> in QEMU?
> 
> If we do, then (AIUI) the controller will be removed in QEMU in one or
> two releases, so this code would become effectively dead in the mid
> term. I wouldn't like to review and/or carry code that's soon to be
> dead.
> 
I just vaguely remember that virtio-scsi is the new default over lsi and
it's not recommended to use lsi except for the old OS without virtio-scsi
driver.

> (2) If there is no official deprecation notice in QEMU, then I agree it
> makes sense to include this driver. In that case, I have another
> question:
> 
> Do you intend this driver for production purposes? I.e., do you expect
> users or "layered products" (libvirt, proxmox, openstack, ...) to use
> this SCSI controller for some well-defined purpose? (The MPT SCSI and PV
> SCSI drivers had a clear product-oriented modivation:
> 
>   https://edk2.groups.io/g/devel/message/55620
>   http://mid.mail-archive.com/a96b6b74-c35d-e291-2122-9d77f1d5f89c@oracle.com
> )
> 
> (2a) If this driver is not meant for a production environment, then
> LSI_SCSI_ENABLE should be FALSE by default (and I'll do a lighter
> review).
> 
> (2b) If the driver is meant for production, then LSI_SCSI_ENABLE should
> indeed be TRUE, and I'll have to be more diligent in reviewing this.
> 
I kind of wonder if the any serious use for lsi+ovmf. If the OS is so
old and only supports lsi, seabios probably serves it better. Anyway,
I'll try to gather more information from my colleagues to see if they
got any feedback about lsi from our customers. If not, I'll set
LSI_SCSI_ENABLE to FALSE by default.

> For either (2a) or (2b), please do split up the driver into smaller
> patches, and please also add yourself to Maintainers.txt as the
> designated reviewer of the new driver.
> 
Sure. Will do that.

Thanks,

Gary Lin

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61370): https://edk2.groups.io/g/devel/message/61370
Mute This Topic: https://groups.io/mt/74836137/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v3 3/4] Silicon/Hisilicon/Acpi: Add update sas address feature

2020-06-16 Thread Ming Huang




在 2020/6/16 22:20, Leif Lindholm 写道:
> One remaining question, then this set is ready to go in:
> 
> On Tue, Jun 09, 2020 at 21:27:24 +0800, Ming Huang wrote:
>> The updating sas address feature is similar with apdating mac address.
>> Modify updating dsdt flow for add this feature.
>>
>> Signed-off-by: Ming Huang 
>> ---
>>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c  |   2 +-
>>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf |   1 +
>>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c| 292 
>> +++-
>>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.h|   2 +-
>>  4 files changed, 227 insertions(+), 70 deletions(-)
>>
>> diff --git a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c 
>> b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c
>> index c45a0bb..9cdf710 100644
>> --- a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c
>> +++ b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c
>> @@ -46,7 +46,7 @@ UpdateAcpiDsdt (
>>  return;
>>}
>>
>> -  Status = EthMacInit ();
>> +  Status = UpdateAcpiDsdtTable ();
>>if (EFI_ERROR (Status)) {
>>  DEBUG ((DEBUG_ERROR, " UpdateAcpiDsdtTable Failed, Status = %r\n", 
>> Status));
>>}
>> diff --git a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf 
>> b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> index 866ff75..856309a 100644
>> --- a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> +++ b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> @@ -46,6 +46,7 @@
>>gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>>gEfiAcpiSdtProtocolGuid   # PROTOCOL ALWAYS_CONSUMED
>>gHisiBoardNicProtocolGuid   # PROTOCOL 
>> ALWAYS_CONSUMED
>> +  gHisiSasConfigProtocolGuid
>>
>>  [FeaturePcd]
>>gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol
>> diff --git a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c 
>> b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c
>> index cd98506..841c94e 100644
>> --- a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c
>> +++ b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c
>> @@ -1,7 +1,7 @@
>>  /** @file
>>
>>Copyright (c) 2014, Applied Micro Curcuit Corporation. All rights 
>> reserved.
>> -  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
>> +  Copyright (c) 2015 - 2020, Hisilicon Limited. All rights reserved.
>>Copyright (c) 2015, Linaro Limited. All rights reserved.
>>SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>> @@ -23,6 +23,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -32,6 +33,7 @@
>>  #include 
>>
>>  #include 
>> +#include 
>>
>>  // Turn on debug message by enabling below define
>>  //#define ACPI_DEBUG
>> @@ -45,17 +47,27 @@
>>  #define EFI_ACPI_MAX_NUM_TABLES 20
>>  #define DSDT_SIGNATURE  0x54445344
>>
>> -#define D03_ACPI_ETH_ID "HISI00C2"
>> -
>>  #define ACPI_ETH_MAC_KEY"local-mac-address"
>> +#define ACPI_ETH_SAS_KEY"sas-addr"
>>
>>  #define PREFIX_VARIABLE_NAMEL"MAC"
>>  #define PREFIX_VARIABLE_NAME_COMPAT L"RGMII_MAC"
>> -#define MAC_MAX_LEN 30
>> +#define ADDRESS_MAX_LEN 30
>> +
>> +CHAR8 *mHisiAcpiDevId[] = {"HISI00C1","HISI00C2","HISI0162"};
>> +
>> +typedef enum {
>> +  DsdtDeviceUnknown,
>> +  DsdtDeviceLan,
>> +  DsdtDeviceSas
>> +} DSDT_DEVICE_TYPE;
>>
>> -EFI_STATUS GetEnvMac(
>> -  IN  UINTNMacNextID,
>> -  IN OUT  UINT8*MacBuffer)
>> +STATIC
>> +EFI_STATUS
>> +GetEnvMac(
>> +  IN UINTNMacNextID,
>> +  IN OUT UINT8*MacBuffer
>> +  )
>>  {
>>EFI_MAC_ADDRESS Mac;
>>EFI_STATUS Status;
>> @@ -89,12 +101,121 @@ EFI_STATUS GetEnvMac(
>>return EFI_SUCCESS;
>>  }
>>
>> -EFI_STATUS _SearchReplacePackageMACAddress(
>> +STATIC
>> +EFI_STATUS
>> +GetSasAddress (
>> +  IN UINT8Index,
>> +  IN OUT UINT8*SasAddrBuffer
>> +  )
>> +{
>> +  EFI_STATUS Status;
>> +  HISI_SAS_CONFIG_PROTOCOL *HisiSasConf;
>> +
>> +  if (SasAddrBuffer == NULL) {
>> +return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  Status = gBS->LocateProtocol (, NULL, (VOID 
>> **));
>> +  if (EFI_ERROR (Status)) {
>> +DEBUG ((DEBUG_ERROR, "Locate Sas Config Protocol failed %r\n", Status));
>> +SasAddrBuffer[0] = 0x50;
>> +SasAddrBuffer[1] = 0x01;
>> +SasAddrBuffer[2] = 0x88;
>> +SasAddrBuffer[3] = 0x20;
>> +SasAddrBuffer[4] = 0x16;
>> +SasAddrBuffer[5] = 0x00;
>> +SasAddrBuffer[6] = 0x00;
>> +SasAddrBuffer[7] = Index;
> 
> This is still a sompletely random-looking value being stuffed into the
> buffer. What is it?

This is a random value. Maybe it is more appropriate to stuff zero into the
buffer here.

Thanks,
Ming

> 
> /
> Leif
> 
>> +return Status;
>> +  }
>> +
>> +

[edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Tue, 06/16/2020 6:30pm-7:30pm #cal-reminder

2020-06-16 Thread devel@edk2.groups.io Calendar

*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Tuesday, 16 June 2020, 6:30pm to 7:30pm, (GMT-07:00) America/Los Angeles

*Where:* https://bluejeans.com/889357567?src=join_info

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=816012 )

*Organizer:* Brian Richardson brian.richard...@intel.com ( 
brian.richard...@intel.com?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

*Description:*

https://www.tianocore.org/bug-triage

Meeting URL

https://bluejeans.com/889357567?src=join_info

Meeting ID

889 357 567

Want to dial in from a phone?

Dial one of the following numbers:

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

(see all numbers - https://www.bluejeans.com/numbers)

Enter the meeting ID and passcode followed by #

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61368): https://edk2.groups.io/g/devel/message/61368
Mute This Topic: https://groups.io/mt/74929625/21656
Mute #cal-reminder: https://groups.io/g/edk2/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib

2020-06-16 Thread Dong, Eric

Reviewed-by: Eric Dong 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> Kirkendall, Garrett
> Sent: Tuesday, June 16, 2020 2:30 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek 
> Subject: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Move
> StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
> 
> Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
> separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
> This allows for future use of StandarSignatureIsAuthinticAMD without
> creating more instances in other modules.
> 
> This function allows IA32/X64 code to determine if it is running on an AMD
> brand processor.
> 
> UefiCpuLib is already included directly or indirectly in all modified modules.
> Complete move is made in this change.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Garrett Kirkendall 
> ---
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf |  7 
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf |  2 ++
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf |  2 ++
>  UefiCpuPkg/Include/Library/UefiCpuLib.h  | 14 
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c   | 38
> 
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c   | 25 
> ++---
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++--
> -
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 23 
> 
>  8 files changed, 67 insertions(+), 69 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> index 006b7acbf14e..34d3a7bb4303 100644
> --- a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> @@ -4,6 +4,7 @@
>  #  The library routines are UEFI specification compliant.
>  #
>  #  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2020, AMD Inc. All rights reserved.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  #  ## @@ -29,6 +30,12 @@
> [Sources.IA32]  [Sources.X64]
>X64/InitializeFpu.nasm
> 
> +[Sources]
> +  BaseUefiCpuLib.c
> +
>  [Packages]
>MdePkg/MdePkg.dec
>UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> index bdb2ff372677..561baa44b0e6 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> @@ -5,6 +5,7 @@
>  #  where local APIC is disabled.
>  #
>  #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2020, AMD Inc. All rights reserved.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  #  ## @@ -37,6 +38,7 @@
> [LibraryClasses]
>TimerLib
>IoLib
>PcdLib
> +  UefiCpuLib
> 
>  [Pcd]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds  ##
> SOMETIMES_CONSUMES diff --git
> a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> index ac1e0a1c9896..1e2a4f8b790f 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> @@ -5,6 +5,7 @@
>  #  where local APIC is disabled.
>  #
>  #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2020, AMD Inc. All rights reserved.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  #  ## @@ -37,6 +38,7 @@
> [LibraryClasses]
>TimerLib
>IoLib
>PcdLib
> +  UefiCpuLib
> 
>  [Pcd]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds  ##
> SOMETIMES_CONSUMES diff --git
> a/UefiCpuPkg/Include/Library/UefiCpuLib.h
> b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> index 82e53bab3a0f..5326e7246301 100644
> --- a/UefiCpuPkg/Include/Library/UefiCpuLib.h
> +++ b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> @@ -5,6 +5,7 @@
>to be UEFI specification compliant.
> 
>Copyright (c) 2009, Intel Corporation. All rights reserved.
> +  Copyright (c) 2020, AMD Inc. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -29,4 +30,17 @@ InitializeFloatingPointUnits (
>VOID
>);
> 
> +/**
> +  Determine if the standard CPU signature is "AuthenticAMD".
> +
> +  @retval TRUE  The CPU signature matches.
> +  @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +StandardSignatureIsAuthenticAMD (
> +  VOID
> +  );
> +
>  #endif
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> new file mode 100644
> index ..c2cc3ff9a709
> --- /dev/null
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> @@ -0,0 +1,38 @@
> +/** @file
> +  This

Re: [edk2-devel] [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI LOADED_IMAGE from SMM.

2020-06-16 Thread Michael D Kinney

Zhiguang,

The commit message should not have more than one Signed-off-by.

Use of Suggested-by may be more appropriate here.

Mike

> -Original Message-
> From: Liu, Zhiguang 
> Sent: Tuesday, June 16, 2020 2:05 AM
> To: devel@edk2.groups.io
> Cc: Tian, Feng ; Zeng, Star
> ; Kinney, Michael D
> ; Laszlo Ersek
> ; Yao, Jiewen 
> Subject: [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI
> LOADED_IMAGE from SMM.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> 
> This patch removes EFI_LOADED_IMAGE_PROTOCOL from DXE
> database.
> The SmmCore only install EFI_LOADED_IMAGE_PROTOCOL to
> SMM database.
> 
> Exposing LOADED_IMAGE_PROTOCOL may cause SMM information
> leakage
> to non-SMM component.
> 
> Cc: Feng Tian 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Laszlo Ersek 
> Signed-off-by: Jiewen Yao 
> Signed-off-by: Zhiguang Liu 
> ---
>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 104
> +++-
> 
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c  |  34 
> --
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h  |   4 ++--
>  3 files changed, 13 insertions(+), 129 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> index 76ee9e0b89..2be2866234 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> @@ -316,7 +316,7 @@ SmmLoadImage (
>EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
> 
>PE_COFF_LOADER_IMAGE_CONTEXT   ImageContext;
> 
> 
> 
> -  PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle);
> 
> +  PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle);
> 
> 
> 
>Buffer   = NULL;
> 
>Size = 0;
> 
> @@ -546,51 +546,14 @@ SmmLoadImage (
>DriverEntry->ImageBuffer  = DstBuffer;
> 
>DriverEntry->NumberOfPage = PageCount;
> 
> 
> 
> -  //
> 
> -  // Allocate a Loaded Image Protocol in
> EfiBootServicesData
> 
> -  //
> 
> -  Status = gBS->AllocatePool (EfiBootServicesData,
> sizeof (EFI_LOADED_IMAGE_PROTOCOL), (VOID
> **)>LoadedImage);
> 
> -  if (EFI_ERROR (Status)) {
> 
> -if (Buffer != NULL) {
> 
> -  gBS->FreePool (Buffer);
> 
> -}
> 
> -SmmFreePages (DstBuffer, PageCount);
> 
> -return Status;
> 
> -  }
> 
> -
> 
> -  ZeroMem (DriverEntry->LoadedImage, sizeof
> (EFI_LOADED_IMAGE_PROTOCOL));
> 
>//
> 
>// Fill in the remaining fields of the Loaded Image
> Protocol instance.
> 
> -  // Note: ImageBase is an SMRAM address that can not
> be accessed outside of SMRAM if SMRAM window is closed.
> 
>//
> 
> -  DriverEntry->LoadedImage->Revision  =
> EFI_LOADED_IMAGE_PROTOCOL_REVISION;
> 
> -  DriverEntry->LoadedImage->ParentHandle  =
> gSmmCorePrivate->SmmIplImageHandle;
> 
> -  DriverEntry->LoadedImage->SystemTable   = gST;
> 
> -  DriverEntry->LoadedImage->DeviceHandle  =
> DeviceHandle;
> 
> -
> 
>DriverEntry->SmmLoadedImage.Revision =
> EFI_LOADED_IMAGE_PROTOCOL_REVISION;
> 
>DriverEntry->SmmLoadedImage.ParentHandle =
> gSmmCorePrivate->SmmIplImageHandle;
> 
>DriverEntry->SmmLoadedImage.SystemTable  = gST;
> 
>DriverEntry->SmmLoadedImage.DeviceHandle =
> DeviceHandle;
> 
> 
> 
> -  //
> 
> -  // Make an EfiBootServicesData buffer copy of
> FilePath
> 
> -  //
> 
> -  Status = gBS->AllocatePool (EfiBootServicesData,
> GetDevicePathSize (FilePath), (VOID **)
> >LoadedImage->FilePath);
> 
> -  if (EFI_ERROR (Status)) {
> 
> -if (Buffer != NULL) {
> 
> -  gBS->FreePool (Buffer);
> 
> -}
> 
> -SmmFreePages (DstBuffer, PageCount);
> 
> -return Status;
> 
> -  }
> 
> -  CopyMem (DriverEntry->LoadedImage->FilePath,
> FilePath, GetDevicePathSize (FilePath));
> 
> -
> 
> -  DriverEntry->LoadedImage->ImageBase = (VOID
> *)(UINTN) ImageContext.ImageAddress;
> 
> -  DriverEntry->LoadedImage->ImageSize =
> ImageContext.ImageSize;
> 
> -  DriverEntry->LoadedImage->ImageCodeType =
> EfiRuntimeServicesCode;
> 
> -  DriverEntry->LoadedImage->ImageDataType =
> EfiRuntimeServicesData;
> 
> -
> 
>//
> 
>// Make a buffer copy of FilePath
> 
>//
> 
> @@ -599,7 +562,6 @@ SmmLoadImage (
>  if (Buffer != NULL) {
> 
>gBS->FreePool (Buffer);
> 
>  }
> 
> -gBS->FreePool (DriverEntry->LoadedImage->FilePath);
> 
>  SmmFreePages (DstBuffer, PageCount);
> 
>  return Status;
> 
>}
> 
> @@ -610,16 +572,6 @@ SmmLoadImage (
>DriverEntry->SmmLoadedImage.ImageCodeType =
> EfiRuntimeServicesCode;
> 
>DriverEntry->SmmLoadedImage.ImageDataType =
> EfiRuntimeServicesData;
> 
> 
> 
> -  //
> 
> -  // Create a new image handle in the UEFI handle
> database for the SMM Driver
> 
> -  //
> 
> -  DriverEntry->ImageHandle = NULL;
> 
> -  Status = gBS->InstallMultipleProtocolInterfaces (
> 
> -  >ImageHandle,
> 
> -  ,
> DriverEntry->LoadedImage,
> 
> -  NULL
> 
> -  );
>

Re: [edk2-devel] [PATCH 3/5] MdePkg: Remove DXE_SMM_DRIVER support for some libraries

2020-06-16 Thread Michael D Kinney

Reviewed-by: Michael D Kinney 

Mike

> -Original Message-
> From: Liu, Zhiguang 
> Sent: Tuesday, June 16, 2020 2:05 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao,
> Liming 
> Subject: [PATCH 3/5] MdePkg: Remove DXE_SMM_DRIVER
> support for some libraries
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> Remove DXE_SMM_DRIVER support for some libraries because
> they
> have the risks of leaking data from SMM mode to non-SMM
> mode.
> 
> Cc: Kinney Michael D 
> Cc: Gao Liming 
> Signed-off-by: Zhiguang Liu 
> ---
> 
> MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuid
> edSectionLib.inf | 2 +-
>  MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
> | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGu
> idedSectionLib.inf
> b/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGu
> idedSectionLib.inf
> index 33eeab405f..acbe62e352 100644
> ---
> a/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGu
> idedSectionLib.inf
> +++
> b/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGu
> idedSectionLib.inf
> @@ -17,7 +17,7 @@
>FILE_GUID  = f773469b-e265-4b0c-
> b0a6-2f971fbfe72b
> 
>MODULE_TYPE= DXE_DRIVER
> 
>VERSION_STRING = 1.0
> 
> -  LIBRARY_CLASS  =
> ExtractGuidedSectionLib|DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION
> UEFI_DRIVER
> 
> +  LIBRARY_CLASS  =
> ExtractGuidedSectionLib|DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
> 
> 
> 
>CONSTRUCTOR=
> DxeExtractGuidedSectionLibConstructor
> 
> 
> 
> diff --git a/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
> b/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
> index c719481cdd..8c1cb3d0cc 100644
> --- a/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
> +++ b/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
> @@ -14,7 +14,7 @@
>FILE_GUID  = 7DE1C620-F587-4116-
> A36D-40F3467B9A0C
> 
>MODULE_TYPE= DXE_DRIVER
> 
>VERSION_STRING = 1.0
> 
> -  LIBRARY_CLASS  = HstiLib|DXE_CORE
> DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
> UEFI_APPLICATION UEFI_DRIVER
> 
> +  LIBRARY_CLASS  = HstiLib|DXE_CORE
> DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION
> UEFI_DRIVER
> 
> 
> 
>  [Sources]
> 
>HstiAip.c
> 
> --
> 2.25.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61365): https://edk2.groups.io/g/devel/message/61365
Mute This Topic: https://groups.io/mt/74912553/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI LOADED_IMAGE from SMM.

2020-06-16 Thread Michael D Kinney

Zhiguang,

There are some source level debug solutions that depend
on the EDI_LOADED_IMAGE_PROTOCOL structure.  Does this 
change reduce the source level debug capabilities for
SMM drivers under development?

Thanks,

Mike

> -Original Message-
> From: Liu, Zhiguang 
> Sent: Tuesday, June 16, 2020 2:05 AM
> To: devel@edk2.groups.io
> Cc: Tian, Feng ; Zeng, Star
> ; Kinney, Michael D
> ; Laszlo Ersek
> ; Yao, Jiewen 
> Subject: [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI
> LOADED_IMAGE from SMM.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> 
> This patch removes EFI_LOADED_IMAGE_PROTOCOL from DXE
> database.
> The SmmCore only install EFI_LOADED_IMAGE_PROTOCOL to
> SMM database.
> 
> Exposing LOADED_IMAGE_PROTOCOL may cause SMM information
> leakage
> to non-SMM component.
> 
> Cc: Feng Tian 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Laszlo Ersek 
> Signed-off-by: Jiewen Yao 
> Signed-off-by: Zhiguang Liu 
> ---
>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 104
> +++-
> 
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c  |  34 
> --
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h  |   4 ++--
>  3 files changed, 13 insertions(+), 129 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> index 76ee9e0b89..2be2866234 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> @@ -316,7 +316,7 @@ SmmLoadImage (
>EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
> 
>PE_COFF_LOADER_IMAGE_CONTEXT   ImageContext;
> 
> 
> 
> -  PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle);
> 
> +  PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle);
> 
> 
> 
>Buffer   = NULL;
> 
>Size = 0;
> 
> @@ -546,51 +546,14 @@ SmmLoadImage (
>DriverEntry->ImageBuffer  = DstBuffer;
> 
>DriverEntry->NumberOfPage = PageCount;
> 
> 
> 
> -  //
> 
> -  // Allocate a Loaded Image Protocol in
> EfiBootServicesData
> 
> -  //
> 
> -  Status = gBS->AllocatePool (EfiBootServicesData,
> sizeof (EFI_LOADED_IMAGE_PROTOCOL), (VOID
> **)>LoadedImage);
> 
> -  if (EFI_ERROR (Status)) {
> 
> -if (Buffer != NULL) {
> 
> -  gBS->FreePool (Buffer);
> 
> -}
> 
> -SmmFreePages (DstBuffer, PageCount);
> 
> -return Status;
> 
> -  }
> 
> -
> 
> -  ZeroMem (DriverEntry->LoadedImage, sizeof
> (EFI_LOADED_IMAGE_PROTOCOL));
> 
>//
> 
>// Fill in the remaining fields of the Loaded Image
> Protocol instance.
> 
> -  // Note: ImageBase is an SMRAM address that can not
> be accessed outside of SMRAM if SMRAM window is closed.
> 
>//
> 
> -  DriverEntry->LoadedImage->Revision  =
> EFI_LOADED_IMAGE_PROTOCOL_REVISION;
> 
> -  DriverEntry->LoadedImage->ParentHandle  =
> gSmmCorePrivate->SmmIplImageHandle;
> 
> -  DriverEntry->LoadedImage->SystemTable   = gST;
> 
> -  DriverEntry->LoadedImage->DeviceHandle  =
> DeviceHandle;
> 
> -
> 
>DriverEntry->SmmLoadedImage.Revision =
> EFI_LOADED_IMAGE_PROTOCOL_REVISION;
> 
>DriverEntry->SmmLoadedImage.ParentHandle =
> gSmmCorePrivate->SmmIplImageHandle;
> 
>DriverEntry->SmmLoadedImage.SystemTable  = gST;
> 
>DriverEntry->SmmLoadedImage.DeviceHandle =
> DeviceHandle;
> 
> 
> 
> -  //
> 
> -  // Make an EfiBootServicesData buffer copy of
> FilePath
> 
> -  //
> 
> -  Status = gBS->AllocatePool (EfiBootServicesData,
> GetDevicePathSize (FilePath), (VOID **)
> >LoadedImage->FilePath);
> 
> -  if (EFI_ERROR (Status)) {
> 
> -if (Buffer != NULL) {
> 
> -  gBS->FreePool (Buffer);
> 
> -}
> 
> -SmmFreePages (DstBuffer, PageCount);
> 
> -return Status;
> 
> -  }
> 
> -  CopyMem (DriverEntry->LoadedImage->FilePath,
> FilePath, GetDevicePathSize (FilePath));
> 
> -
> 
> -  DriverEntry->LoadedImage->ImageBase = (VOID
> *)(UINTN) ImageContext.ImageAddress;
> 
> -  DriverEntry->LoadedImage->ImageSize =
> ImageContext.ImageSize;
> 
> -  DriverEntry->LoadedImage->ImageCodeType =
> EfiRuntimeServicesCode;
> 
> -  DriverEntry->LoadedImage->ImageDataType =
> EfiRuntimeServicesData;
> 
> -
> 
>//
> 
>// Make a buffer copy of FilePath
> 
>//
> 
> @@ -599,7 +562,6 @@ SmmLoadImage (
>  if (Buffer != NULL) {
> 
>gBS->FreePool (Buffer);
> 
>  }
> 
> -gBS->FreePool (DriverEntry->LoadedImage->FilePath);
> 
>  SmmFreePages (DstBuffer, PageCount);
> 
>  return Status;
> 
>}
> 
> @@ -610,16 +572,6 @@ SmmLoadImage (
>DriverEntry->SmmLoadedImage.ImageCodeType =
> EfiRuntimeServicesCode;
> 
>DriverEntry->SmmLoadedImage.ImageDataType =
> EfiRuntimeServicesData;
> 
> 
> 
> -  //
> 
> -  // Create a new image handle in the UEFI handle
> database for the SMM Driver
> 
> -  //
> 
> -  DriverEntry->ImageHandle = NULL;
> 
> -  Status = gBS->InstallMultipleProtocolInterfaces (
> 
> -  >ImageHandle,
> 
> -

Re: [edk2-devel] [PATCH 1/5] MdeModulePkg: avoid SMM pointers being leaked by not using CopyMem()

2020-06-16 Thread Michael D Kinney

Zhiguang,

An implementation of CopyGuid() could use CopyMem().
Does CopyGuid() also need to be avoided?

Mike

> -Original Message-
> From: devel@edk2.groups.io  On
> Behalf Of Zhiguang Liu
> Sent: Tuesday, June 16, 2020 2:05 AM
> To: devel@edk2.groups.io
> Cc: Zeng, Star ; Gao, Liming
> ; Wang, Jian J
> ; Wu, Hao A 
> Subject: [edk2-devel] [PATCH 1/5] MdeModulePkg: avoid
> SMM pointers being leaked by not using CopyMem()
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2002
> 
> This commit will update the logic in function
> SmmVariableGetStatistics()
> so that the pointer fields ('Next' and 'Name') in
> structure
> VARIABLE_INFO_ENTRY will not be copied into the SMM
> communication buffer.
> 
> Doing so will prevent SMM pointers address from being
> leaked into non-SMM
> environment.
> 
> Please note that newly introduced internal function
> CopyVarInfoEntry()
> will not use CopyMem() to copy the whole
> VARIABLE_INFO_ENTRY structure and
> then zero out the 'Next' and 'Name' fields. This is for
> preventing race
> conditions where the pointers value might still be read.
> 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Cc: Jian J Wang 
> Signed-off-by: Hao A Wu 
> Signed-off-by: Zhiguang Liu 
> ---
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 33 +++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> .c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> .c
> index caca5c3241..74e756bc00 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> .c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> .c
> @@ -315,6 +315,35 @@ GetFvbCountAndBuffer (
>  }
> 
> 
> 
> 
> 
> +/**
> 
> +  Copy only the meaningful fields of the variable
> statistics information from
> 
> +  source buffer to the destination buffer. Other fields
> are filled with zero.
> 
> +
> 
> +  @param[out]  DstInfoEntryA pointer to the buffer
> of destination variable
> 
> +   information entry.
> 
> +  @param[in]   SrcInfoEntryA pointer to the buffer
> of source variable
> 
> +   information entry.
> 
> +
> 
> +**/
> 
> +static
> 
> +VOID
> 
> +CopyVarInfoEntry (
> 
> +  OUT VARIABLE_INFO_ENTRY*DstInfoEntry,
> 
> +  IN  VARIABLE_INFO_ENTRY*SrcInfoEntry
> 
> +  )
> 
> +{
> 
> +  DstInfoEntry->Next = NULL;
> 
> +  DstInfoEntry->Name = NULL;
> 
> +
> 
> +  CopyGuid (>VendorGuid, 
> >VendorGuid);
> 
> +  DstInfoEntry->Attributes  = SrcInfoEntry->Attributes;
> 
> +  DstInfoEntry->ReadCount   = SrcInfoEntry->ReadCount;
> 
> +  DstInfoEntry->WriteCount  = SrcInfoEntry->WriteCount;
> 
> +  DstInfoEntry->DeleteCount = SrcInfoEntry-
> >DeleteCount;
> 
> +  DstInfoEntry->CacheCount  = SrcInfoEntry->CacheCount;
> 
> +  DstInfoEntry->Volatile= SrcInfoEntry->Volatile;
> 
> +}
> 
> +
> 
>  /**
> 
>Get the variable statistics information from the
> information buffer pointed by gVariableInfo.
> 
> 
> 
> @@ -377,7 +406,7 @@ SmmVariableGetStatistics (
>*InfoSize = StatisticsInfoSize;
> 
>return EFI_BUFFER_TOO_SMALL;
> 
>  }
> 
> -CopyMem (InfoEntry, VariableInfo, sizeof
> (VARIABLE_INFO_ENTRY));
> 
> +CopyVarInfoEntry (InfoEntry, VariableInfo);
> 
>  CopyMem (InfoName, VariableInfo->Name, NameSize);
> 
>  *InfoSize = StatisticsInfoSize;
> 
>  return EFI_SUCCESS;
> 
> @@ -417,7 +446,7 @@ SmmVariableGetStatistics (
>  return EFI_BUFFER_TOO_SMALL;
> 
>}
> 
> 
> 
> -  CopyMem (InfoEntry, VariableInfo, sizeof
> (VARIABLE_INFO_ENTRY));
> 
> +  CopyVarInfoEntry (InfoEntry, VariableInfo);
> 
>CopyMem (InfoName, VariableInfo->Name, NameSize);
> 
>*InfoSize = StatisticsInfoSize;
> 
> 
> 
> --
> 2.25.1.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this
> group.
> 
> View/Reply Online (#61324):
> https://edk2.groups.io/g/devel/message/61324
> Mute This Topic: https://groups.io/mt/74912557/1643496
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [michael.d.kin...@intel.com]
> -=-=-=-=-=-=


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61363): https://edk2.groups.io/g/devel/message/61363
Mute This Topic: https://groups.io/mt/74912557/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol

2020-06-16 Thread Ard Biesheuvel


On 6/16/20 8:17 PM, Ard Biesheuvel wrote:

NetLibDetectMedia () in DxeNetLib is used as a fallback on implementations
of the SNP protocol that do not also carry an implementation of the adapter
info protocol to provide media state information, and it does all kinds of
terrible things to the network interface (stopping and restarting multiple
times, reprogramming the multicast filters etc etc) to workaround some
alleged UNDI shortcoming.

Although our GENET code should be bullet proof and therefore able to take
this kind of abuse, it is better to avoid it, and provide an implementation
of the adapter info protocol that returns the media state directly, without
the need to mistreat the SNP layer.

Cc: Pete Batard 
Cc: Andrei Warkentin (awarken...@vmware.com) 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Ard Biesheuvel 
---


Note that this code was build tested only, and turns out not to work as 
expected. Samer and I are collaborating off-list, I will follow up with 
a v2 once we have code that actually works.




  Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf |   3 +
  Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h   |   4 +
  Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c   | 109 

  Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c |  12 ++-
  4 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
index 28f3e66ebaf0..89dee9f10c83 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
@@ -19,6 +19,7 @@ [Defines]
UNLOAD_IMAGE   = GenetUnload
  
  [Sources]

+  AdapterInfo.c
BcmGenetDxe.h
ComponentName.c
DriverBinding.c
@@ -49,10 +50,12 @@ [LibraryClasses]
  
  [Protocols]

gBcmGenetPlatformDeviceProtocolGuid ## TO_START
+  gEfiAdapterInformationProtocolGuid  ## BY_START
gEfiDevicePathProtocolGuid  ## BY_START
gEfiSimpleNetworkProtocolGuid   ## BY_START
  
  [Guids]

+  gEfiAdapterInfoMediaStateGuid
gEfiEventExitBootServicesGuid
  
  [FixedPcd]

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
index 0af9d5209cf2..48bb8550426f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
@@ -14,6 +14,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -209,6 +210,8 @@ typedef struct {
EFI_SIMPLE_NETWORK_PROTOCOL Snp;
EFI_SIMPLE_NETWORK_MODE SnpMode;
  
+  EFI_ADAPTER_INFORMATION_PROTOCOLAip;

+
BCM_GENET_PLATFORM_DEVICE_PROTOCOL  *Dev;
  
GENERIC_PHY_PRIVATE_DATAPhy;

@@ -234,6 +237,7 @@ extern EFI_COMPONENT_NAME_PROTOCOL
gGenetComponentName;
  extern EFI_COMPONENT_NAME2_PROTOCOL   gGenetComponentName2;
  
  extern CONST EFI_SIMPLE_NETWORK_PROTOCOL  gGenetSimpleNetworkTemplate;

+extern CONST EFI_ADAPTER_INFORMATION_PROTOCOL gGenetAdapterInfoTemplate;
  
  #define GENET_DRIVER_SIGNATURESIGNATURE_32('G', 'N', 'E', 'T')

  #define GENET_PRIVATE_DATA_FROM_SNP_THIS(a)   CR(a, GENET_PRIVATE_DATA, Snp, 
GENET_DRIVER_SIGNATURE)
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
new file mode 100644
index ..cb7733bbba76
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
@@ -0,0 +1,109 @@
+/** @file
+
+  Copyright (c) 2020 Arm, Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+#include "BcmGenetDxe.h"
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipGetInformation (
+  IN  EFI_ADAPTER_INFORMATION_PROTOCOL  *This,
+  IN  EFI_GUID  *InformationType,
+  OUT VOID  **InformationBlock,
+  OUT UINTN *InformationBlockSize
+  )
+{
+  EFI_ADAPTER_INFO_MEDIA_STATE  *AdapterInfo;
+  GENET_PRIVATE_DATA*Genet;
+
+  if (This == NULL || InformationBlock == NULL ||
+  InformationBlockSize == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (!CompareGuid (InformationType, )) {
+return EFI_UNSUPPORTED;
+  }
+
+  AdapterInfo = AllocateZeroPool (sizeof (EFI_ADAPTER_INFO_MEDIA_STATE));
+  if (AdapterInfo == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  *InformationBlock = AdapterInfo;
+  *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
+
+  Genet = GENET_PRIVATE_DATA_FROM_SNP_THIS (This);
+  if (Genet->Snp.Mode->MediaPresent) {
+AdapterInfo->MediaState = EFI_SUCCESS;
+  } else {
+AdapterInfo->MediaState = EFI_NOT_READY;
+  }
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipSetInformation (
+

Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings

2020-06-16 Thread Ard Biesheuvel


On 6/16/20 11:12 PM, Andrew Fish wrote:
I was wondering what would happen if we converted the CHAR8 arrays in to 
strings with multiple nulls [1]. Looks like it saves space in 
uncompressed FVs, but not in compressed FVs.




Yeah, those NUL bytes compress really well, so the overhead of the extra 
code you need to unpack your strings stands out in the compressed FV.




Here are the Xcode numbers for X64 DEBUG Ovmf:

With this Patch:
SECFV [14%Full] 212992 total, 30128 used, 182864 free
PEIFV [29%Full] 917504 total, 273192 used, 644312 free
DXEFV [39%Full] 12582912 total, 4997120 used, 7585792 free
FVMAIN_COMPACT [36%Full] 3440640 total, 1271264 used, 2169376 free

Vs my patch:
SECFV [14%Full] 212992 total, 29872 used, 183120 free
PEIFV [29%Full] 917504 total, 271048 used, 646456 free
DXEFV [39%Full] 12582912 total, 4979552 used, 7603360 free
FVMAIN_COMPACT [36%Full] 3440640 total, 1271824 used, 2168816 free

SEC: -256
PEI: -2,144
Dxe: -17,568
Compact: +560

[1] $ git diff
*diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c 
b/MdePkg/Library/BasePrintLib/PrintLibInternal.c*

*index 50c6e8559c43..db2533e7affb 100644*
*--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c*
*+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c*
@@ -30,53 +30,73 @@GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 mHexStr[] = 
{'0','1','2','3','4','5','

  //
  // Longest string: RETURN_WARN_BUFFER_TOO_SMALL => 24 characters plus 
NUL byte

  //
-GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 mWarningString[][24+1] = {
-  "Success",                      //  RETURN_SUCCESS                = 0
-  "Warning Unknown Glyph",        //  RETURN_WARN_UNKNOWN_GLYPH     = 1
-  "Warning Delete Failure",       //  RETURN_WARN_DELETE_FAILURE    = 2
-  "Warning Write Failure",        //  RETURN_WARN_WRITE_FAILURE     = 3
-  "Warning Buffer Too Small",     //  RETURN_WARN_BUFFER_TOO_SMALL  = 4
-  "Warning Stale Data",           //  RETURN_WARN_STALE_DATA        = 5
-};
+GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mWarningString = 
     \
+  "Success\0"                      /*  RETURN_SUCCESS                = 
0 */ \
+  "Warning Unknown Glyph\0"        /*  RETURN_WARN_UNKNOWN_GLYPH     = 
1 */ \
+  "Warning Delete Failure\0"       /*  RETURN_WARN_DELETE_FAILURE    = 
2 */ \
+  "Warning Write Failure\0"        /*  RETURN_WARN_WRITE_FAILURE     = 
3 */ \
+  "Warning Buffer Too Small\0"     /*  RETURN_WARN_BUFFER_TOO_SMALL  = 
4 */ \

+  "Warning Stale Data";            /*  RETURN_WARN_STALE_DATA        = 5 */


  //
  // Longest string: RETURN_INCOMPATIBLE_VERSION => 20 characters plus 
NUL byte

  //
-GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 mErrorString[][20+1] = {
-  "Load Error",                   //  RETURN_LOAD_ERROR             = 
1  | MAX_BIT
-  "Invalid Parameter",            //  RETURN_INVALID_PARAMETER      = 
2  | MAX_BIT
-  "Unsupported",                  //  RETURN_UNSUPPORTED            = 
3  | MAX_BIT
-  "Bad Buffer Size",              //  RETURN_BAD_BUFFER_SIZE        = 
4  | MAX_BIT
-  "Buffer Too Small",             //  RETURN_BUFFER_TOO_SMALL,      = 
5  | MAX_BIT
-  "Not Ready",                    //  RETURN_NOT_READY              = 
6  | MAX_BIT
-  "Device Error",                 //  RETURN_DEVICE_ERROR           = 
7  | MAX_BIT
-  "Write Protected",              //  RETURN_WRITE_PROTECTED        = 
8  | MAX_BIT
-  "Out of Resources",             //  RETURN_OUT_OF_RESOURCES       = 
9  | MAX_BIT
-  "Volume Corrupt",               //  RETURN_VOLUME_CORRUPTED       = 
10 | MAX_BIT
-  "Volume Full",                  //  RETURN_VOLUME_FULL            = 
11 | MAX_BIT
-  "No Media",                     //  RETURN_NO_MEDIA               = 
12 | MAX_BIT
-  "Media changed",                //  RETURN_MEDIA_CHANGED          = 
13 | MAX_BIT
-  "Not Found",                    //  RETURN_NOT_FOUND              = 
14 | MAX_BIT
-  "Access Denied",                //  RETURN_ACCESS_DENIED          = 
15 | MAX_BIT
-  "No Response",                  //  RETURN_NO_RESPONSE            = 
16 | MAX_BIT
-  "No mapping",                   //  RETURN_NO_MAPPING             = 
17 | MAX_BIT
-  "Time out",                     //  RETURN_TIMEOUT                = 
18 | MAX_BIT
-  "Not started",                  //  RETURN_NOT_STARTED            = 
19 | MAX_BIT
-  "Already started",              //  RETURN_ALREADY_STARTED        = 
20 | MAX_BIT
-  "Aborted",                      //  RETURN_ABORTED                = 
21 | MAX_BIT
-  "ICMP Error",                   //  RETURN_ICMP_ERROR             = 
22 | MAX_BIT
-  "TFTP Error",                   //  RETURN_TFTP_ERROR             = 
23 | MAX_BIT
-  "Protocol Error",               //  RETURN_PROTOCOL_ERROR         = 
24 | MAX_BIT
-  "Incompatible Version",         //  RETURN_INCOMPATIBLE_VERSION   = 
25 | MAX_BIT
-  "Security Violation",           //  RETURN_SECURITY_VIOLATION     = 
26 | MAX_BIT
-  "CRC Error",                    //  RETURN_CRC_ERROR              = 
27 | MAX_BIT
-  "End of Media",

Re: [edk2-devel] [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot options on boot failure

2020-06-16 Thread Samer El-Haj-Mahmoud

Reviewed-by: Samer El-Haj-Mahmoud 


> -Original Message-
> From: Ard Biesheuvel 
> Sent: Tuesday, June 16, 2020 1:49 PM
> To: devel@edk2.groups.io
> Cc: l...@nuviainc.com; Ard Biesheuvel ; Pete
> Batard ; Andrei Warkentin (awarken...@vmware.com)
> ; Samer El-Haj-Mahmoud  mahm...@arm.com>
> Subject: [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot
> options on boot failure
>
> One of the side effects of the recent changes to PlatformBootManagerLib
> changes to avoid connecting all devices on every boot is that we no longer
> default to network boot on a virgin boot, but end up in the UiApp menu. At
> this point, the autogenerated boot options that we used to rely on will be
> instantiated too, but it does break the unattended boot case where devices
> are expected to attempt a network boot on the very first power on.
>
> Let's work around this by refreshing all boot options explicitly in the
> UnableToBoot() handler, and rebooting the system if doing so resulted in a
> change to the total number of configured boot options.
> This way, we ultimately end up in the UiApp as before if no boot options
> could be started, but only after all the autogenerated ones have been
> attempted as well.
> Cc: Pete Batard Cc: Andrei Warkentin
> (awarken...@vmware.com) Cc: Samer El-Haj-
> Mahmoud Signed-off-by: Ard
> Biesheuvel 
> ---
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 34
> 
>  1 file changed, 34 insertions(+)
>
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 15c5cac1bea0..9905cad22908 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -820,6 +820,40 @@ PlatformBootManagerUnableToBoot (
>  {   EFI_STATUS   Status;   EFI_BOOT_MANAGER_LOAD_OPTION
> BootManagerMenu;+  EFI_BOOT_MANAGER_LOAD_OPTION
> *BootOptions;+  UINTNOldBootOptionCount;+  UINTN
> NewBootOptionCount;++  //+  // Record the total number of boot
> configured boot options+  //+  BootOptions =
> EfiBootManagerGetLoadOptions (,+
> LoadOptionTypeBoot);+  EfiBootManagerFreeLoadOptions (BootOptions,
> OldBootOptionCount);++  //+  // Connect all devices, and regenerate all boot
> options+  //+  EfiBootManagerConnectAll ();+
> EfiBootManagerRefreshAllBootOption ();++  //+  // Record the updated
> number of boot configured boot options+  //+  BootOptions =
> EfiBootManagerGetLoadOptions (,+
> LoadOptionTypeBoot);+  EfiBootManagerFreeLoadOptions (BootOptions,
> NewBootOptionCount);++  //+  // If the number of configured boot options
> has changed, reboot+  // the system so the new boot options will be taken
> into account+  // while executing the ordinary BDS bootflow sequence.+  //+
> if (NewBootOptionCount != OldBootOptionCount) {+DEBUG
> ((DEBUG_WARN, "%a: rebooting after refreshing all boot options\n",+
> __FUNCTION__));+gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0,
> NULL);+  }Status = EfiBootManagerGetBootManagerMenu
> ();   if (EFI_ERROR (Status)) {--
> 2.27.0

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61360): https://edk2.groups.io/g/devel/message/61360
Mute This Topic: https://groups.io/mt/74921613/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] Revert "OvmfPkg: use generic QEMU image loader for secure boot enabled builds"

2020-06-16 Thread Laszlo Ersek

On 06/15/20 16:49, Philippe Mathieu-Daudé wrote:
> On 6/15/20 4:45 PM, Laszlo Ersek wrote:
>> This reverts commit ced77332cab626f35fbdb36630be27303d289d79.
>>
>> The command
>>
>>   virt-install --location NETWORK-URL
>>
>> downloads the vmlinuz and initrd files from the remote OS tree, and passes
>> them to the guest firmware via fw_cfg.
>>
>> When used with IA32 / X64 guests, virt-install expects the guest firmware
>> to do two things, at the same time:
>>
>> - launch the fw_cfg kernel image even if the latter does not pass SB
>>   verification (SB checking is supposed to be bypassed entirely in favor
>>   of the Linux/x86 Boot Protocol),
>>
>> - still let the guest kernel perceive SB as enabled.
>>
>> Commit ced77332cab6 prevented this, by removing the Linux/x86 Boot
>> Protocol from such an OVMF image that was built with SECURE_BOOT_ENALBE.
>> While that's the right thing in theory, in practice "virt-install
>> --location NETWORK-URL" is entrenched, and we shouldn't break it.
>>
>> We can tolerate the Linux/x86 Boot Protocol as a one-of-a-kind SB bypass
>> for direct-booted kernels, because:
>>
>> - the fw_cfg content comes from QEMU, and the guest is already at QEMU's
>>   mercy,
>>
>> - in the guest, OS boots after the initial installation will use "shim"
>>   rather than an fw_cfg kernel, which we can consider somewhat similar to
>>   "Audit Mode / Deployed Mode" (~ trust for install, lock down after).
>>
>> Cc: Ard Biesheuvel 
>> Cc: Jordan Justen 
>> Cc: Philippe Mathieu-Daudé 
>> Signed-off-by: Laszlo Ersek 
>> Acked-by: Ard Biesheuvel 
>> ---
>>
>> Notes:
>> - pick up Ard's ACK from
>> 
>>   
>> c06ee730-e421-0aa5-882f-bc09ae9c546f@arm.com">http://mid.mail-archive.com/c06ee730-e421-0aa5-882f-bc09ae9c546f@arm.com
>>   https://edk2.groups.io/g/devel/message/61169
>> 
>> - posting to the list to enable feedback on the commit message (I intend
>>   to push the patch in one or two days)
>> 
>> - repo:   https://pagure.io/lersek/edk2.git
>>   branch: reenable_fwcfg_x86_boot_proto
>>
>>  OvmfPkg/OvmfPkgIa32.dsc| 4 
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 4 
>>  OvmfPkg/OvmfPkgX64.dsc | 4 
>>  3 files changed, 12 deletions(-)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index d0df9cbbfb2b..16103d177374 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -379,11 +379,7 @@ [LibraryClasses.common.DXE_DRIVER]
>>PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>>QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>> -  
>> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>> -!else
>>
>> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>> -!endif
>>  !if $(TPM_ENABLE) == TRUE
>>Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
>>Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index b3ae62fee92b..9597ef6721da 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER]
>>PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>>QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>> -  
>> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>> -!else
>>
>> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>> -!endif
>>  !if $(TPM_ENABLE) == TRUE
>>Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
>>Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index f7fe75ebf531..a6e585c03d41 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER]
>>PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>>QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>> -!if $(SECURE_BOOT_ENABLE) == TRUE
>> -  
>> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>> -!else
>>
>> QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
>> -!endif
>>  !if $(TPM_ENABLE) == TRUE
>>Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
>>Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
>>
> 
> Reviewed-by: Philippe Mathieu-Daude 

Thanks!

Merged as commit 82808b422617 ('Revert "OvmfPkg: use generic QEMU image
loader

[edk2-devel] [PATCH edk2-platforms v2 0/3] Silicon/NXP: LX2160A: Add SerDes Support

2020-06-16 Thread Wasim Khan

From: Wasim Khan 

NXP SoCs supports different Serdes protocols using reset
configuration word (RCW). 
Based on Serdes protocol value in reset configuration word (RCW)
different IP blocks gets enabled in HW.

This patch add Serdes support for LX2160A, which has 3 Serdes.

Changes in V2:
- Incorporate review comments to change SerDesConfigTable to STATIC
- Incorporate similar changes for LS1043
- Update Debug log in SerdesHelperLib

V1 series can be accessed from:
https://edk2.groups.io/g/devel/message/61056

 
Wasim Khan (3):
  Silicon/NXP: LX2160A: Add SerDes Support
  Silicon/NXP: LS1043A: Change SerDes ConfigTable to STATIC
  Silicon/NXP: SerDesHelperLib: Correct DEBUG log

 Silicon/NXP/LX2160A/LX2160A.dsc.inc   |   2 +
 Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf |   6 +
 Silicon/NXP/LX2160A/Include/SocSerDes.h   |  74 +++
 Silicon/NXP/LS1043A/Library/SocLib/SerDes.c   |   4 +-
 Silicon/NXP/LX2160A/Library/SocLib/SerDes.c   | 211 

 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c |   2 +-
 6 files changed, 296 insertions(+), 3 deletions(-)
 create mode 100644 Silicon/NXP/LX2160A/Include/SocSerDes.h
 create mode 100644 Silicon/NXP/LX2160A/Library/SocLib/SerDes.c

-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61357): https://edk2.groups.io/g/devel/message/61357
Mute This Topic: https://groups.io/mt/74923745/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 3/3] Silicon/NXP: SerDesHelperLib: Correct DEBUG log

2020-06-16 Thread Wasim Khan

From: Wasim Khan 

Update DEBUG log to indicate SerDes protocol
is in hex.

Signed-off-by: Wasim Khan 
---

Changes in V2:
- New Commit

 Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c 
b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
index ddefcc2fac98..1e8158541c83 100644
--- a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
+++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c
@@ -65,7 +65,7 @@ IsSerDesProtocolValid (
 
   while (Config->Protocol) {
 if (Config->Protocol == SerDesProtocol) {
-  DEBUG ((DEBUG_INFO, "Protocol: %x Matched with the one in Table\n", 
SerDesProtocol));
+  DEBUG ((DEBUG_INFO, "Protocol: 0x%x Matched with the one in Table\n", 
SerDesProtocol));
   break;
 }
 Config++;
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61356): https://edk2.groups.io/g/devel/message/61356
Mute This Topic: https://groups.io/mt/74923744/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 1/3] Silicon/NXP: LX2160A: Add SerDes Support

2020-06-16 Thread Wasim Khan

From: Wasim Khan 

Based on SerDes protocol value in reset configuration word (RCW)
different IP blocks gets enabled in HW.
Add SoC specific SerDes configuration for LX2160A, which can be
used by different IPs to know the enabled interfaces and perform
the required initialization.

Signed-off-by: Wasim Khan 
---

Changes in V2:
- Update SerDesConfigTable type and prefix with 'm'

 Silicon/NXP/LX2160A/LX2160A.dsc.inc   |   2 +
 Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf |   6 +
 Silicon/NXP/LX2160A/Include/SocSerDes.h   |  74 +++
 Silicon/NXP/LX2160A/Library/SocLib/SerDes.c   | 211 
 4 files changed, 293 insertions(+)

diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc 
b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
index af22b4dd973c..fe8ed402fc4e 100644
--- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
+++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
@@ -15,6 +15,7 @@ [LibraryClasses.common]
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
   PL011UartClockLib|Silicon/NXP/Library/PL011UartClockLib/PL011UartClockLib.inf
+  SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
 
 

 #
@@ -32,6 +33,7 @@ [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x23A
   gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x239
   gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|91
+  gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes|0x8
 
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C
diff --git a/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf 
b/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
index 421072b88019..54bcb82e6877 100644
--- a/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
+++ b/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
@@ -22,6 +22,12 @@ [Packages]
 [LibraryClasses]
   ChassisLib
   DebugLib
+  PcdLib
+  SerDesHelperLib
 
 [Sources.common]
+  SerDes.c
   SocLib.c
+
+[FixedPcd]
+  gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes
diff --git a/Silicon/NXP/LX2160A/Include/SocSerDes.h 
b/Silicon/NXP/LX2160A/Include/SocSerDes.h
new file mode 100644
index ..02000622d89a
--- /dev/null
+++ b/Silicon/NXP/LX2160A/Include/SocSerDes.h
@@ -0,0 +1,74 @@
+/** SocSerDes.h
+  SoC Specific header file for SerDes
+
+  Copyright 2017-2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SOC_SERDES_H
+#define SOC_SERDES_H
+
+typedef enum {
+  NONE = 0,
+  PCIE1,
+  PCIE2,
+  PCIE3,
+  PCIE4,
+  PCIE5,
+  PCIE6,
+  SATA1,
+  SATA2,
+  SATA3,
+  SATA4,
+  XFI1,
+  XFI2,
+  XFI3,
+  XFI4,
+  XFI5,
+  XFI6,
+  XFI7,
+  XFI8,
+  XFI9,
+  XFI10,
+  XFI11,
+  XFI12,
+  XFI13,
+  XFI14,
+  SGMII1,
+  SGMII2,
+  SGMII3,
+  SGMII4,
+  SGMII5,
+  SGMII6,
+  SGMII7,
+  SGMII8,
+  SGMII9,
+  SGMII10,
+  SGMII11,
+  SGMII12,
+  SGMII13,
+  SGMII14,
+  SGMII15,
+  SGMII16,
+  SGMII17,
+  SGMII18,
+  GE100_1,
+  GE100_2,
+  GE50_1,
+  GE50_2,
+  GE40_1,
+  GE40_2,
+  GE25_1,
+  GE25_2,
+  GE25_3,
+  GE25_4,
+  GE25_5,
+  GE25_6,
+  GE25_7,
+  GE25_8,
+  GE25_9,
+  GE25_10,
+  SERDES_PROTOCOL_COUNT
+} SERDES_PROTOCOL;
+#endif
diff --git a/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c 
b/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
new file mode 100644
index ..d02393b59d40
--- /dev/null
+++ b/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
@@ -0,0 +1,211 @@
+/** SerDes.c
+  Provides SoC specific SerDes interface
+
+  Copyright 2017-2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// SerDes Protocol Mask and Shift in Reset Configuration Word (RCW) Status 
Register
+#define SERDES1_PROTOCOL_MASK   0x001f
+#define SERDES1_PROTOCOL_SHIFT  16
+#define SERDES2_PROTOCOL_MASK   0x03E0
+#define SERDES2_PROTOCOL_SHIFT  21
+#define SERDES3_PROTOCOL_MASK   0x7C00
+#define SERDES3_PROTOCOL_SHIFT  26
+
+STATIC SERDES_CONFIG mSerDes1ConfigTable[] = {
+  {  1, { PCIE2, PCIE2, PCIE2, PCIE2, PCIE1, PCIE1, PCIE1, PCIE1 } },
+  {  2, { PCIE2, PCIE2, PCIE2, PCIE2, SGMII6, SGMII5, SGMII4, SGMII3 } },
+  {  3, { PCIE2, PCIE2, PCIE2, PCIE2, XFI6, XFI5, XFI4, XFI3 } },
+  {  4, { SGMII10, SGMII9, SGMII8, SGMII7, SGMII6, SGMII5, SGMII4, SGMII3 } },
+  {  5, { XFI10, XFI9, XFI8, XFI7, PCIE1, PCIE1, PCIE1, PCIE1} },
+  {  6, { SGMII10, SGMII9, SGMII8, SGMII7, SGMII6, SGMII5, XFI4, XFI3 } },
+  {  7, { SGMII10, SGMII9, SGMII8, SGMII7, XFI6, XFI5, XFI4, XFI3  } },
+  {  8, { XFI10, XFI9, XFI8, XFI7, XFI6, XFI5, XFI4, XFI3 } },
+  {  9, { SGMII10, SGMII9, SGMII8, PCIE2, SGMII6, SGMII5, SGMII4, PCIE1 } },
+  { 10, { XFI10, XFI9, XFI8, PCIE2, XFI6, XFI5, XFI4, PCIE1 } },
+  { 11, { SGMII10, SGMII9, PCIE2, PCIE2, SGMII6, SGMII5, PCIE1, PCIE1 } },
+  { 12, { SGMII10, SGMII9, PCIE2, PCIE2, PCIE1, PCIE1, PCIE1, PCIE1 }

[edk2-devel] [PATCH edk2-platforms v2 2/3] Silicon/NXP: LS1043A: Change SerDes ConfigTable to STATIC

2020-06-16 Thread Wasim Khan

From: Wasim Khan 

Change SerDes1ConfigTable type to STATIC and add module
prefix.

Signed-off-by: Wasim Khan 
---

Changes in V2:
- New Commit

 Silicon/NXP/LS1043A/Library/SocLib/SerDes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c 
b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
index b34da76eeac9..640e1ee4e392 100644
--- a/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
+++ b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c
@@ -19,7 +19,7 @@
 // SerDes1 Protocol Shift in Reset Configuration Word (RCW) Status Register
 #define SERDES1_PROTOCOL_SHIFT 16
 
-SERDES_CONFIG gSerDes1ConfigTable[] = {
+STATIC SERDES_CONFIG mSerDes1ConfigTable[] = {
   {0x1555, {XFI_FM1_MAC9, PCIE1, PCIE2, PCIE3 } },
   {0x2555, {SGMII_2500_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } },
   {0x4555, {QSGMII_FM1_A, PCIE1, PCIE2, PCIE3 } },
@@ -49,7 +49,7 @@ SERDES_CONFIG gSerDes1ConfigTable[] = {
 };
 
 SERDES_CONFIG *gSerDesConfig[] = {
-  gSerDes1ConfigTable
+  mSerDes1ConfigTable
 };
 
 /**
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61355): https://edk2.groups.io/g/devel/message/61355
Mute This Topic: https://groups.io/mt/74923743/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Silicon/SbsaQemu: Fix NOR flash RegionBaseAddress

2020-06-16 Thread Tanmay Jagdale

The EFI_FIMRWARE_VOLUME_HEADER is present at an offset in the
NOR flash of Sbsa QEMU model. Use the right RegionBaseAddress
so that the EFI firmware volume header can be found correctly.

Signed-off-by: Tanmay Jagdale 
---
 .../SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c  | 2 +-
 .../Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git 
a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c 
b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c
index e7bb626596..0946327cb5 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c
@@ -23,7 +23,7 @@ NorFlashPlatformInitialization (
 NOR_FLASH_DESCRIPTION mNorFlashDevice =
 {
  FixedPcdGet64(PcdFdBaseAddress),
- FixedPcdGet64(PcdFdBaseAddress),
+ FixedPcdGet64(PcdFlashNvStorageVariableBase),
  FixedPcdGet32(PcdFdSize),
  QEMU_NOR_BLOCK_SIZE
 };
diff --git 
a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf 
b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf
index 82712c8901..f2ba41e1df 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf
@@ -23,7 +23,9 @@
   ArmPlatformPkg/ArmPlatformPkg.dec
   ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
 
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFdBaseAddress
   gArmTokenSpaceGuid.PcdFdSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
-- 
2.27.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61353): https://edk2.groups.io/g/devel/message/61353
Mute This Topic: https://groups.io/mt/74923728/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg: Add New Memory Attributes

2020-06-16 Thread Oleksiy Yakovlev

Hi Laszlo.

There was separate patch for MdeModulePkg submitted with this one as well. I 
did not include you into cc, because you are not in the maintainers list for 
MdeModulePkg. Patch addresses EFI_MEMORY_(RO|RP|XP) macroses and all issues you 
mentioned in your comment in same way as UefiCpuPkg patch (without introducing 
single macros and using MEMORY_ATTRIBUTE_MASK instead of | ).

Regards, Oleksiy.

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, June 16, 2020 2:12 PM
To: Oleksiy Yakovlev; devel@edk2.groups.io
Cc: eric.d...@intel.com; ray...@intel.com; rahul1.ku...@intel.com; Felix 
Polyudov; Liming Gao; Michael Kinney; Jian J Wang; Hao A Wu; Dandan Bi
Subject: Re: [PATCH] UefiCpuPkg: Add New Memory Attributes

Hi Oleksiy,

On 06/15/20 23:45, Oleksiy Yakovlev wrote:
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
> attributes introduced in UEFI 2.8
> (UEFI 2.8, mantis 1919 and 1872)
>
> Signed-off-by: Oleksiy Yakovlev 
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.c | 2 +-
>  UefiCpuPkg/CpuDxe/CpuDxe.h | 4 +++-
>  UefiCpuPkg/CpuDxe/CpuPageTable.c   | 2 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 2 +-
>  4 files changed, 6 insertions(+), 4 deletions(-)

I suggest / request turning this patch into 5 patches:

(a) The first patch should please correct a mistake in commit
c18708d2f002 ("MdePkg-UefiSpec.h: Add UEFI 2.8 new memory attributes",
2019-11-04).

Namely, in commit c18708d2f002, the EFI_MEMORY_CPU_CRYPTO macro's
documentation includes the string "CPU?s", twice, in place of "CPU's".

I don't understand how this happened. In the mailing list archive, I can
only find Liming's confirmation that he pushed the patch:

  https://edk2.groups.io/g/devel/message/49893

but not the original patch posting.

Note that, in the context quoted in that message (that is, the patch),
the string was "CPU’s". That string did not use ASCII character 0x27,
but U+2019 (RIGHT SINGLE QUOTATION MARK). So indeed the patch was
incorrect. But the solution should not have been to replace U+2019 with
"?", but to request a repost using ASCII 0x27.

Either way, even though it is obviously not your mistake, can you please
include a patch for replacing both "CPU?s" instances with "CPU's"? In
file "MdePkg/Include/Uefi/UefiSpec.h".

For the rest of the patches, please consider:

$ git grep -E 'EFI_MEMORY_(RO|RP|XP) \| EFI_MEMORY_(RO|RP|XP) \| 
EFI_MEMORY_(RO|RP|XP)'

MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c:#define MEMORY_ATTRIBUTE_MASK  
(EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c:#define MEMORY_PAGE_ATTRIBUTES  
(EFI_MEMORY_XP | EFI_MEMORY_RP | EFI_MEMORY_RO)
UefiCpuPkg/CpuDxe/CpuDxe.c:#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | 
EFI_MEMORY_XP | EFI_MEMORY_RO)
UefiCpuPkg/CpuDxe/CpuPageTable.c:  if ((Attributes & ~(EFI_MEMORY_RP | 
EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) {
UefiCpuPkg/CpuDxe/CpuPageTable.c:Capabilities = EFI_MEMORY_RO | 
EFI_MEMORY_RP | EFI_MEMORY_XP;
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c:  ASSERT ((Attributes & 
~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0);

This output tells us the following:

- the bitmask (RP|XP|RO) is *triplicated* between the macro
  definitions in:

  - MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
  - MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
  - UefiCpuPkg/CpuDxe/CpuDxe.c

- "UefiCpuPkg/CpuDxe/CpuPageTable.c" open-codes the bitmask in two
  separate spots (rather than using MEMORY_ATTRIBUTE_MASK)

- "UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c" open codes the
  bitmask also (rather than using any macro).

(b) Therefore, the second patch should introduce a central macro for
(RP|XP|RO) somewhere under MdePkg or MdeModulePkg. Perhaps it can even
be a fixed-only PCD.

(c) The third patch should replace all of the open coded bitmasks in
MdeModulePkg (see the list above) with references to the new central
macro (or PCD).

(d) The fourth patch should do the same in UefiCpuPkg.

(e) The final patch should modify the central macro to include
EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO.

This is just my opinion of course, please discuss it further with the
MdePkg / MdeModulePkg / UefiCpuPkg maintainers (I've CC'd them).

Thanks,
Laszlo

>
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index a571fc3..55ca764 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -11,7 +11,7 @@
>  #include "CpuPageTable.h"
>
>  #define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | 
> EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> -#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | 
> EFI_MEMORY_RO)
> +#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | 
> EFI_MEMORY_RO | EFI_MEMORY_SP | EFI_MEMORY_CPU_CRYPTO)
>
>  //
>  // Global Variables
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/NXP: LX2160A: Add SerDes Support

2020-06-16 Thread Wasim Khan (OSS)




> -Original Message-
> From: Leif Lindholm 
> Sent: Tuesday, June 16, 2020 8:50 PM
> To: Wasim Khan (OSS) 
> Cc: devel@edk2.groups.io; Meenakshi Aggarwal
> ; Varun Sethi ;
> ard.biesheu...@arm.com; Wasim Khan 
> Subject: Re: [PATCH edk2-platforms 1/1] Silicon/NXP: LX2160A: Add SerDes
> Support
> 
> On Wed, Jun 10, 2020 at 02:38:54 +0530, Wasim Khan wrote:
> > From: Wasim Khan 
> >
> > Based on SerDes protocol value in reset configuration word (RCW)
> > different IP blocks gets enabled in HW.
> > Add SoC specific SerDes configuration for LX2160A, which can be used
> > by different IPs to know the enabled interfaces and perform the
> > required initialization.
> >
> > Signed-off-by: Wasim Khan 
> > ---
> >  Silicon/NXP/LX2160A/LX2160A.dsc.inc   |   2 +
> >  Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf |   6 +
> >  Silicon/NXP/LX2160A/Include/SocSerDes.h   |  74 +++
> >  Silicon/NXP/LX2160A/Library/SocLib/SerDes.c   | 211 
> >  4 files changed, 293 insertions(+)
> >
> > diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > index af22b4dd973c..fe8ed402fc4e 100644
> > --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> > @@ -15,6 +15,7 @@ [LibraryClasses.common]
> >PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> >
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> >
> > PL011UartClockLib|Silicon/NXP/Library/PL011UartClockLib/PL011UartClock
> > Lib.inf
> > +
> > + SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.
> > + inf
> >
> >
> >
> #
> #
> > ##
> >  #
> > @@ -32,6 +33,7 @@ [PcdsFixedAtBuild.common]
> >gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x23A
> >gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x239
> >gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|91
> > +  gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes|0x8
> >
> >gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> >gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C
> > diff --git a/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> > b/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> > index 421072b88019..54bcb82e6877 100644
> > --- a/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> > +++ b/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> > @@ -22,6 +22,12 @@ [Packages]
> >  [LibraryClasses]
> >ChassisLib
> >DebugLib
> > +  PcdLib
> > +  SerDesHelperLib
> >
> >  [Sources.common]
> > +  SerDes.c
> >SocLib.c
> > +
> > +[FixedPcd]
> > +  gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes
> > diff --git a/Silicon/NXP/LX2160A/Include/SocSerDes.h
> > b/Silicon/NXP/LX2160A/Include/SocSerDes.h
> > new file mode 100644
> > index ..02000622d89a
> > --- /dev/null
> > +++ b/Silicon/NXP/LX2160A/Include/SocSerDes.h
> > @@ -0,0 +1,74 @@
> > +/** SocSerDes.h
> > +  SoC Specific header file for SerDes
> > +
> > +  Copyright 2017-2020 NXP
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> > +
> > +#ifndef SOC_SERDES_H
> > +#define SOC_SERDES_H
> > +
> > +typedef enum {
> > +  NONE = 0,
> > +  PCIE1,
> > +  PCIE2,
> > +  PCIE3,
> > +  PCIE4,
> > +  PCIE5,
> > +  PCIE6,
> > +  SATA1,
> > +  SATA2,
> > +  SATA3,
> > +  SATA4,
> > +  XFI1,
> > +  XFI2,
> > +  XFI3,
> > +  XFI4,
> > +  XFI5,
> > +  XFI6,
> > +  XFI7,
> > +  XFI8,
> > +  XFI9,
> > +  XFI10,
> > +  XFI11,
> > +  XFI12,
> > +  XFI13,
> > +  XFI14,
> > +  SGMII1,
> > +  SGMII2,
> > +  SGMII3,
> > +  SGMII4,
> > +  SGMII5,
> > +  SGMII6,
> > +  SGMII7,
> > +  SGMII8,
> > +  SGMII9,
> > +  SGMII10,
> > +  SGMII11,
> > +  SGMII12,
> > +  SGMII13,
> > +  SGMII14,
> > +  SGMII15,
> > +  SGMII16,
> > +  SGMII17,
> > +  SGMII18,
> > +  GE100_1,
> > +  GE100_2,
> > +  GE50_1,
> > +  GE50_2,
> > +  GE40_1,
> > +  GE40_2,
> > +  GE25_1,
> > +  GE25_2,
> > +  GE25_3,
> > +  GE25_4,
> > +  GE25_5,
> > +  GE25_6,
> > +  GE25_7,
> > +  GE25_8,
> > +  GE25_9,
> > +  GE25_10,
> > +  SERDES_PROTOCOL_COUNT
> > +} SERDES_PROTOCOL;
> > +#endif
> > diff --git a/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
> > b/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
> > new file mode 100644
> > index ..58f2b3df3600
> > --- /dev/null
> > +++ b/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
> > @@ -0,0 +1,211 @@
> > +/** SerDes.c
> > +  Provides SoC specific SerDes interface
> > +
> > +  Copyright 2017-2020 NXP
> > +
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +// SerDes Protocol Mask and Shift in Reset Configuration Word (RCW) Status
> Register
> > +#define SERDES1_PROTOCOL_MASK   0x001f
> > +#define SERDES1_PROTOCOL_SHIFT  16
> > +#define SERDES2_PROTOCOL_MASK   0x03E0
> > +#define SERDES2_PROTOCOL_SHIFT  21
> > +#define SERDES3_PROTOCOL_MASK

Re: [edk2-devel] [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load

2020-06-16 Thread Laszlo Ersek

(+Paolo, +Ray)

On 06/16/20 04:49, Igor Druzhinin wrote:
> RestoreTPL called while at TPL_HIGH_LEVEL unconditionally enables interrupts
> even if called in interrupt handler. That opens a window while interrupt
> is not completely handled but another interrupt could be accepted.
> 
> If a VM starts on a heavily loaded host hundreds of periodic timer interrupts
> might be queued while vCPU is descheduled (the behavior is typical for
> a Xen host). The next time vCPU is scheduled again all of them get
> delivered back to back causing OVMF to accept each one without finishing
> a previous one and cleaning up the stack. That quickly results in stack
> overflow and a triple fault.
> 
> Fix it by postponing sending EOI until we finished processing the current
> tick giving interrupt handler opportunity to clean up the stack before
> accepting the next tick.
> 
> Signed-off-by: Igor Druzhinin 
> ---
> 
> Laszlo, Anthony,
> 
> Do you think it's the right way to address it?
> 
> Alternatively, we might avoid calling RaiseTPL in interrupt handler at all
> like it's done in HpetTimer implementation for instance.
> 
> Or we might try to address it in Raise/RestoreTPL calls by saving/restoring
> interrupt state along with TPL.
> 
> ---
>  OvmfPkg/8254TimerDxe/Timer.c  | 5 +++--
>  OvmfPkg/XenTimerDxe/XenTimerDxe.c | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)

If I understand correctly, TimerInterruptHandler()
[OvmfPkg/8254TimerDxe/Timer.c] currently does the following:

- RaiseTPL (TPL_HIGH_LEVEL) --> mask interrupts from being delivered

- mLegacy8259->EndOfInterrupt() --> permit the PIC to generate further
interrupts (= make them pending)

- RestoreTPL() --> unmask interrupts (allow delivery)

RestoreTPL() is always expected to invoke handlers (on its own stack)
that have just been unmasked, so that behavior is not unexpected, in my
opinion.

What seems unexpected is the queueing of a huge number of timer
interrupts. I would think a timer interrupt is either pending or not
pending (i.e. if it's already pending, then the next generated interrupt
is coalesced, not queued). While there would still be a window between
the EOI and the unmasking, I don't think it would normally allow for a
*huge* number of queued interrupts (and consequently a stack overflow).

So I basically see the root of the problem in the interrupts being
queued rather than coalesced. I'm pretty unfamiliar with this x86 area
(= the 8259 PIC in general), but the following wiki article seems to
agree with my suspicion:

https://wiki.osdev.org/8259_PIC#How_does_the_8259_PIC_chip_work.3F

[...] and whether there's an interrupt already pending. If the
channel is unmasked and there's no interrupt pending, the PIC will
raise the interrupt line [...]

Can we say that the interrupt queueing (as opposed to coalescing) is a
Xen issue?

(Hmmm... maybe the hypervisor *has* to queue the timer interrupts,
otherwise some of them would simply be lost, and the guest would lose
track of time.)

Either way, I'm not sure what the best approach is. This driver was
moved under OvmfPkg from PcAtChipsetPkg in commit 1a3ffdff82e6
("OvmfPkg: Copy 8254TimerDxe driver from PcAtChipsetPkg", 2019-04-11).
HpetTimerDxe also lives under PcAtChipsetPkg.

So I think I'll have to rely on the expertise of Ray here (CC'd).

Also, I recall a recent-ish QEMU commit that seems vaguely related
(i.e., to timer interrupt coalescing -- see 7a3e29b12f5a, "mc146818rtc:
fix timer interrupt reinjection again", 2019-11-19), so I'm CC'ing Paolo
too.

Some more comments / questions below:

> 
> diff --git a/OvmfPkg/8254TimerDxe/Timer.c b/OvmfPkg/8254TimerDxe/Timer.c
> index 67e22f5..fd1691b 100644
> --- a/OvmfPkg/8254TimerDxe/Timer.c
> +++ b/OvmfPkg/8254TimerDxe/Timer.c
> @@ -79,8 +79,6 @@ TimerInterruptHandler (
>  
>OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>  
> -  mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
> -
>if (mTimerNotifyFunction != NULL) {
>  //
>  // @bug : This does not handle missed timer interrupts
> @@ -89,6 +87,9 @@ TimerInterruptHandler (
>}
>  
>gBS->RestoreTPL (OriginalTPL);
> +
> +  DisableInterrupts ();
> +  mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
>  }

So this briefly (temporarily) unmasks interrupt delivery (between
RestoreTPL() and DisableInterrupts()) while the PIC is still blocked
from generating more, and then unblocks the PIC.

It looks plausible for preventing the unbounded recursion per se, but
why is it safe to leave the function with interrupts disabled? Before
the patch, that didn't use to be the case.

I'm generally concerned about such an "indiscriminate" 8254TimerDxe
patch, because I haven't seen a similar report on QEMU/KVM yet. And
again I'm quite out of my depth with the 8259 / 8254. Even if we end up
merging a patch like this, I feel tempted to restrict it to Xen.

... Regarding XenTimerDxe, I don't have the slightest idea, unfortunately.

Thanks
Laszlo

>  
>  /**
> diff

[edk2-devel] [PATCH edk2-platforms] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol

2020-06-16 Thread Ard Biesheuvel

NetLibDetectMedia () in DxeNetLib is used as a fallback on implementations
of the SNP protocol that do not also carry an implementation of the adapter
info protocol to provide media state information, and it does all kinds of
terrible things to the network interface (stopping and restarting multiple
times, reprogramming the multicast filters etc etc) to workaround some
alleged UNDI shortcoming.

Although our GENET code should be bullet proof and therefore able to take
this kind of abuse, it is better to avoid it, and provide an implementation
of the adapter info protocol that returns the media state directly, without
the need to mistreat the SNP layer.

Cc: Pete Batard 
Cc: Andrei Warkentin (awarken...@vmware.com) 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Ard Biesheuvel 
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf |   3 +
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h   |   4 +
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c   | 109 

 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c |  12 ++-
 4 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
index 28f3e66ebaf0..89dee9f10c83 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
@@ -19,6 +19,7 @@ [Defines]
   UNLOAD_IMAGE   = GenetUnload
 
 [Sources]
+  AdapterInfo.c
   BcmGenetDxe.h
   ComponentName.c
   DriverBinding.c
@@ -49,10 +50,12 @@ [LibraryClasses]
 
 [Protocols]
   gBcmGenetPlatformDeviceProtocolGuid ## TO_START
+  gEfiAdapterInformationProtocolGuid  ## BY_START
   gEfiDevicePathProtocolGuid  ## BY_START
   gEfiSimpleNetworkProtocolGuid   ## BY_START
 
 [Guids]
+  gEfiAdapterInfoMediaStateGuid
   gEfiEventExitBootServicesGuid
 
 [FixedPcd]
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
index 0af9d5209cf2..48bb8550426f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -209,6 +210,8 @@ typedef struct {
   EFI_SIMPLE_NETWORK_PROTOCOL Snp;
   EFI_SIMPLE_NETWORK_MODE SnpMode;
 
+  EFI_ADAPTER_INFORMATION_PROTOCOLAip;
+
   BCM_GENET_PLATFORM_DEVICE_PROTOCOL  *Dev;
 
   GENERIC_PHY_PRIVATE_DATAPhy;
@@ -234,6 +237,7 @@ extern EFI_COMPONENT_NAME_PROTOCOL
gGenetComponentName;
 extern EFI_COMPONENT_NAME2_PROTOCOL   gGenetComponentName2;
 
 extern CONST EFI_SIMPLE_NETWORK_PROTOCOL  gGenetSimpleNetworkTemplate;
+extern CONST EFI_ADAPTER_INFORMATION_PROTOCOL gGenetAdapterInfoTemplate;
 
 #define GENET_DRIVER_SIGNATURESIGNATURE_32('G', 'N', 'E', 'T')
 #define GENET_PRIVATE_DATA_FROM_SNP_THIS(a)   CR(a, GENET_PRIVATE_DATA, Snp, 
GENET_DRIVER_SIGNATURE)
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
new file mode 100644
index ..cb7733bbba76
--- /dev/null
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
@@ -0,0 +1,109 @@
+/** @file
+
+  Copyright (c) 2020 Arm, Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+#include "BcmGenetDxe.h"
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipGetInformation (
+  IN  EFI_ADAPTER_INFORMATION_PROTOCOL  *This,
+  IN  EFI_GUID  *InformationType,
+  OUT VOID  **InformationBlock,
+  OUT UINTN *InformationBlockSize
+  )
+{
+  EFI_ADAPTER_INFO_MEDIA_STATE  *AdapterInfo;
+  GENET_PRIVATE_DATA*Genet;
+
+  if (This == NULL || InformationBlock == NULL ||
+  InformationBlockSize == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (!CompareGuid (InformationType, )) {
+return EFI_UNSUPPORTED;
+  }
+
+  AdapterInfo = AllocateZeroPool (sizeof (EFI_ADAPTER_INFO_MEDIA_STATE));
+  if (AdapterInfo == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  *InformationBlock = AdapterInfo;
+  *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
+
+  Genet = GENET_PRIVATE_DATA_FROM_SNP_THIS (This);
+  if (Genet->Snp.Mode->MediaPresent) {
+AdapterInfo->MediaState = EFI_SUCCESS;
+  } else {
+AdapterInfo->MediaState = EFI_NOT_READY;
+  }
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+GenetAipSetInformation (
+  IN  EFI_ADAPTER_INFORMATION_PROTOCOL  *This,
+  IN  EFI_GUID  *InformationType,
+  IN  VOID  *InformationBlock,
+  IN  UINTN InformationBlockSize
+  )
+{
+  if (This == NULL || InformationBlock == NULL) {
+

Re: [edk2-devel] [PATCH] UefiCpuPkg: Add New Memory Attributes

2020-06-16 Thread Laszlo Ersek

Hi Oleksiy,

On 06/15/20 23:45, Oleksiy Yakovlev wrote:
> Add usage of EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO
> attributes introduced in UEFI 2.8
> (UEFI 2.8, mantis 1919 and 1872)
>
> Signed-off-by: Oleksiy Yakovlev 
> ---
>  UefiCpuPkg/CpuDxe/CpuDxe.c | 2 +-
>  UefiCpuPkg/CpuDxe/CpuDxe.h | 4 +++-
>  UefiCpuPkg/CpuDxe/CpuPageTable.c   | 2 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 2 +-
>  4 files changed, 6 insertions(+), 4 deletions(-)

I suggest / request turning this patch into 5 patches:

(a) The first patch should please correct a mistake in commit
c18708d2f002 ("MdePkg-UefiSpec.h: Add UEFI 2.8 new memory attributes",
2019-11-04).

Namely, in commit c18708d2f002, the EFI_MEMORY_CPU_CRYPTO macro's
documentation includes the string "CPU?s", twice, in place of "CPU's".

I don't understand how this happened. In the mailing list archive, I can
only find Liming's confirmation that he pushed the patch:

  https://edk2.groups.io/g/devel/message/49893

but not the original patch posting.

Note that, in the context quoted in that message (that is, the patch),
the string was "CPU’s". That string did not use ASCII character 0x27,
but U+2019 (RIGHT SINGLE QUOTATION MARK). So indeed the patch was
incorrect. But the solution should not have been to replace U+2019 with
"?", but to request a repost using ASCII 0x27.

Either way, even though it is obviously not your mistake, can you please
include a patch for replacing both "CPU?s" instances with "CPU's"? In
file "MdePkg/Include/Uefi/UefiSpec.h".

For the rest of the patches, please consider:

$ git grep -E 'EFI_MEMORY_(RO|RP|XP) \| EFI_MEMORY_(RO|RP|XP) \| 
EFI_MEMORY_(RO|RP|XP)'

MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c:#define MEMORY_ATTRIBUTE_MASK  
(EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c:#define MEMORY_PAGE_ATTRIBUTES  
(EFI_MEMORY_XP | EFI_MEMORY_RP | EFI_MEMORY_RO)
UefiCpuPkg/CpuDxe/CpuDxe.c:#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | 
EFI_MEMORY_XP | EFI_MEMORY_RO)
UefiCpuPkg/CpuDxe/CpuPageTable.c:  if ((Attributes & ~(EFI_MEMORY_RP | 
EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) {
UefiCpuPkg/CpuDxe/CpuPageTable.c:Capabilities = EFI_MEMORY_RO | 
EFI_MEMORY_RP | EFI_MEMORY_XP;
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c:  ASSERT ((Attributes & 
~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0);

This output tells us the following:

- the bitmask (RP|XP|RO) is *triplicated* between the macro
  definitions in:

  - MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
  - MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
  - UefiCpuPkg/CpuDxe/CpuDxe.c

- "UefiCpuPkg/CpuDxe/CpuPageTable.c" open-codes the bitmask in two
  separate spots (rather than using MEMORY_ATTRIBUTE_MASK)

- "UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c" open codes the
  bitmask also (rather than using any macro).

(b) Therefore, the second patch should introduce a central macro for
(RP|XP|RO) somewhere under MdePkg or MdeModulePkg. Perhaps it can even
be a fixed-only PCD.

(c) The third patch should replace all of the open coded bitmasks in
MdeModulePkg (see the list above) with references to the new central
macro (or PCD).

(d) The fourth patch should do the same in UefiCpuPkg.

(e) The final patch should modify the central macro to include
EFI_MEMORY_SP and EFI_MEMORY_CPU_CRYPTO.

This is just my opinion of course, please discuss it further with the
MdePkg / MdeModulePkg / UefiCpuPkg maintainers (I've CC'd them).

Thanks,
Laszlo

>
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index a571fc3..55ca764 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -11,7 +11,7 @@
>  #include "CpuPageTable.h"
>
>  #define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | 
> EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> -#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | 
> EFI_MEMORY_RO)
> +#define MEMORY_ATTRIBUTE_MASK  (EFI_MEMORY_RP | EFI_MEMORY_XP | 
> EFI_MEMORY_RO | EFI_MEMORY_SP | EFI_MEMORY_CPU_CRYPTO)
>
>  //
>  // Global Variables
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index 9299eaa..37fb38e 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -48,7 +48,9 @@
>
>  #define EFI_MEMORY_PAGETYPE_MASK  (EFI_MEMORY_RP  | \
> EFI_MEMORY_XP  | \
> -   EFI_MEMORY_RO\
> +   EFI_MEMORY_RO  | \
> +   EFI_MEMORY_SP  | \
> +   EFI_MEMORY_CPU_CRYPTO \
> )
>
>  #define HEAP_GUARD_NONSTOP_MODE   \
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c 
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 0a02cb3..d769e4a 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++

[edk2-devel] [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot options on boot failure

2020-06-16 Thread Ard Biesheuvel

One of the side effects of the recent changes to PlatformBootManagerLib
changes to avoid connecting all devices on every boot is that we no
longer default to network boot on a virgin boot, but end up in the
UiApp menu. At this point, the autogenerated boot options that we used
to rely on will be instantiated too, but it does break the unattended
boot case where devices are expected to attempt a network boot on the
very first power on.

Let's work around this by refreshing all boot options explicitly in
the UnableToBoot() handler, and rebooting the system if doing so
resulted in a change to the total number of configured boot options.
This way, we ultimately end up in the UiApp as before if no boot
options could be started, but only after all the autogenerated ones
have been attempted as well.

Cc: Pete Batard 
Cc: Andrei Warkentin (awarken...@vmware.com) 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 34 
 1 file changed, 34 insertions(+)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c 
b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 15c5cac1bea0..9905cad22908 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -820,6 +820,40 @@ PlatformBootManagerUnableToBoot (
 {
   EFI_STATUS   Status;
   EFI_BOOT_MANAGER_LOAD_OPTION BootManagerMenu;
+  EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
+  UINTNOldBootOptionCount;
+  UINTNNewBootOptionCount;
+
+  //
+  // Record the total number of boot configured boot options
+  //
+  BootOptions = EfiBootManagerGetLoadOptions (,
+  LoadOptionTypeBoot);
+  EfiBootManagerFreeLoadOptions (BootOptions, OldBootOptionCount);
+
+  //
+  // Connect all devices, and regenerate all boot options
+  //
+  EfiBootManagerConnectAll ();
+  EfiBootManagerRefreshAllBootOption ();
+
+  //
+  // Record the updated number of boot configured boot options
+  //
+  BootOptions = EfiBootManagerGetLoadOptions (,
+  LoadOptionTypeBoot);
+  EfiBootManagerFreeLoadOptions (BootOptions, NewBootOptionCount);
+
+  //
+  // If the number of configured boot options has changed, reboot
+  // the system so the new boot options will be taken into account
+  // while executing the ordinary BDS bootflow sequence.
+  //
+  if (NewBootOptionCount != OldBootOptionCount) {
+DEBUG ((DEBUG_WARN, "%a: rebooting after refreshing all boot options\n",
+  __FUNCTION__));
+gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
+  }
 
   Status = EfiBootManagerGetBootManagerMenu ();
   if (EFI_ERROR (Status)) {
-- 
2.27.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61347): https://edk2.groups.io/g/devel/message/61347
Mute This Topic: https://groups.io/mt/74921613/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1] EdkRepo: Fixed missing parameter in error string.

2020-06-16 Thread Nate DeSimone

Pushed: https://github.com/tianocore/edk2-staging/commit/3e6b805a

-Original Message-
From: Bjorge, Erik C  
Sent: Wednesday, June 10, 2020 3:07 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E ; Desimone, Nathaniel L 
; Pandya, Puja ; Bret 
Barkelew ; Agyeman, Prince 

Subject: [edk2-staging/EdkRepo] [PATCH v1] EdkRepo: Fixed missing parameter in 
error string.

Cc: Ashley E Desimone 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
Cc: Erik Bjorge 
Signed-off-by: Erik Bjorge 
---
 edkrepo/commands/checkout_command.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/edkrepo/commands/checkout_command.py 
b/edkrepo/commands/checkout_command.py
index f4483d4..0169f30 100644
--- a/edkrepo/commands/checkout_command.py
+++ b/edkrepo/commands/checkout_command.py
@@ -44,4 +44,4 @@ class CheckoutCommand(EdkrepoCommand):
 if combination_is_in_manifest(args.Combination, 
get_workspace_manifest()):

 checkout(args.Combination, args.verbose, args.override)

 else:

-raise EdkrepoInvalidParametersException(humble.NO_COMBO)

+raise 
EdkrepoInvalidParametersException(humble.NO_COMBO.format(args.Combination))

-- 
2.27.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61346): https://edk2.groups.io/g/devel/message/61346
Mute This Topic: https://groups.io/mt/74807241/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1] EdkRepo: Fixed missing parameter in error string.

2020-06-16 Thread Nate DeSimone

Reviewed-by: Nate DeSimone 

-Original Message-
From: Bjorge, Erik C  
Sent: Wednesday, June 10, 2020 3:07 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E ; Desimone, Nathaniel L 
; Pandya, Puja ; Bret 
Barkelew ; Agyeman, Prince 

Subject: [edk2-staging/EdkRepo] [PATCH v1] EdkRepo: Fixed missing parameter in 
error string.

Cc: Ashley E Desimone 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
Cc: Erik Bjorge 
Signed-off-by: Erik Bjorge 
---
 edkrepo/commands/checkout_command.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/edkrepo/commands/checkout_command.py 
b/edkrepo/commands/checkout_command.py
index f4483d4..0169f30 100644
--- a/edkrepo/commands/checkout_command.py
+++ b/edkrepo/commands/checkout_command.py
@@ -44,4 +44,4 @@ class CheckoutCommand(EdkrepoCommand):
 if combination_is_in_manifest(args.Combination, 
get_workspace_manifest()):

 checkout(args.Combination, args.verbose, args.override)

 else:

-raise EdkrepoInvalidParametersException(humble.NO_COMBO)

+raise 
EdkrepoInvalidParametersException(humble.NO_COMBO.format(args.Combination))

-- 
2.27.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61345): https://edk2.groups.io/g/devel/message/61345
Mute This Topic: https://groups.io/mt/74807241/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE

2020-06-16 Thread Laszlo Ersek

Hi Garrett,

On 06/15/20 20:30, Garrett Kirkendall wrote:
> AMD processor does not support MSR_IA32_MISC_ENABLE register.  Accessing
> this register on AMD causes an unhandled exception in SmmEntry.nasm and
> a subsequent failure to boot since this is too early in SMM path for the
> exception handler to be loaded.
> 
> First, to distinguish between AMD and other processors, refactor
> StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only
> one copy in the source. All changed modules already include UefiCpuLib
> either directly or indirectly so could not easly split first patch.
> 
> Second, Skip manipulation of MSR_IA32_MISC_ENABLE register if running
> on an AMD processor.
> 
> Tested on AMD X64 processor.
> 
> Modified source patching in 2/2 for FALSE
> and TRUE to test failure and passing case when AMD processor detected.
> Did not have a way to test UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm.

Please test the series as follows:

(1) Download

https://www.kraxel.org/repos/images/fedora-30-efi-systemd-i686.qcow2.xz
https://www.kraxel.org/repos/images/fedora-31-efi-grub2-x86_64.qcow2.xz

and decompress both files.


(2) Test using a 32-bit guest on an Intel host (standing in your edk2 tree, 
with the patches applied):

$ build -a IA32 -b DEBUG -p OvmfPkg/OvmfPkgIa32.dsc -t GCC5 -D SMM_REQUIRE

$ qemu-system-i386 \
-cpu coreduo,-nx \
-machine q35,smm=on,accel=kvm \
-m 4096 \
-smp 4 \
-global driver=cfi.pflash01,property=secure,value=on \
-drive 
if=pflash,format=raw,unit=0,readonly=on,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_CODE.fd
 \
-drive 
if=pflash,format=raw,unit=1,snapshot=on,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_VARS.fd
 \
-drive 
id=hdd,if=none,format=qcow2,snapshot=on,file=fedora-30-efi-systemd-i686.qcow2 \
-device virtio-scsi-pci,id=scsi0 \
-device scsi-hd,drive=hdd,bus=scsi0.0,bootindex=1

(Once you get a login prompt, feel free to interrupt QEMU with Ctrl-C.)


(3) Test using a 64-bit guest on an Intel host:

$ build -a IA32 -a X64 -b DEBUG -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -D 
SMM_REQUIRE

$ qemu-system-x86_64 \
-cpu host \
-machine q35,smm=on,accel=kvm \
-m 4096 \
-smp 4 \
-global driver=cfi.pflash01,property=secure,value=on \
-drive 
if=pflash,format=raw,unit=0,readonly=on,file=Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_CODE.fd
 \
-drive 
if=pflash,format=raw,unit=1,snapshot=on,file=Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_VARS.fd
 \
-drive 
id=hdd,if=none,format=qcow2,snapshot=on,file=fedora-31-efi-grub2-x86_64.qcow2 \
-device virtio-scsi-pci,id=scsi0 \
-device scsi-hd,drive=hdd,bus=scsi0.0,bootindex=1


(4) Test using a 64-bit guest on an AMD host -- just repeat step (3) on an AMD 
host.

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61344): https://edk2.groups.io/g/devel/message/61344
Mute This Topic: https://groups.io/mt/74901067/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD

2020-06-16 Thread Laszlo Ersek

On 06/15/20 20:30, Garrett Kirkendall wrote:
> AMD does not support MSR_IA32_MISC_ENABLE.  Accessing that register
> causes and exception on AMD processors.  If Execution Disable is
> supported, and if the processor is an AMD processor skip manipulating
> MSR_IA32_MISC_ENABLE[34] XD Disable bit.

(1a) I suggest replacing "and if" with "but".

(1b) I suggest inserting a comman before "skip".

(Side comment: someone give a medal to whomever invented the name
"Execution Disable Disable".)

> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Garrett Kirkendall 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |  3 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |  9 -
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   | 19 +--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm| 20 ++--
>  4 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> index 43f6935cf9dc..0f6e0c9c98ad 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> @@ -2,6 +2,7 @@
>  SMM profile internal header file.
>  
>  Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2020, AMD Incorporated. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -13,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "SmmProfileArch.h"
> @@ -99,6 +101,7 @@ extern SMM_S3_RESUME_STATE   *mSmmS3ResumeState;
>  extern UINTN gSmiExceptionHandlers[];
>  extern BOOLEAN   mXdSupported;
>  X86_ASSEMBLY_PATCH_LABEL gPatchXdSupported;
> +X86_ASSEMBLY_PATCH_LABEL gPatchMsrIA32MiscEnableSupported;

The edk2 coding style subjects acronyms to CamelCasing. For example,
"PCI" is CamelCased as "Pci" in identifiers.

(2) Therefore, please replace "IA32" with "Ia32" in the above.

>  extern UINTN *mPFEntryCount;
>  extern UINT64(*mLastPFEntryValue)[MAX_PF_ENTRY_COUNT];
>  extern UINT64*(*mLastPFEntryPointer)[MAX_PF_ENTRY_COUNT];
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index c47b5573e366..146600e6c3b4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -2,7 +2,7 @@
>  Enable SMM profile.
>  
>  Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.
> -Copyright (c) 2017, AMD Incorporated. All rights reserved.
> +Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.
>  
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -1015,6 +1015,13 @@ CheckFeatureSupported (
>mXdSupported = FALSE;
>PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
>  }
> +
> +if (StandardSignatureIsAuthenticAMD()) {

(3) Please insert a space character before the opening parenthesis.

> +  //
> +  // AMD processors do not support MSR_IA32_MISC_ENABLE
> +  //
> +  PatchInstructionX86 (gPatchMsrIA32MiscEnableSupported, FALSE, 1);
> +}
>}
>  
>if (mBtsSupported) {
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> index f96de9bdeb43..e4ef5899f274 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> @@ -1,5 +1,6 @@
>  
> ;--
>  ;
>  ; Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.
> +; Copyright (c) 2020, AMD Incorporated. All rights reserved.
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ;
>  ; Module Name:
> @@ -59,6 +60,7 @@ global ASM_PFX(gPatchSmiStack)
>  global ASM_PFX(gPatchSmbase)
>  extern ASM_PFX(mXdSupported)
>  global ASM_PFX(gPatchXdSupported)
> +global ASM_PFX(gPatchMsrIA32MiscEnableSupported)
>  extern ASM_PFX(gSmiHandlerIdtr)
>  
>  extern ASM_PFX(mCetSupported)
> @@ -153,17 +155,30 @@ ASM_PFX(gPatchSmiCr3):
>  ASM_PFX(gPatchXdSupported):
>  cmp al, 0
>  jz  @SkipXd
> +
> +; Clear XD Disable bit if supported

(4) please expand the comment: "... if MSR_IA32_MISC_ENABLE is supported"

> +mov al, strict byte 1   ; source operand may be patched
> +ASM_PFX(gPatchMsrIA32MiscEnableSupported):
> +cmp al, 1
> +jz  MsrIA32MiscEnableSupported
> +
> +; MSR_IA32_MISC_ENABLE not supported
> +xor edx, edx
> +pushedx

(5) Please append a comment to the right:

"; don't try to restore the XD Disable bit just before RSM"

> +jmp EnableNxe
> +
>  ;
>  ; Check XD disable bit
>  ;
> +MsrIA32MiscEnableSupported:

(6) Please CamelCase "IA32"

>  mov ecx, MSR_IA32_MISC_ENABLE
>  rdmsr
>  pushedx

Re: [edk2-devel] [PATCH 0/5] StandaloneMmPkg: make StMM core relocatable

2020-06-16 Thread Ard Biesheuvel


On 6/10/20 10:17 AM, Ard Biesheuvel wrote:

It is not always possible to deploy the standalone MM core in a way where
the runtime address is known at build time. This does not matter for most
modules, since they are relocated at dispatch time. However, for the MM
core itself, it means we need to do some extra work to relocate the image
in place if it ends up at a different offset than expected.

On AARCH64, the standalone MM stack is deployed inside a non-privileged
secure world container which only has limited control over its memory
mappings, and so we need to ensure that the executable code itself is
free of absolute quantities that need to be fixed up. This is very similar
to how shared libraries are constructed, given that pages can only be
shared between processes if they are not modified, even by the dynamic
loader. So we can use this support to emit the standaline MM core in a
way that guarantees that the executable code does not need to modify
itself (patch #4)

Patch #5 adds the actual code to perform the self relocation after the
.data section has been made writable and non-executable. Note that the
PE/COFF library code modifies the header in place, and so in the case
where we need to perform the runtime relocation, we need to remap the
header page writable and non-executable as well.

The remaining patches are optimizations and fixes I picked up along
the way.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Sami Mujawar 
Cc: Ilias Apalodimas 

Ard Biesheuvel (5):
   MdePkg/BasePrintLib: avoid absolute addresses for error strings
   StandaloneMmPkg/Core: fix bogus FV pointer in DEBUG string
   StandaloneMmPkg/Core: add missing GUID reference
   StandaloneMmPkg: generate position independent code for StMM core
   StandaloneMmPkg/StandaloneMmCoreEntryPoint: relocate StMM core on the
 fly



Patches 2-5 merged as #702 (patch #1 was respun separately, and merged 
as #699 earlier)


Thanks all

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61342): https://edk2.groups.io/g/devel/message/61342
Mute This Topic: https://groups.io/mt/74792287/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib

2020-06-16 Thread Laszlo Ersek

On 06/15/20 20:30, Garrett Kirkendall wrote:
> Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
> separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
> This allows for future use of StandarSignatureIsAuthinticAMD without
> creating more instances in other modules.
> 
> This function allows IA32/X64 code to determine if it is running on an
> AMD brand processor.
> 
> UefiCpuLib is already included directly or indirectly in all modified
> modules.  Complete move is made in this change.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Garrett Kirkendall 
> ---
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf |  7 
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf |  2 ++
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf |  2 ++
>  UefiCpuPkg/Include/Library/UefiCpuLib.h  | 14 
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c   | 38 
> 
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c   | 25 
> ++---
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 
> ++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 23 
> 
>  8 files changed, 67 insertions(+), 69 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf 
> b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> index 006b7acbf14e..34d3a7bb4303 100644
> --- a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> @@ -4,6 +4,7 @@
>  #  The library routines are UEFI specification compliant.
>  #
>  #  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2020, AMD Inc. All rights reserved.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -29,6 +30,12 @@ [Sources.IA32]
>  [Sources.X64]
>X64/InitializeFpu.nasm
>  
> +[Sources]
> +  BaseUefiCpuLib.c
> +
>  [Packages]
>MdePkg/MdePkg.dec
>UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf 
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> index bdb2ff372677..561baa44b0e6 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> @@ -5,6 +5,7 @@
>  #  where local APIC is disabled.
>  #
>  #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2020, AMD Inc. All rights reserved.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -37,6 +38,7 @@ [LibraryClasses]
>TimerLib
>IoLib
>PcdLib
> +  UefiCpuLib
>  
>  [Pcd]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds  ## 
> SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf 
> b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> index ac1e0a1c9896..1e2a4f8b790f 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> @@ -5,6 +5,7 @@
>  #  where local APIC is disabled.
>  #
>  #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2020, AMD Inc. All rights reserved.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  ##
> @@ -37,6 +38,7 @@ [LibraryClasses]
>TimerLib
>IoLib
>PcdLib
> +  UefiCpuLib
>  
>  [Pcd]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds  ## 
> SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/Include/Library/UefiCpuLib.h 
> b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> index 82e53bab3a0f..5326e7246301 100644
> --- a/UefiCpuPkg/Include/Library/UefiCpuLib.h
> +++ b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> @@ -5,6 +5,7 @@
>to be UEFI specification compliant.
>  
>Copyright (c) 2009, Intel Corporation. All rights reserved.
> +  Copyright (c) 2020, AMD Inc. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -29,4 +30,17 @@ InitializeFloatingPointUnits (
>VOID
>);
>  
> +/**
> +  Determine if the standard CPU signature is "AuthenticAMD".
> +
> +  @retval TRUE  The CPU signature matches.
> +  @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +StandardSignatureIsAuthenticAMD (
> +  VOID
> +  );
> +
>  #endif
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c 
> b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> new file mode 100644
> index ..c2cc3ff9a709
> --- /dev/null
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> @@ -0,0 +1,38 @@
> +/** @file
> +  This library defines some routines that are generic for IA32 family CPU.
> +
> +  The library routines are UEFI specification compliant.
> +
> +  Copyright (c) 2020, AMD Inc. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
>

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/NXP: LX2160A: Add SerDes Support

2020-06-16 Thread Leif Lindholm

On Wed, Jun 10, 2020 at 02:38:54 +0530, Wasim Khan wrote:
> From: Wasim Khan 
> 
> Based on SerDes protocol value in reset configuration word (RCW)
> different IP blocks gets enabled in HW.
> Add SoC specific SerDes configuration for LX2160A, which can be
> used by different IPs to know the enabled interfaces and perform
> the required initialization.
> 
> Signed-off-by: Wasim Khan 
> ---
>  Silicon/NXP/LX2160A/LX2160A.dsc.inc   |   2 +
>  Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf |   6 +
>  Silicon/NXP/LX2160A/Include/SocSerDes.h   |  74 +++
>  Silicon/NXP/LX2160A/Library/SocLib/SerDes.c   | 211 
>  4 files changed, 293 insertions(+)
> 
> diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc 
> b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> index af22b4dd973c..fe8ed402fc4e 100644
> --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> @@ -15,6 +15,7 @@ [LibraryClasses.common]
>PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>
> PL011UartClockLib|Silicon/NXP/Library/PL011UartClockLib/PL011UartClockLib.inf
> +  SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf
>  
>  
> 
>  #
> @@ -32,6 +33,7 @@ [PcdsFixedAtBuild.common]
>gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x23A
>gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x239
>gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|91
> +  gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes|0x8
>  
>gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C
> diff --git a/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf 
> b/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> index 421072b88019..54bcb82e6877 100644
> --- a/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> +++ b/Silicon/NXP/LX2160A/Library/SocLib/SocLib.inf
> @@ -22,6 +22,12 @@ [Packages]
>  [LibraryClasses]
>ChassisLib
>DebugLib
> +  PcdLib
> +  SerDesHelperLib
>  
>  [Sources.common]
> +  SerDes.c
>SocLib.c
> +
> +[FixedPcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes
> diff --git a/Silicon/NXP/LX2160A/Include/SocSerDes.h 
> b/Silicon/NXP/LX2160A/Include/SocSerDes.h
> new file mode 100644
> index ..02000622d89a
> --- /dev/null
> +++ b/Silicon/NXP/LX2160A/Include/SocSerDes.h
> @@ -0,0 +1,74 @@
> +/** SocSerDes.h
> +  SoC Specific header file for SerDes
> +
> +  Copyright 2017-2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef SOC_SERDES_H
> +#define SOC_SERDES_H
> +
> +typedef enum {
> +  NONE = 0,
> +  PCIE1,
> +  PCIE2,
> +  PCIE3,
> +  PCIE4,
> +  PCIE5,
> +  PCIE6,
> +  SATA1,
> +  SATA2,
> +  SATA3,
> +  SATA4,
> +  XFI1,
> +  XFI2,
> +  XFI3,
> +  XFI4,
> +  XFI5,
> +  XFI6,
> +  XFI7,
> +  XFI8,
> +  XFI9,
> +  XFI10,
> +  XFI11,
> +  XFI12,
> +  XFI13,
> +  XFI14,
> +  SGMII1,
> +  SGMII2,
> +  SGMII3,
> +  SGMII4,
> +  SGMII5,
> +  SGMII6,
> +  SGMII7,
> +  SGMII8,
> +  SGMII9,
> +  SGMII10,
> +  SGMII11,
> +  SGMII12,
> +  SGMII13,
> +  SGMII14,
> +  SGMII15,
> +  SGMII16,
> +  SGMII17,
> +  SGMII18,
> +  GE100_1,
> +  GE100_2,
> +  GE50_1,
> +  GE50_2,
> +  GE40_1,
> +  GE40_2,
> +  GE25_1,
> +  GE25_2,
> +  GE25_3,
> +  GE25_4,
> +  GE25_5,
> +  GE25_6,
> +  GE25_7,
> +  GE25_8,
> +  GE25_9,
> +  GE25_10,
> +  SERDES_PROTOCOL_COUNT
> +} SERDES_PROTOCOL;
> +#endif
> diff --git a/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c 
> b/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
> new file mode 100644
> index ..58f2b3df3600
> --- /dev/null
> +++ b/Silicon/NXP/LX2160A/Library/SocLib/SerDes.c
> @@ -0,0 +1,211 @@
> +/** SerDes.c
> +  Provides SoC specific SerDes interface
> +
> +  Copyright 2017-2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +// SerDes Protocol Mask and Shift in Reset Configuration Word (RCW) Status 
> Register
> +#define SERDES1_PROTOCOL_MASK   0x001f
> +#define SERDES1_PROTOCOL_SHIFT  16
> +#define SERDES2_PROTOCOL_MASK   0x03E0
> +#define SERDES2_PROTOCOL_SHIFT  21
> +#define SERDES3_PROTOCOL_MASK   0x7C00
> +#define SERDES3_PROTOCOL_SHIFT  26
> +
> +SERDES_CONFIG gSerDes1ConfigTable[] = {

Are these intended to be directly externally accessed (as opposed to
through gSerDesConfig[])? If not, could they be STATIC? (And if so,
they should probably have an 'm' (module) prefix rather than a 'g'
(global) one.

/
Leif

> +  {  1, { PCIE2, PCIE2, PCIE2, PCIE2, PCIE1, PCIE1, PCIE1, PCIE1 } },
> +  {  2, { PCIE2, PCIE2, PCIE2, PCIE2, SGMII6, SGMII5, SGMII4, SGMII3 } },
> +  {  3, { PCIE2, PCIE2, PCIE2, PCIE2, XFI6, XFI5, XFI4, XFI3 } },
> +  {  4, { SGMII10, SGMII9, SGMII8, SGMII7, SGMII6, SGMII5, SGMII4, SGMII3 } 
> },
>

Re: [edk2-devel] [PATCH 5/5] UefiCpuPkg: Uninstall EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.

2020-06-16 Thread Laszlo Ersek

On 06/16/20 11:04, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> To avoid leaking information from SMM, uninstall
> EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 37 
> +
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  1 +
>  2 files changed, 38 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index db68e1316e..a1b209e125 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -520,6 +520,33 @@ SmmReadyToLockEventNotify (
>return EFI_SUCCESS;
>  }
>  
> +/**
> +  SMM End of Dxe event notification handler.
> +
> +  To avoid leaking information from SMM, uninstall 
> EFI_SMM_CONFIGURATION_PROTOCOL
> +  at end of Dxe.
> +
> +  @param[in] Protocol   Points to the protocol's unique identifier.
> +  @param[in] Interface  Points to the interface instance.
> +  @param[in] Handle The handle on which the interface was installed.
> +
> +  @retval EFI_SUCCESS   Notification handler runs successfully.
> + **/
> +EFI_STATUS
> +EFIAPI
> +SmmEndOfDxeNotify (
> +  IN CONST EFI_GUID  *Protocol,
> +  IN VOID*Interface,
> +  IN EFI_HANDLE  Handle
> +  )
> +{
> +  gBS->UninstallProtocolInterface (
> + gSmmCpuPrivate->SmmCpuHandle,
> + , >SmmConfiguration
> + );
> +  return EFI_SUCCESS;
> +}

(1) I suggest setting "gSmmCpuPrivate->SmmCpuHandle" to NULL here.

(2) I also suggest de-registering the gEfiSmmEndOfDxeProtocolGuid
notification.

Thanks
Laszlo

> +
>  /**
>The module Entry Point of the CPU SMM driver.
>  
> @@ -1038,6 +1065,16 @@ PiCpuSmmEntry (
>  );
>ASSERT_EFI_ERROR (Status);
>  
> +  //
> +  // register SMM End of Dxe notification
> +  //
> +  Status = gSmst->SmmRegisterProtocolNotify (
> +,
> +SmmEndOfDxeNotify,
> +
> +);
> +  ASSERT_EFI_ERROR (Status);
> +
>//
>// Initialize SMM Profile feature
>//
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index 76b1462996..bb994814d6 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -105,6 +105,7 @@
>gEfiSmmConfigurationProtocolGuid ## PRODUCES
>gEfiSmmCpuProtocolGuid   ## PRODUCES
>gEfiSmmReadyToLockProtocolGuid   ## NOTIFY
> +  gEfiSmmEndOfDxeProtocolGuid  ## NOTIFY
>gEfiSmmCpuServiceProtocolGuid## PRODUCES
>gEdkiiSmmMemoryAttributeProtocolGuid ## PRODUCES
>gEfiMmMpProtocolGuid## PRODUCES
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61339): https://edk2.groups.io/g/devel/message/61339
Mute This Topic: https://groups.io/mt/74912556/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 4/5] SecurityPkg: Remove DXE_SMM_DRIVER support for some libraries

2020-06-16 Thread Laszlo Ersek

On 06/16/20 11:04, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> Remove DXE_SMM_DRIVER support for some libraries because they
> have the risks of leaking data from SMM mode to non-SMM mode.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Chao Zhang 
> Signed-off-by: Zhiguang Liu 
> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf | 2 
> +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> index 1e1a639857..9494d04b1d 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> @@ -20,7 +20,7 @@
>FILE_GUID  = 0CA970E1-43FA-4402-BC0A-81AF336BFFD6
>MODULE_TYPE= DXE_DRIVER
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = NULL|DXE_DRIVER DXE_RUNTIME_DRIVER 
> DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
> +  LIBRARY_CLASS  = NULL|DXE_DRIVER DXE_RUNTIME_DRIVER 
> UEFI_APPLICATION UEFI_DRIVER
>CONSTRUCTOR= DxeImageVerificationLibConstructor
>  
>  #
> 

"Min Xu " is missing from the CC list, according to
"BaseTools/Scripts/GetMaintainer.py" (fixing that now).

thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61338): https://edk2.groups.io/g/devel/message/61338
Mute This Topic: https://groups.io/mt/74912554/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI LOADED_IMAGE from SMM.

2020-06-16 Thread Laszlo Ersek

On 06/16/20 11:04, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
> 
> This patch removes EFI_LOADED_IMAGE_PROTOCOL from DXE database.
> The SmmCore only install EFI_LOADED_IMAGE_PROTOCOL to SMM database.
> 
> Exposing LOADED_IMAGE_PROTOCOL may cause SMM information leakage
> to non-SMM component.
> 
> Cc: Feng Tian 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Laszlo Ersek 
> Signed-off-by: Jiewen Yao 
> Signed-off-by: Zhiguang Liu 
> ---
>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 104 
> +++-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c  |  34 
> --
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h  |   4 ++--
>  3 files changed, 13 insertions(+), 129 deletions(-)

The CC list on this patch is lacking. Per
"BaseTools/Scripts/GetMaintainer.py", you should have included the
following CC's:

  Eric Dong 
  Hao A Wu 
  Jian J Wang 
  Ray Ni 

adding them now.


> diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c 
> b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> index 76ee9e0b89..2be2866234 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> @@ -316,7 +316,7 @@ SmmLoadImage (
>EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
>PE_COFF_LOADER_IMAGE_CONTEXT   ImageContext;
>  
> -  PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle);
> +  PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle);
>  
>Buffer   = NULL;
>Size = 0;
> @@ -546,51 +546,14 @@ SmmLoadImage (
>DriverEntry->ImageBuffer  = DstBuffer;
>DriverEntry->NumberOfPage = PageCount;
>  
> -  //
> -  // Allocate a Loaded Image Protocol in EfiBootServicesData
> -  //
> -  Status = gBS->AllocatePool (EfiBootServicesData, sizeof 
> (EFI_LOADED_IMAGE_PROTOCOL), (VOID **)>LoadedImage);
> -  if (EFI_ERROR (Status)) {
> -if (Buffer != NULL) {
> -  gBS->FreePool (Buffer);
> -}
> -SmmFreePages (DstBuffer, PageCount);
> -return Status;
> -  }

(Side comment: the error path that's being removed here seems to contain
a resource leak. We're past PeCoffLoaderLoadImage(), but we're not
calling PeCoffLoaderUnloadImage().)

> -
> -  ZeroMem (DriverEntry->LoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL));
>//
>// Fill in the remaining fields of the Loaded Image Protocol instance.
> -  // Note: ImageBase is an SMRAM address that can not be accessed outside of 
> SMRAM if SMRAM window is closed.
>//
> -  DriverEntry->LoadedImage->Revision  = 
> EFI_LOADED_IMAGE_PROTOCOL_REVISION;
> -  DriverEntry->LoadedImage->ParentHandle  = 
> gSmmCorePrivate->SmmIplImageHandle;
> -  DriverEntry->LoadedImage->SystemTable   = gST;
> -  DriverEntry->LoadedImage->DeviceHandle  = DeviceHandle;
> -
>DriverEntry->SmmLoadedImage.Revision = 
> EFI_LOADED_IMAGE_PROTOCOL_REVISION;
>DriverEntry->SmmLoadedImage.ParentHandle = 
> gSmmCorePrivate->SmmIplImageHandle;
>DriverEntry->SmmLoadedImage.SystemTable  = gST;
>DriverEntry->SmmLoadedImage.DeviceHandle = DeviceHandle;
>  
> -  //
> -  // Make an EfiBootServicesData buffer copy of FilePath
> -  //
> -  Status = gBS->AllocatePool (EfiBootServicesData, GetDevicePathSize 
> (FilePath), (VOID **)>LoadedImage->FilePath);
> -  if (EFI_ERROR (Status)) {
> -if (Buffer != NULL) {
> -  gBS->FreePool (Buffer);
> -}
> -SmmFreePages (DstBuffer, PageCount);
> -return Status;
> -  }

(Side comment: at this point, we've used to leak even
"DriverEntry->LoadedImage".)

> -  CopyMem (DriverEntry->LoadedImage->FilePath, FilePath, GetDevicePathSize 
> (FilePath));
> -
> -  DriverEntry->LoadedImage->ImageBase = (VOID *)(UINTN) 
> ImageContext.ImageAddress;
> -  DriverEntry->LoadedImage->ImageSize = ImageContext.ImageSize;
> -  DriverEntry->LoadedImage->ImageCodeType = EfiRuntimeServicesCode;
> -  DriverEntry->LoadedImage->ImageDataType = EfiRuntimeServicesData;
> -
>//
>// Make a buffer copy of FilePath
>//
> @@ -599,7 +562,6 @@ SmmLoadImage (
>  if (Buffer != NULL) {
>gBS->FreePool (Buffer);
>  }
> -gBS->FreePool (DriverEntry->LoadedImage->FilePath);
>  SmmFreePages (DstBuffer, PageCount);
>  return Status;
>}

(side comment: DriverEntry->LoadedImage was still leaked)

> @@ -610,16 +572,6 @@ SmmLoadImage (
>DriverEntry->SmmLoadedImage.ImageCodeType = EfiRuntimeServicesCode;
>DriverEntry->SmmLoadedImage.ImageDataType = EfiRuntimeServicesData;
>  
> -  //
> -  // Create a new image handle in the UEFI handle database for the SMM Driver
> -  //
> -  DriverEntry->ImageHandle = NULL;
> -  Status = gBS->InstallMultipleProtocolInterfaces (
> -  >ImageHandle,
> -  , DriverEntry->LoadedImage,
> -  NULL
> -  );
> -
>//
>// Create a new image handle in the SMM handle database for the SMM Driver
>//
> @@ -631,7 +583,7 @@

Re: [edk2-devel] [PATCH edk2-platforms v2 0/2] Optimize Smbios type 9

2020-06-16 Thread Leif Lindholm

On Tue, Jun 09, 2020 at 21:41:06 +0800, Ming Huang wrote:
> Main change since v1:
> 1 Add some commit messages.

For the series:
Reviewed-by: Leif Lindholm 

Pushed as c764551258ee..1c1506e2b28a

> Ming Huang (2):
>   Silicon/Hisilicon/Smbios: correct coding style issue in type 9
>   Silicon/Hisilicon/Smbios: Optimize type 9
> 
>  Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.c | 325 
> +++-
>  1 file changed, 174 insertions(+), 151 deletions(-)
> 
> -- 
> 2.8.1
> 
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61336): https://edk2.groups.io/g/devel/message/61336
Mute This Topic: https://groups.io/mt/74774164/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v3 3/4] Silicon/Hisilicon/Acpi: Add update sas address feature

2020-06-16 Thread Leif Lindholm

One remaining question, then this set is ready to go in:

On Tue, Jun 09, 2020 at 21:27:24 +0800, Ming Huang wrote:
> The updating sas address feature is similar with apdating mac address.
> Modify updating dsdt flow for add this feature.
> 
> Signed-off-by: Ming Huang 
> ---
>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c  |   2 +-
>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf |   1 +
>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c| 292 
> +++-
>  Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.h|   2 +-
>  4 files changed, 227 insertions(+), 70 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c 
> b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c
> index c45a0bb..9cdf710 100644
> --- a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c
> +++ b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatform.c
> @@ -46,7 +46,7 @@ UpdateAcpiDsdt (
>  return;
>}
>  
> -  Status = EthMacInit ();
> +  Status = UpdateAcpiDsdtTable ();
>if (EFI_ERROR (Status)) {
>  DEBUG ((DEBUG_ERROR, " UpdateAcpiDsdtTable Failed, Status = %r\n", 
> Status));
>}
> diff --git a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf 
> b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
> index 866ff75..856309a 100644
> --- a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
> +++ b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
> @@ -46,6 +46,7 @@
>gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>gEfiAcpiSdtProtocolGuid   # PROTOCOL ALWAYS_CONSUMED
>gHisiBoardNicProtocolGuid   # PROTOCOL 
> ALWAYS_CONSUMED
> +  gHisiSasConfigProtocolGuid
>  
>  [FeaturePcd]
>gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol
> diff --git a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c 
> b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c
> index cd98506..841c94e 100644
> --- a/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c
> +++ b/Silicon/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c
> @@ -1,7 +1,7 @@
>  /** @file
>  
>Copyright (c) 2014, Applied Micro Curcuit Corporation. All rights 
> reserved.
> -  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +  Copyright (c) 2015 - 2020, Hisilicon Limited. All rights reserved.
>Copyright (c) 2015, Linaro Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,6 +33,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  // Turn on debug message by enabling below define
>  //#define ACPI_DEBUG
> @@ -45,17 +47,27 @@
>  #define EFI_ACPI_MAX_NUM_TABLES 20
>  #define DSDT_SIGNATURE  0x54445344
>  
> -#define D03_ACPI_ETH_ID "HISI00C2"
> -
>  #define ACPI_ETH_MAC_KEY"local-mac-address"
> +#define ACPI_ETH_SAS_KEY"sas-addr"
>  
>  #define PREFIX_VARIABLE_NAMEL"MAC"
>  #define PREFIX_VARIABLE_NAME_COMPAT L"RGMII_MAC"
> -#define MAC_MAX_LEN 30
> +#define ADDRESS_MAX_LEN 30
> +
> +CHAR8 *mHisiAcpiDevId[] = {"HISI00C1","HISI00C2","HISI0162"};
> +
> +typedef enum {
> +  DsdtDeviceUnknown,
> +  DsdtDeviceLan,
> +  DsdtDeviceSas
> +} DSDT_DEVICE_TYPE;
>  
> -EFI_STATUS GetEnvMac(
> -  IN  UINTNMacNextID,
> -  IN OUT  UINT8*MacBuffer)
> +STATIC
> +EFI_STATUS
> +GetEnvMac(
> +  IN UINTNMacNextID,
> +  IN OUT UINT8*MacBuffer
> +  )
>  {
>EFI_MAC_ADDRESS Mac;
>EFI_STATUS Status;
> @@ -89,12 +101,121 @@ EFI_STATUS GetEnvMac(
>return EFI_SUCCESS;
>  }
>  
> -EFI_STATUS _SearchReplacePackageMACAddress(
> +STATIC
> +EFI_STATUS
> +GetSasAddress (
> +  IN UINT8Index,
> +  IN OUT UINT8*SasAddrBuffer
> +  )
> +{
> +  EFI_STATUS Status;
> +  HISI_SAS_CONFIG_PROTOCOL *HisiSasConf;
> +
> +  if (SasAddrBuffer == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = gBS->LocateProtocol (, NULL, (VOID 
> **));
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "Locate Sas Config Protocol failed %r\n", Status));
> +SasAddrBuffer[0] = 0x50;
> +SasAddrBuffer[1] = 0x01;
> +SasAddrBuffer[2] = 0x88;
> +SasAddrBuffer[3] = 0x20;
> +SasAddrBuffer[4] = 0x16;
> +SasAddrBuffer[5] = 0x00;
> +SasAddrBuffer[6] = 0x00;
> +SasAddrBuffer[7] = Index;

This is still a sompletely random-looking value being stuffed into the
buffer. What is it?

/
Leif

> +return Status;
> +  }
> +
> +  return HisiSasConf->GetAddr (Index, SasAddrBuffer);
> +}
> +
> +STATIC
> +EFI_STATUS
> +UpdateAddressInOption (
> +  IN EFI_ACPI_SDT_PROTOCOL  *AcpiTableProtocol,
> +  IN EFI_ACPI_HANDLEChildHandle,
> +  IN UINTN  DevNextID,
> +  IN DSDT_DEVICE_TYPE

[edk2-devel] [PATCH v7 2/2] Features/Intel/PostCodeDebugFeaturePkg: add it.

2020-06-16 Thread Tan, Ming

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

The PostCodeDebugFeaturePkg include some useful post code debug
libraries, such as get post code from status code and show it.

It provide a library PostCodeStatusCodeHandlerLib used by edk2
StatusCodeHandler.efi, used to show the post code.
It also provide a library of PostCodeMap lib, it map the status code
to post code.

Cc: Eric Dong 
Cc: Liming Gao 
Signed-off-by: Ming Tan 
---
V7: Add the dec files in AdvancedFeaturesPcd.dsc. Combine Beep and PostCode to 
2/2 patchs.
And change to same V7 of Beep.
V4: In dec files, change the PcdStatusCodeUsePostCode default to FALSE.
V3: Modify according Eric's comments.
Modify some bugs about dsc file when it is included.
Update Readme.md.
V2: Delete the last empty line in 
PostCodeDebugFeaturePkg/Library/PostCodeMapLib/PostCodeMapLib.inf
 .../Include/AdvancedFeaturesPcd.dsc   |   1 +
 .../Include/Library/PostCodeMapLib.h  |  32 +++
 .../Include/PostCodeDebugFeature.dsc  | 200 +
 .../PlatformStatusCodesInternal.h | 270 ++
 .../Library/PostCodeMapLib/PostCodeMapLib.c   | 207 ++
 .../Library/PostCodeMapLib/PostCodeMapLib.inf |  27 ++
 .../PeiPostCodeStatusCodeHandlerLib.c | 102 +++
 .../PeiPostCodeStatusCodeHandlerLib.inf   |  49 
 .../RuntimeDxePostCodeStatusCodeHandlerLib.c  | 188 
 ...RuntimeDxePostCodeStatusCodeHandlerLib.inf |  51 
 .../SmmPostCodeStatusCodeHandlerLib.c | 141 +
 .../SmmPostCodeStatusCodeHandlerLib.inf   |  50 
 .../PostCodeDebugFeaturePkg.dec   |  32 +++
 .../PostCodeDebugFeaturePkg.dsc   |  30 ++
 .../PostCodeDebugFeaturePkg/Readme.md | 117 
 15 files changed, 1497 insertions(+)
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/Library/PostCodeMapLib.h
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeMapLib/PlatformStatusCodesInternal.h
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeMapLib/PostCodeMapLib.c
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeMapLib/PostCodeMapLib.inf
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib/PeiPostCodeStatusCodeHandlerLib.c
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib/PeiPostCodeStatusCodeHandlerLib.inf
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib/RuntimeDxePostCodeStatusCodeHandlerLib.c
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib/RuntimeDxePostCodeStatusCodeHandlerLib.inf
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib/SmmPostCodeStatusCodeHandlerLib.c
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCodeHandlerLib/SmmPostCodeStatusCodeHandlerLib.inf
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/PostCodeDebugFeaturePkg.dec
 create mode 100644 
Features/Intel/Debugging/PostCodeDebugFeaturePkg/PostCodeDebugFeaturePkg.dsc
 create mode 100644 Features/Intel/Debugging/PostCodeDebugFeaturePkg/Readme.md

diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc 
b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
index 366b551bd3..ad248de800 100644
--- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
+++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
@@ -25,6 +25,7 @@
   UserAuthFeaturePkg/UserAuthFeaturePkg.dec
   LogoFeaturePkg/LogoFeaturePkg.dec
   BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec
+  PostCodeDebugFeaturePkg/PostCodeDebugFeaturePkg.dec
 
 #
 # The section below sets all PCDs to FALSE in this DSC file so the feature is 
not enabled by default.
diff --git 
a/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/Library/PostCodeMapLib.h
 
b/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/Library/PostCodeMapLib.h
new file mode 100644
index 00..834be623a1
--- /dev/null
+++ 
b/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/Library/PostCodeMapLib.h
@@ -0,0 +1,32 @@
+/** @file
+  This library class provides Platform PostCode Map.
+
+  Copyright (c) 2011 - 2020, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __POST_CODE_MAP_LIB__
+#define __POST_CODE_MAP_LIB__
+
+/**
+  Get PostCode from status code type and value.
+
+  @param  CodeType Indicates the type of status code being reported.
+  @param  ValueDescribes the current status of a hardware or
+

[edk2-devel] [PATCH v7 1/2] Features/Intel/BeepDebugFeaturePkg: add it.

2020-06-16 Thread Tan, Ming

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

The BeepDebugFeaturePkg include some useful beep debug
libraries, such as get beep value from status code and beep.

It provide a library BeepStatusCodeHandlerLib used by edk2
StatusCodeHandler.efi, used to do beep if needed.
It also provide a library of BeepMap lib, it map the status code
to beep value.
A library of Beep lib is needed by platform, and this pkg has a
Null implementation.

Cc: Eric Dong 
Cc: Liming Gao 
Signed-off-by: Ming Tan 
---
V7: Add the dec files in AdvancedFeaturesPcd.dsc. Combine Beep and PostCode to 
2/2 patchs.
V6: Modify some bug when include the platform dsc file. And modify Readme.md.
V5: In .inf files, remove some useless library.
In RuntimeDxeBeepStatusCodeHandlerLib.c, add a variable to indicate whether 
need unregister.
V4: Change Include/BeepDebugFeature.dsc, make it can be included in platform 
dsc file.
V3: Modify according the Eric's review comments.
V2: Delete the last empty line in 
BeepDebugFeaturePkg/Library/BeepMapLib/BeepMapLib.inf
 .../Include/AdvancedFeaturesPcd.dsc   |  83 +++---
 .../BeepDebugFeaturePkg.dec   |  36 +++
 .../BeepDebugFeaturePkg.dsc   |  30 ++
 .../Include/BeepDebugFeature.dsc  | 201 +
 .../Include/Library/BeepLib.h |  33 +++
 .../Include/Library/BeepMapLib.h  |  32 +++
 .../Library/BeepLib/BeepLibNull.c |  37 +++
 .../Library/BeepLib/BeepLibNull.inf   |  26 ++
 .../Library/BeepMapLib/BeepMapLib.c   | 116 
 .../Library/BeepMapLib/BeepMapLib.inf |  27 ++
 .../BeepMapLib/PlatformStatusCodesInternal.h  | 270 ++
 .../PeiBeepStatusCodeHandlerLib.c | 101 +++
 .../PeiBeepStatusCodeHandlerLib.inf   |  49 
 .../RuntimeDxeBeepStatusCodeHandlerLib.c  | 184 
 .../RuntimeDxeBeepStatusCodeHandlerLib.inf|  51 
 .../SmmBeepStatusCodeHandlerLib.c | 138 +
 .../SmmBeepStatusCodeHandlerLib.inf   |  50 
 .../Debugging/BeepDebugFeaturePkg/Readme.md   | 125 
 18 files changed, 1548 insertions(+), 41 deletions(-)
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dsc
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepLib.h
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepMapLib.h
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepLib/BeepLibNull.c
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepLib/BeepLibNull.inf
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepMapLib/BeepMapLib.c
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepMapLib/BeepMapLib.inf
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepMapLib/PlatformStatusCodesInternal.h
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/PeiBeepStatusCodeHandlerLib.c
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/PeiBeepStatusCodeHandlerLib.inf
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.c
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/RuntimeDxeBeepStatusCodeHandlerLib.inf
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/SmmBeepStatusCodeHandlerLib.c
 create mode 100644 
Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandlerLib/SmmBeepStatusCodeHandlerLib.inf
 create mode 100644 Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md

diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc 
b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
index 836da7c944..366b551bd3 100644
--- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
+++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
@@ -1,41 +1,42 @@
-## @file

-#  DSC file for defining Pcd of advanced features.

-#

-#  This file is intended to be included into another package so advanced 
features

-#  can be conditionally built by enabling the respective feature via its 
FeaturePCD.

-#

-# Copyright (c) 2020, Intel Corporation. All rights reserved.

-#

-# SPDX-License-Identifier: BSD-2-Clause-Patent

-#

-##

-

-#

-# The section references the package DEC files,

-# it allow a FeaturePCD to be used in a conditional statement

-#

-[Packages]

-  MdePkg/MdePkg.dec

-  AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec

-

[edk2-devel] [PATCH 1/2] SecurityPkg/Tpm2CommandLib: add new function Tpm2GetCapabilityIsCmdImpl.

2020-06-16 Thread Qi Zhang

From: "Zhang, Qi" 

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

check if the commad is supported by comparing the command code with
command index.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Cc: Rahul Kumar 
Signed-off-by: Qi Zhang 
---
 SecurityPkg/Include/Library/Tpm2CommandLib.h| 16 
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 40 

 2 files changed, 56 insertions(+)

diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h 
b/SecurityPkg/Include/Library/Tpm2CommandLib.h
index ce381e786b..ce7290c0f5 100644
--- a/SecurityPkg/Include/Library/Tpm2CommandLib.h
+++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h
@@ -790,6 +790,22 @@ Tpm2GetCapabilityAlgorithmSet (
   OUT UINT32  *AlgorithmSet
   );
 
+/**
+  This function will query if the command is supported.
+
+  @param[In]  Command TPM_CC command starts from TPM_CC_FIRST.
+  @param[out] IsCmdImpl   The command is supported or not.
+
+  @retval EFI_SUCCESSOperation completed successfully.
+  @retval EFI_DEVICE_ERROR   The command was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2GetCapabilityIsCmdImpl (
+  IN  TPM_CC  Command,
+  OUT BOOLEAN *IsCmdImpl
+  );
+
 /**
   This command is used to check to see if specific combinations of algorithm 
parameters are supported.
 
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c 
b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
index 85b11c7715..0c8f980e69 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
@@ -39,6 +39,8 @@ typedef struct {
 
 #pragma pack()
 
+#define TPMA_CC_COMMANDINDEX_MASK  0x2000
+
 /**
   This command returns various information regarding the TPM and its current 
state.
 
@@ -628,6 +630,44 @@ Tpm2GetCapabilityAlgorithmSet (
   return EFI_SUCCESS;
 }
 
+/**
+  This function will query if the command is supported.
+
+  @param[In]  Command TPM_CC command starts from TPM_CC_FIRST.
+  @param[out] IsCmdImpl   The command is supported or not.
+
+  @retval EFI_SUCCESSOperation completed successfully.
+  @retval EFI_DEVICE_ERROR   The command was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2GetCapabilityIsCmdImpl (
+  IN  TPM_CC  Command,
+  OUT BOOLEAN *IsCmdImpl
+  )
+{
+  TPMS_CAPABILITY_DATATpmCap;
+  TPMI_YES_NO MoreData;
+  EFI_STATUS  Status;
+  UINT32  Attribute;
+
+  Status = Tpm2GetCapability (
+ TPM_CAP_COMMANDS,
+ Command,
+ 1,
+ ,
+ 
+ );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  CopyMem (, [0], sizeof 
(UINT32));
+  *IsCmdImpl = (Command == (SwapBytes32(Attribute) & 
TPMA_CC_COMMANDINDEX_MASK));
+
+  return EFI_SUCCESS;
+}
+
 /**
   This command is used to check to see if specific combinations of algorithm 
parameters are supported.
 
-- 
2.26.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61331): https://edk2.groups.io/g/devel/message/61331
Mute This Topic: https://groups.io/mt/74913407/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/2] SecurityPkg/Tcg2Config: remove TPM2_ChangEPS if it is not supported.

2020-06-16 Thread Qi Zhang

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

In current implementation TPM2_ChangeEPS command is always available
in the TPM2 operation pull down list in TCG2 Configuration, which
is confusing when the command is not supported by specific TPM chip.
As a user experience improvement, TPM2_ChangeEPS command should be
removed from the list when it is not supported.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Cc: Rahul Kumar 
Signed-off-by: Qi Zhang 
---
 SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr | 2 ++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c   | 7 +++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h | 1 +
 3 files changed, 10 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
index 91a463997c..47d63b009d 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
@@ -144,7 +144,9 @@ formset
 option text = STRING_TOKEN(STR_TCG2_DISABLE), value = 
TCG2_PHYSICAL_PRESENCE_DISABLE, flags = RESET_REQUIRED;
 option text = STRING_TOKEN(STR_TCG2_CLEAR), value = 
TCG2_PHYSICAL_PRESENCE_CLEAR, flags = RESET_REQUIRED;
 option text = STRING_TOKEN(STR_TCG2_SET_PCD_BANKS), value = 
TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS, flags = RESET_REQUIRED;
+suppressif ideqval TCG2_CONFIGURATION_INFO.ChangeEPSSupported == 0;
 option text = STRING_TOKEN(STR_TCG2_CHANGE_EPS), value = 
TCG2_PHYSICAL_PRESENCE_CHANGE_EPS, flags = RESET_REQUIRED;
+endif
 option text = STRING_TOKEN(STR_TCG2_LOG_ALL_DIGESTS), value = 
TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS, flags = RESET_REQUIRED;
 option text = 
STRING_TOKEN(STR_TCG2_DISABLE_ENDORSEMENT_ENABLE_STORAGE_HIERARCHY), value = 
TCG2_PHYSICAL_PRESENCE_DISABLE_ENDORSEMENT_ENABLE_STORAGE_HIERARCHY, flags = 
RESET_REQUIRED;
 endoneof;
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
index baa8fcd08d..464cacc207 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
@@ -788,6 +788,7 @@ InstallTcg2ConfigForm (
   CHAR16  TempBuffer[1024];
   TCG2_CONFIGURATION_INFO Tcg2ConfigInfo;
   TPM2_PTP_INTERFACE_TYPE TpmDeviceInterfaceDetected;
+  BOOLEAN IsCmdImp = FALSE;
 
   DriverHandle = NULL;
   ConfigAccess = >ConfigAccess;
@@ -870,6 +871,12 @@ InstallTcg2ConfigForm (
 HiiSetString (PrivateData->HiiHandle, STRING_TOKEN 
(STR_TPM2_SUPPORTED_HASH_ALGO_CONTENT), TempBuffer, NULL);
   }
 
+  Status = Tpm2GetCapabilityIsCmdImpl(TPM_CC_ChangeEPS, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Tpm2GetCapabilityIsCmdImpl fails %r\n", Status));
+  }
+  Tcg2ConfigInfo.ChangeEPSSupported = IsCmdImp;
+
   FillBufferWithBootHashAlg (TempBuffer, sizeof(TempBuffer), PcdGet32 
(PcdTcg2HashAlgorithmBitmap));
   HiiSetString (PrivateData->HiiHandle, STRING_TOKEN 
(STR_BIOS_HASH_ALGO_CONTENT), TempBuffer, NULL);
 
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
index a91c052850..b84af40a04 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
@@ -70,6 +70,7 @@ typedef struct {
   UINT8TpmDeviceInterfaceAttempt;
   BOOLEAN  TpmDeviceInterfacePtpFifoSupported;
   BOOLEAN  TpmDeviceInterfacePtpCrbSupported;
+  BOOLEAN  ChangeEPSSupported;
 } TCG2_CONFIGURATION_INFO;
 
 //
-- 
2.26.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61332): https://edk2.groups.io/g/devel/message/61332
Mute This Topic: https://groups.io/mt/74913408/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 0/2] refine TPM2 operation pull down list

2020-06-16 Thread Qi Zhang

v1 change: separate the change into two patches

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Cc: Rahul Kumar 
Signed-off-by: Qi Zhang 

Qi Zhang (1):
  SecurityPkg/Tcg2Config: remove TPM2_ChangEPS if it is not supported.

Zhang, Qi (1):
  SecurityPkg/Tpm2CommandLib: add new function
Tpm2GetCapabilityIsCmdImpl.

 SecurityPkg/Include/Library/Tpm2CommandLib.h| 16 
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 40 

 SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr   |  2 ++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c |  7 +++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h   |  1 +
 5 files changed, 66 insertions(+)

-- 
2.26.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61330): https://edk2.groups.io/g/devel/message/61330
Mute This Topic: https://groups.io/mt/74913406/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load

2020-06-16 Thread Igor Druzhinin

RestoreTPL called while at TPL_HIGH_LEVEL unconditionally enables interrupts
even if called in interrupt handler. That opens a window while interrupt
is not completely handled but another interrupt could be accepted.

If a VM starts on a heavily loaded host hundreds of periodic timer interrupts
might be queued while vCPU is descheduled (the behavior is typical for
a Xen host). The next time vCPU is scheduled again all of them get
delivered back to back causing OVMF to accept each one without finishing
a previous one and cleaning up the stack. That quickly results in stack
overflow and a triple fault.

Fix it by postponing sending EOI until we finished processing the current
tick giving interrupt handler opportunity to clean up the stack before
accepting the next tick.

Signed-off-by: Igor Druzhinin 
---

Laszlo, Anthony,

Do you think it's the right way to address it?

Alternatively, we might avoid calling RaiseTPL in interrupt handler at all
like it's done in HpetTimer implementation for instance.

Or we might try to address it in Raise/RestoreTPL calls by saving/restoring
interrupt state along with TPL.

---
 OvmfPkg/8254TimerDxe/Timer.c  | 5 +++--
 OvmfPkg/XenTimerDxe/XenTimerDxe.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/8254TimerDxe/Timer.c b/OvmfPkg/8254TimerDxe/Timer.c
index 67e22f5..fd1691b 100644
--- a/OvmfPkg/8254TimerDxe/Timer.c
+++ b/OvmfPkg/8254TimerDxe/Timer.c
@@ -79,8 +79,6 @@ TimerInterruptHandler (
 
   OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
-  mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
-
   if (mTimerNotifyFunction != NULL) {
 //
 // @bug : This does not handle missed timer interrupts
@@ -89,6 +87,9 @@ TimerInterruptHandler (
   }
 
   gBS->RestoreTPL (OriginalTPL);
+
+  DisableInterrupts ();
+  mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
 }
 
 /**
diff --git a/OvmfPkg/XenTimerDxe/XenTimerDxe.c 
b/OvmfPkg/XenTimerDxe/XenTimerDxe.c
index 9f9e047..0bec593 100644
--- a/OvmfPkg/XenTimerDxe/XenTimerDxe.c
+++ b/OvmfPkg/XenTimerDxe/XenTimerDxe.c
@@ -61,8 +61,6 @@ TimerInterruptHandler (
 
   OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
-  SendApicEoi();
-
   if (mTimerNotifyFunction != NULL) {
 //
 // @bug : This does not handle missed timer interrupts
@@ -71,6 +69,9 @@ TimerInterruptHandler (
   }
 
   gBS->RestoreTPL (OriginalTPL);
+
+  DisableInterrupts ();
+  SendApicEoi ();
 }
 
 /**
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61329): https://edk2.groups.io/g/devel/message/61329
Mute This Topic: https://groups.io/mt/74913405/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2][PATCH 0/1] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2020-06-16 Thread Pete Batard

This single patch addresses what we consider to be an overlook of the
current EDK2 codeset in that the ReadyToBoot event is only signaled when
a formal Boot Manager option is executed but not when a Platform Recovery
one is, whereas the boot loaders executed in both cases tend to be similar
in terms of requirements, and therefore, since there is no dedicated event
associated with pre Platform Recovery execution, ReadyToBoot should also
apply there.

Especially, this patch is required to fix an issue that we encounter on
the Raspberry Pi platform (https://github.com/pftf/RPi4/issues/64), as
it uses EmbeddedPkg/Drivers/ConsolePrefDxe to set up the graphical console
as default, but the code to switch to graphical is tied to the ReadyToBoot
event being triggered.

This means that, currently, unless users go to the UEFI options to save
their boot preferences, the console defaults to a non-initialized instance
that happens to be serial.

Obviously, this is very problematic as it means that installation of an
OS such as Debian-Linux (which Platform Recovery will boot into if the
ISO content have been extracted to a bootable drive) will occur on the
serial console rather than the graphical console by default, leaving
users, who do not have a serial adapter plugged in, getting only an
unexpected a black screen instead of the Debian installer...

Also, due to the nature of the UEFI firmware for the Raspberry Pi
platform, which actually resides on the bootable USB or SD media rather
than flash, and therefore is something that platform users are excepted
to update on a whim (with the effect of resetting all the UEFI variables
when they do so), and as opposed to what might be the case for other
platforms, it is not reasonable to expect for users to go to their
settings to set up the Boot Manager options so that they don't end up
using Platform Recovery. As a matter of fact, it's the opposite that is
likely to hold true, with most Raspberry Pi users using Platform
Recovery for their boot process. Especially, considering that the
platform defaults should be fine for most users, we do expect that
the bulk of UEFI vanilla Linux installations on the Raspberry Pi 4 are
going to use Platform Recovery instead of Boot Manager.

As such it is crucial that the Platform Recovery and formal Boot Manager
boot processes do behave similarly in terms of ReadyToBoot event being
signaled, hence the reason for this patch.


Note however that this may require a specs update, as the current UEFI
specs for EFI_BOOT_SERVICES.CreateEventEx() have:

> EFI_EVENT_GROUP_READY_TO_BOOT
>   This event group is notified by the system when the Boot Manager
>   is about to load and execute a boot option.

and, once this patch has been applied, we may want to update this
section to mention that it applies to both Boot Manager and Platform
Recovery.

Regards,

/Pete

Pete Batard (1):
  MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform
recovery

 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +
 1 file changed, 9 insertions(+)

-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61327): https://edk2.groups.io/g/devel/message/61327
Mute This Topic: https://groups.io/mt/74912985/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2][PATCH 1/1] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

2020-06-16 Thread Pete Batard

Currently, the ReadyToBoot event is only signaled when a formal Boot
Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).

However, with the introduction of Platform Recovery in UEFI 2.5, which may
lead to the execution of a boot loader that has similar requirements to a
regular one, yet is not launched as a Boot Manager option, it also becomes
necessary to signal ReadyToBoot when a Platform Recovery boot loader runs.

Especially, this can be critical to ensuring that the graphical console
is actually usable during platform recovery, as some platforms do rely on
the ConsolePrefDxe driver, which only performs console initialization after
ReadyToBoot is triggered.

This patch fixes that behaviour by calling EfiSignalEventReadyToBoot () in
EfiBootManagerProcessLoadOption (), which is the function that sets up the
platform recovery boot process.

Signed-off-by: Pete Batard 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 89372b3b97b8..117f1f5b124c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1376,6 +1376,15 @@ EfiBootManagerProcessLoadOption (
 return EFI_SUCCESS;
   }
 
+  //
+  // Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load and 
execute the boot option.
+  //
+  EfiSignalEventReadyToBoot ();
+  //
+  // Report Status Code to indicate ReadyToBoot was signalled
+  //
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | 
EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   //
   // Load and start the load option.
   //
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61328): https://edk2.groups.io/g/devel/message/61328
Mute This Topic: https://groups.io/mt/74912987/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings

2020-06-16 Thread Ard Biesheuvel


On 6/16/20 3:57 AM, Gao, Liming wrote:
This data is great. The change is good. Reviewed-by: Liming Gao 





Submitted as #699 and merged,

Thanks all


*From:*Liu, Zhiguang 
*Sent:* 2020年6月15日15:34
*To:* Ard Biesheuvel ; Gao, Liming 
; devel@edk2.groups.io

*Cc:* Kinney, Michael D 
*Subject:* RE: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid 
absolute addresses for error strings


Hi Ard,

I also collected the image size of OVMFX64 size building with VS2015x86.



Debug



Release



before



after



After-before



before



after



after-before

sec



27664



27409



-255



13968



13968



0

pei



223016



221000



-2016



107208



106984



-224

dxe



4507000



4481336



-25664



2987064



2979384



-7680

compact



1179776



1172528



-7248



922664



920304



-2360

It can reduce the image size in X64.

Reviewed-by: Zhiguang Liu >



-Original Message-



From: Ard Biesheuvel mailto:ard.biesheu...@arm.com>>



Sent: Friday, June 12, 2020 11:11 PM


To: Gao, Liming mailto:liming@intel.com>>; 

devel@edk2.groups.io 

Cc: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; Liu, 

Zhiguang


mailto:zhiguang@intel.com>>



Subject: Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute



addresses for error strings







On 6/12/20 4:33 PM, Gao, Liming wrote:



> Ard:



>    I will collect the image size on OVMF X64 platform with this patch.



>







Building OvmfPkgX64.dsc in RELEASE mode using GCC5 profile gives me











Before:



SECFV [5%Full] 212992 total, 11760 used, 201232 free PEIFV [9%Full] 917504



total, 89384 used, 828120 free DXEFV [22%Full] 12582912 total, 2806320 used,



9776592 free FVMAIN_COMPACT [26%Full] 3440640 total, 918760 used,



2521880 free







After:



SECFV [5%Full] 212992 total, 11760 used, 201232 free PEIFV [9%Full] 917504



total, 89192 used, 828312 free DXEFV [22%Full] 12582912 total, 2802928 used,



9779984 free FVMAIN_COMPACT [26%Full] 3440640 total, 916784 used,



2523856 free







So no change for SECFV, -192 bytes for PEIFV, -3392 bytes for DXEFV and



-1976 bytes for the resulting outer FV image.















>> -Original Message-


>> From: devel@edk2.groups.io  
> On Behalf Of Ard


Biesheuvel



>> Sent: Friday, June 12, 2020 6:35 AM



>> To: devel@edk2.groups.io 



>> Cc: Ard Biesheuvel mailto:ard.biesheu...@arm.com>>; 
Kinney, Michael D


mailto:michael.d.kin...@intel.com>>; Gao, 

Liming mailto:liming@intel.com>>


>> Subject: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute



addresses for error strings



>>



>> The mStatusString[] array is constructed as an array of pointer-to-char,



>> which means that on X64 or AARCH64, it is emitted as a single linear list



>> of 64-bit quantities, each containing the absolute address of one of the



>> string literals in memory.



>>



>> This means that each string takes up 8 bytes of additional space, along



>> with 2 bytes of relocation data. It also means that extra work needs to



>> be done at runtime to process these relocations, every time a module is



>> loaded that incorporates this library.



>>



>> So fix both issues, by splitting mStatusString into two arrays of char



>> arrays. The memory footprint decreases from 955 to 843 bytes, and given



>> that in the latter case, the overhead consists of 278 NUL characters rather



>> than 390 bytes worth of absolute addresses and relocation records, the



size



>> of a compressed image is reduced even further. For example, when



building



>> ArmVirtQemu.dsc in RELEASE mode for AARCH64 with the GCC5 profile, I



get:



>>



>>    Before



>>



>>    FV Space Information



>>    FVMAIN [100%Full] 5329920 total, 5329920 used, 0 free



>>    FVMAIN_COMPACT [38%Full] 2093056 total, 811840 used, 1281216 free



>>



>>    After



>>



>>    FV Space Information



>>    FVMAIN [100%Full] 5321728 total, 5321728 used, 0 free



>>    FVMAIN_COMPACT [38%Full] 2093056 total, 809696 used, 1283360 free



>>



>> So the uncompressed contents of the compressed image are 8 KB smaller,



>> whereas the resulting flash image (consisting of the compressed image



>> along with SEC, PEI_CORE and a set of PEIMs that execute in place) is



>> 2 KB smaller.



>>



>> Cc: "Kinney, Michael D" mailto:michael.d.kin...@intel.com>>



>> Cc: "Gao, Liming" mailto:liming@intel.com>>



>> Signed-off-by: Ard Biesheuvel mailto:ard.biesheu...@arm.com>>



>> ---



>> v3:



>> - add code comments to explain what the inner dimension of each

[edk2-devel] [PATCH 5/5] UefiCpuPkg: Uninstall EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.

2020-06-16 Thread Zhiguang Liu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
To avoid leaking information from SMM, uninstall
EFI_SMM_CONFIGURATION_PROTOCOL at end of Dxe.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Signed-off-by: Zhiguang Liu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 37 
+
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  1 +
 2 files changed, 38 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index db68e1316e..a1b209e125 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -520,6 +520,33 @@ SmmReadyToLockEventNotify (
   return EFI_SUCCESS;
 }
 
+/**
+  SMM End of Dxe event notification handler.
+
+  To avoid leaking information from SMM, uninstall 
EFI_SMM_CONFIGURATION_PROTOCOL
+  at end of Dxe.
+
+  @param[in] Protocol   Points to the protocol's unique identifier.
+  @param[in] Interface  Points to the interface instance.
+  @param[in] Handle The handle on which the interface was installed.
+
+  @retval EFI_SUCCESS   Notification handler runs successfully.
+ **/
+EFI_STATUS
+EFIAPI
+SmmEndOfDxeNotify (
+  IN CONST EFI_GUID  *Protocol,
+  IN VOID*Interface,
+  IN EFI_HANDLE  Handle
+  )
+{
+  gBS->UninstallProtocolInterface (
+ gSmmCpuPrivate->SmmCpuHandle,
+ , >SmmConfiguration
+ );
+  return EFI_SUCCESS;
+}
+
 /**
   The module Entry Point of the CPU SMM driver.
 
@@ -1038,6 +1065,16 @@ PiCpuSmmEntry (
 );
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // register SMM End of Dxe notification
+  //
+  Status = gSmst->SmmRegisterProtocolNotify (
+,
+SmmEndOfDxeNotify,
+
+);
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Initialize SMM Profile feature
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 76b1462996..bb994814d6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -105,6 +105,7 @@
   gEfiSmmConfigurationProtocolGuid ## PRODUCES
   gEfiSmmCpuProtocolGuid   ## PRODUCES
   gEfiSmmReadyToLockProtocolGuid   ## NOTIFY
+  gEfiSmmEndOfDxeProtocolGuid  ## NOTIFY
   gEfiSmmCpuServiceProtocolGuid## PRODUCES
   gEdkiiSmmMemoryAttributeProtocolGuid ## PRODUCES
   gEfiMmMpProtocolGuid## PRODUCES
-- 
2.25.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61323): https://edk2.groups.io/g/devel/message/61323
Mute This Topic: https://groups.io/mt/74912556/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/5] MdeModulePkg: avoid SMM pointers being leaked by not using CopyMem()

2020-06-16 Thread Zhiguang Liu

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

This commit will update the logic in function SmmVariableGetStatistics()
so that the pointer fields ('Next' and 'Name') in structure
VARIABLE_INFO_ENTRY will not be copied into the SMM communication buffer.

Doing so will prevent SMM pointers address from being leaked into non-SMM
environment.

Please note that newly introduced internal function CopyVarInfoEntry()
will not use CopyMem() to copy the whole VARIABLE_INFO_ENTRY structure and
then zero out the 'Next' and 'Name' fields. This is for preventing race
conditions where the pointers value might still be read.

Cc: Star Zeng 
Cc: Liming Gao 
Cc: Jian J Wang 
Signed-off-by: Hao A Wu 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 33 
+++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index caca5c3241..74e756bc00 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -315,6 +315,35 @@ GetFvbCountAndBuffer (
 }
 
 
+/**
+  Copy only the meaningful fields of the variable statistics information from
+  source buffer to the destination buffer. Other fields are filled with zero.
+
+  @param[out]  DstInfoEntryA pointer to the buffer of destination variable
+   information entry.
+  @param[in]   SrcInfoEntryA pointer to the buffer of source variable
+   information entry.
+
+**/
+static
+VOID
+CopyVarInfoEntry (
+  OUT VARIABLE_INFO_ENTRY*DstInfoEntry,
+  IN  VARIABLE_INFO_ENTRY*SrcInfoEntry
+  )
+{
+  DstInfoEntry->Next = NULL;
+  DstInfoEntry->Name = NULL;
+
+  CopyGuid (>VendorGuid, >VendorGuid);
+  DstInfoEntry->Attributes  = SrcInfoEntry->Attributes;
+  DstInfoEntry->ReadCount   = SrcInfoEntry->ReadCount;
+  DstInfoEntry->WriteCount  = SrcInfoEntry->WriteCount;
+  DstInfoEntry->DeleteCount = SrcInfoEntry->DeleteCount;
+  DstInfoEntry->CacheCount  = SrcInfoEntry->CacheCount;
+  DstInfoEntry->Volatile= SrcInfoEntry->Volatile;
+}
+
 /**
   Get the variable statistics information from the information buffer pointed 
by gVariableInfo.
 
@@ -377,7 +406,7 @@ SmmVariableGetStatistics (
   *InfoSize = StatisticsInfoSize;
   return EFI_BUFFER_TOO_SMALL;
 }
-CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));
+CopyVarInfoEntry (InfoEntry, VariableInfo);
 CopyMem (InfoName, VariableInfo->Name, NameSize);
 *InfoSize = StatisticsInfoSize;
 return EFI_SUCCESS;
@@ -417,7 +446,7 @@ SmmVariableGetStatistics (
 return EFI_BUFFER_TOO_SMALL;
   }
 
-  CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY));
+  CopyVarInfoEntry (InfoEntry, VariableInfo);
   CopyMem (InfoName, VariableInfo->Name, NameSize);
   *InfoSize = StatisticsInfoSize;
 
-- 
2.25.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61324): https://edk2.groups.io/g/devel/message/61324
Mute This Topic: https://groups.io/mt/74912557/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 4/5] SecurityPkg: Remove DXE_SMM_DRIVER support for some libraries

2020-06-16 Thread Zhiguang Liu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
Remove DXE_SMM_DRIVER support for some libraries because they
have the risks of leaking data from SMM mode to non-SMM mode.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Signed-off-by: Zhiguang Liu 
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
index 1e1a639857..9494d04b1d 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
@@ -20,7 +20,7 @@
   FILE_GUID  = 0CA970E1-43FA-4402-BC0A-81AF336BFFD6
   MODULE_TYPE= DXE_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = NULL|DXE_DRIVER DXE_RUNTIME_DRIVER 
DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS  = NULL|DXE_DRIVER DXE_RUNTIME_DRIVER 
UEFI_APPLICATION UEFI_DRIVER
   CONSTRUCTOR= DxeImageVerificationLibConstructor
 
 #
-- 
2.25.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61322): https://edk2.groups.io/g/devel/message/61322
Mute This Topic: https://groups.io/mt/74912554/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/5] MdePkg: Remove DXE_SMM_DRIVER support for some libraries

2020-06-16 Thread Zhiguang Liu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
Remove DXE_SMM_DRIVER support for some libraries because they
have the risks of leaking data from SMM mode to non-SMM mode.

Cc: Kinney Michael D 
Cc: Gao Liming 
Signed-off-by: Zhiguang Liu 
---
 MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf | 2 +-
 MdePkg/Library/DxeHstiLib/DxeHstiLib.inf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf 
b/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
index 33eeab405f..acbe62e352 100644
--- a/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
+++ b/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
@@ -17,7 +17,7 @@
   FILE_GUID  = f773469b-e265-4b0c-b0a6-2f971fbfe72b
   MODULE_TYPE= DXE_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = ExtractGuidedSectionLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS  = ExtractGuidedSectionLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
 
   CONSTRUCTOR= DxeExtractGuidedSectionLibConstructor
 
diff --git a/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf 
b/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
index c719481cdd..8c1cb3d0cc 100644
--- a/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
+++ b/MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
@@ -14,7 +14,7 @@
   FILE_GUID  = 7DE1C620-F587-4116-A36D-40F3467B9A0C
   MODULE_TYPE= DXE_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = HstiLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS  = HstiLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
 
 [Sources]
   HstiAip.c
-- 
2.25.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61321): https://edk2.groups.io/g/devel/message/61321
Mute This Topic: https://groups.io/mt/74912553/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI LOADED_IMAGE from SMM.

2020-06-16 Thread Zhiguang Liu

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

This patch removes EFI_LOADED_IMAGE_PROTOCOL from DXE database.
The SmmCore only install EFI_LOADED_IMAGE_PROTOCOL to SMM database.

Exposing LOADED_IMAGE_PROTOCOL may cause SMM information leakage
to non-SMM component.

Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Signed-off-by: Jiewen Yao 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 104 
+++-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c  |  34 
--
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h  |   4 ++--
 3 files changed, 13 insertions(+), 129 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c 
b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
index 76ee9e0b89..2be2866234 100644
--- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
+++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
@@ -316,7 +316,7 @@ SmmLoadImage (
   EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
   PE_COFF_LOADER_IMAGE_CONTEXT   ImageContext;
 
-  PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle);
+  PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle);
 
   Buffer   = NULL;
   Size = 0;
@@ -546,51 +546,14 @@ SmmLoadImage (
   DriverEntry->ImageBuffer  = DstBuffer;
   DriverEntry->NumberOfPage = PageCount;
 
-  //
-  // Allocate a Loaded Image Protocol in EfiBootServicesData
-  //
-  Status = gBS->AllocatePool (EfiBootServicesData, sizeof 
(EFI_LOADED_IMAGE_PROTOCOL), (VOID **)>LoadedImage);
-  if (EFI_ERROR (Status)) {
-if (Buffer != NULL) {
-  gBS->FreePool (Buffer);
-}
-SmmFreePages (DstBuffer, PageCount);
-return Status;
-  }
-
-  ZeroMem (DriverEntry->LoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL));
   //
   // Fill in the remaining fields of the Loaded Image Protocol instance.
-  // Note: ImageBase is an SMRAM address that can not be accessed outside of 
SMRAM if SMRAM window is closed.
   //
-  DriverEntry->LoadedImage->Revision  = EFI_LOADED_IMAGE_PROTOCOL_REVISION;
-  DriverEntry->LoadedImage->ParentHandle  = gSmmCorePrivate->SmmIplImageHandle;
-  DriverEntry->LoadedImage->SystemTable   = gST;
-  DriverEntry->LoadedImage->DeviceHandle  = DeviceHandle;
-
   DriverEntry->SmmLoadedImage.Revision = 
EFI_LOADED_IMAGE_PROTOCOL_REVISION;
   DriverEntry->SmmLoadedImage.ParentHandle = 
gSmmCorePrivate->SmmIplImageHandle;
   DriverEntry->SmmLoadedImage.SystemTable  = gST;
   DriverEntry->SmmLoadedImage.DeviceHandle = DeviceHandle;
 
-  //
-  // Make an EfiBootServicesData buffer copy of FilePath
-  //
-  Status = gBS->AllocatePool (EfiBootServicesData, GetDevicePathSize 
(FilePath), (VOID **)>LoadedImage->FilePath);
-  if (EFI_ERROR (Status)) {
-if (Buffer != NULL) {
-  gBS->FreePool (Buffer);
-}
-SmmFreePages (DstBuffer, PageCount);
-return Status;
-  }
-  CopyMem (DriverEntry->LoadedImage->FilePath, FilePath, GetDevicePathSize 
(FilePath));
-
-  DriverEntry->LoadedImage->ImageBase = (VOID *)(UINTN) 
ImageContext.ImageAddress;
-  DriverEntry->LoadedImage->ImageSize = ImageContext.ImageSize;
-  DriverEntry->LoadedImage->ImageCodeType = EfiRuntimeServicesCode;
-  DriverEntry->LoadedImage->ImageDataType = EfiRuntimeServicesData;
-
   //
   // Make a buffer copy of FilePath
   //
@@ -599,7 +562,6 @@ SmmLoadImage (
 if (Buffer != NULL) {
   gBS->FreePool (Buffer);
 }
-gBS->FreePool (DriverEntry->LoadedImage->FilePath);
 SmmFreePages (DstBuffer, PageCount);
 return Status;
   }
@@ -610,16 +572,6 @@ SmmLoadImage (
   DriverEntry->SmmLoadedImage.ImageCodeType = EfiRuntimeServicesCode;
   DriverEntry->SmmLoadedImage.ImageDataType = EfiRuntimeServicesData;
 
-  //
-  // Create a new image handle in the UEFI handle database for the SMM Driver
-  //
-  DriverEntry->ImageHandle = NULL;
-  Status = gBS->InstallMultipleProtocolInterfaces (
-  >ImageHandle,
-  , DriverEntry->LoadedImage,
-  NULL
-  );
-
   //
   // Create a new image handle in the SMM handle database for the SMM Driver
   //
@@ -631,7 +583,7 @@ SmmLoadImage (
  >SmmLoadedImage
  );
 
-  PERF_LOAD_IMAGE_END (DriverEntry->ImageHandle);
+  PERF_LOAD_IMAGE_END (DriverEntry->SmmImageHandle);
 
   //
   // Print the load address and the PDB file name if it is available
@@ -856,7 +808,7 @@ SmmDispatcher (
   // Untrused to Scheduled it would have already been loaded so we may 
need to
   // skip the LoadImage
   //
-  if (DriverEntry->ImageHandle == NULL) {
+  if (DriverEntry->SmmImageHandle == NULL) {
 Status = SmmLoadImage (DriverEntry);
 
 //
@@ -885,8 +837,8 @@ SmmDispatcher (
   REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
 EFI_PROGRESS_CODE,
 EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_BEGIN,
->ImageHandle,
-sizeof (DriverEntry->ImageHandle)
+

Re: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files

2020-06-16 Thread Bob Feng

Hi Pierre,

Yes. It's fine for me. 


Thanks,
Bob

-Original Message-
From: Pierre Gondois  
Sent: Monday, June 15, 2020 11:58 PM
To: Feng, Bob C ; devel@edk2.groups.io
Cc: Gao, Liming ; Sami Mujawar ; 
Tomas Pilar ; nd 
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules 
when multiple output files

Hello Bob,
I have locally corrected the patch. On non-arm architecture, I had previously 
tested it for the following configurations, but this wasn't enough apparently. 
I am currently testing it on more platforms and I will send you the list of the 
tested platforms along with the V2.
Plarform="Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc"|"Platform/Intel/BoardModulePkg/BoardModulePkg.dsc"
  Host=Windows|Linux
BuildType=DEBUG|RELEASE
  Toolchain=VS2017|GCC5
Arch=X64|IA32
Before submitting a V2, I am planning to create a pull request on the edk2 
github repository, allowing me to check whether it passes the CI tests. Would 
it be fine for you?

About your comment about changing the logic of ApplyBuildRule, the logic of 
_ApplyBuildRule currently breaks out of the loop whenever a final target is 
found. The new rule on ".amli" files needs to continue looking for rules to 
apply, otherwise the processing on the initial ASL file stops when the AML file 
is generated (thus, not continuing the path ".amli"->".c"->".obj"). I agree 
this is hard to check, but I believe the edk2 CI tests should allow to put more 
trust in the patch serie.

Regards,
Pierre

-Original Message-
From: Feng, Bob C 
Sent: 10 June 2020 04:58
To: Pierre Gondois ; devel@edk2.groups.io
Cc: Gao, Liming ; Sami Mujawar ; 
Tomas Pilar ; nd 
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules 
when multiple output files

Hi Pierre,

I found there is an incorrect target generated in the OvmfPkg/AcpiTables 
Makefile when I tried to build Ovmf. That incorrect target causes ovmf to build 
failed.


$(DEBUG_DIR)\PlatformAcpiTables : $(MAKE_FILE) $(DEBUG_DIR)\PlatformAcpiTables 
: $(STATIC_LIBRARY_FILES) $(DEBUG_DIR)\PlatformAcpiTables : 
$(STATIC_LIBRARY_FILES_LIST)
"$(DLINK)" $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)

Regarding this patch, I think it changes the logic of the _ApplyBuildRule, 
replacing "break" with "continue" and removing some if and elseif blocks, so it 
would be hard for me to make sure your current logic can cover all the original 
cases.  Would you show me how many testing you have done?

Thanks,
Bob 

-Original Message-
From: Pierre Gondois 
Sent: Monday, June 8, 2020 10:01 PM
To: Feng, Bob C ; devel@edk2.groups.io
Cc: Gao, Liming ; Sami Mujawar ; 
Tomas Pilar ; nd 
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules 
when multiple output files

Hello Bob,
Should I modify the patch ?

Regards,
Pierre

-Original Message-
From: Pierre Gondois
Sent: Tuesday, June 2, 2020 2:04 PM
To: Feng, Bob C ; devel@edk2.groups.io
Cc: Gao, Liming ; Sami Mujawar ; 
Tomas Pilar ; nd 
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules 
when multiple output files

Hello Bob,
Thank you for your answer,
I put my comments as [Pierre],

Regards,
Pierre


-Original Message-
From: Feng, Bob C 
Sent: 02 June 2020 12:16
To: devel@edk2.groups.io; Pierre Gondois 
Cc: Gao, Liming ; Sami Mujawar ; 
Tomas Pilar ; nd 
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules 
when multiple output files

My comments are inline marked as [Bob].

Thanks,
Bob

-Original Message-
From: devel@edk2.groups.io  On Behalf Of PierreGondois
Sent: Monday, May 18, 2020 10:11 PM
To: devel@edk2.groups.io
Cc: Pierre Gondois ; Feng, Bob C 
; Gao, Liming ; 
sami.muja...@arm.com; tomas.pi...@arm.com; n...@arm.com
Subject: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when 
multiple output files

From: Pierre Gondois 

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

This patch modifies the Makefile generation not to stop adding Makfile rules 
when the first final target is found.
E.g.:
If the following rules are described in build_rule.txt:
 -[Rule1]: .X files generate .Y and .Z files;
 -[Rule2]: .Z files generate .Z1 files.
Currently, if a File1.X file was part of the sources of a module, only [Rule1] 
would be generated in the Makefile.
Indeed, there are no rules to apply to .Y files: .Y files are a final target. 
However, there is still [Rule2] to apply to .Z files.

[Bob] I think currently a rule's output file will be added back to source file 
list, and in the later loop, that output file will be handled by another rule.  
Doesn't that algorithm handle your case above?
[Pierre]
The rule's output file was effectively added to the list of source files, and a 
rule was searched  for this output file. However, the loop stopped when the 
first final target was found. By final target I mean "a file that isn't the 
input of a rule".
For the asl/aml/amli

Re: [edk2-devel] [PATCH v2 0/3] add DwMmcHcDxe driver

2020-06-16 Thread Loh, Tien Hock

Hi Leif, Ard,

I talked to Haojian and got to know that you wanted the patch to go into the 
MdeModulePkg. 
I don't have a lot of context on it, do you have specific requirement?
The driver at its current state already uses the MdeModulePkg's SdDxe's API, so 
some more specific changes would help me understand and fix the concern you 
have.

Thanks

> -Original Message-
> From: Loh, Tien Hock
> Sent: Thursday, September 12, 2019 12:59 PM
> To: Haojian Zhuang ; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org; christopher...@microsoft.com
> Cc: devel@edk2.groups.io; thlo...@gmail.com
> Subject: RE: [PATCH v2 0/3] add DwMmcHcDxe driver
> 
> Hi Ard, Leif, Christopher,
> 
> Any comments on the patches?
> 
> Thanks!
> Tien Hock
> > -Original Message-
> > From: Haojian Zhuang 
> > Sent: Monday, September 2, 2019 5:31 PM
> > To: Loh, Tien Hock 
> > Cc: leif.lindh...@linaro.org; ard.biesheu...@linaro.org;
> > christopher...@microsoft.com; devel@edk2.groups.io; thlo...@gmail.com
> > Subject: Re: [PATCH v2 0/3] add DwMmcHcDxe driver
> >
> > Hi Leif, Ard, Christopher,
> >
> > Could you help to share the comments on this patch set? Thanks a lot.
> >
> > Best Regards
> > Haojian
> >
> > On Thu, Aug 15, 2019 at 09:09:19AM +, Loh, Tien Hock wrote:
> > > Hi Leif, Ard, Christopher,
> > >
> > > Haojian and I have tested the driver on 2 platforms, any further
> > > comments
> > on this?
> > >
> > > Thanks
> > > Tien Hock
> > >
> > > > -Original Message-
> > > > From: Haojian Zhuang 
> > > > Sent: Tuesday, July 30, 2019 3:33 PM
> > > > To: Loh, Tien Hock ;
> > > > leif.lindh...@linaro.org; ard.biesheu...@linaro.org;
> > > > christopher...@microsoft.com
> > > > Cc: devel@edk2.groups.io; thlo...@gmail.com
> > > > Subject: Re: [PATCH v2 0/3] add DwMmcHcDxe driver
> > > >
> > > > On Wed, Jul 24, 2019 at 05:26:03PM +0800, tien.hock@intel.com
> > wrote:
> > > > > From: "Tien Hock, Loh" 
> > > > >
> > > > > Changelog:
> > > > > v3:
> > > > >   * Fix an issue in NonDiscoverableDeviceDxe driver where it did
> > > > > not
> > > > invalidate
> > > > > cache before copying the memory.
> > > > > v2:
> > > > >   *Split DwMmcHcDxe driver into two patches. One is for
> > > > > PlatformDwMmc
> > > > protocol,
> > > > >and the other is for DwMmcHcDxe driver.
> > > > > v1:
> > > > >   *Add NonDiscoverableDeviceDxe for embedded platform. Make
> > > > DwMmcHcDxe driver
> > > > >to support both eMMC and SD controller.
> > > > >
> > > > > Haojian Zhuang (3):
> > > > >   EmbeddedPkg: add NonDiscoverableDeviceDxe driver
> > > > >   EmbeddedPkg: add PlatformDwMmc protocol
> > > > >   EmbeddedPkg/Drivers: add DwMmcHcDxe driver
> > > > >
> > > > >  .../Drivers/DwMmcHcDxe/ComponentName.c|  214 ++
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.c   | 1295
> > > > +
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.dec |   40 +
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.h   |  815
> > ++
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.inf |   69 +
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHci.c | 2366
> > > > +
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHci.h |  983 +++
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/EmmcDevice.c   | 1042
> > 
> > > > >  EmbeddedPkg/Drivers/DwMmcHcDxe/SdDevice.c | 1104 
> > > > >  EmbeddedPkg/EmbeddedPkg.dec   |1 +
> > > > >  EmbeddedPkg/Include/Protocol/PlatformDwMmc.h  |   79 +
> > > > >  .../NonDiscoverableDeviceDxe/ComponentName.c  |  124 +
> > > > >  .../NonDiscoverableDeviceDxe.c|  243 ++
> > > > >  .../NonDiscoverableDeviceDxe.inf  |   52 +
> > > > >  .../NonDiscoverableDeviceIo.c |  976 +++
> > > > >  .../NonDiscoverableDeviceIo.h |   92 +
> > > > >  16 files changed, 9495 insertions(+)  create mode 100644
> > > > EmbeddedPkg/Drivers/DwMmcHcDxe/ComponentName.c
> > > > >  create mode 100644
> > > > EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.c
> > > > >  create mode 100644
> > > > EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.dec
> > > > >  create mode 100644
> > > > EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.h
> > > > >  create mode 100644
> > > > EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHcDxe.inf
> > > > >  create mode 100644
> > EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHci.c
> > > > >  create mode 100644
> > EmbeddedPkg/Drivers/DwMmcHcDxe/DwMmcHci.h
> > > > >  create mode 100644
> > EmbeddedPkg/Drivers/DwMmcHcDxe/EmmcDevice.c
> > > > >  create mode 100644 EmbeddedPkg/Drivers/DwMmcHcDxe/SdDevice.c
> > > > >  create mode 100644
> > EmbeddedPkg/Include/Protocol/PlatformDwMmc.h
> > > > >  create mode 100644
> > > > >
> > EmbeddedPkg/Universal/NonDiscoverableDeviceDxe/ComponentName.c
> > > > >  create mode 100644
> > > > >
> > > >
> > EmbeddedPkg/Universal/NonDiscoverableDeviceDxe/NonDiscoverableDevic
> > > > eDx
> > > > > e.c  create mode 100644
> > > > >
> > > >
> >

Re: [edk2-devel] [PATCH v9 40/46] UefiCpuPkg: Add a 16-bit protected mode code segment descriptor

2020-06-16 Thread Dong, Eric

Reviewed-by: Eric Dong 

> -Original Message-
> From: Tom Lendacky 
> Sent: Friday, June 5, 2020 9:28 PM
> To: devel@edk2.groups.io
> Cc: Brijesh Singh ; Ard Biesheuvel
> ; Dong, Eric ; Justen,
> Jordan L ; Laszlo Ersek ;
> Gao, Liming ; Kinney, Michael D
> ; Ni, Ray 
> Subject: [PATCH v9 40/46] UefiCpuPkg: Add a 16-bit protected mode code
> segment descriptor
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> A hypervisor is not allowed to update an SEV-ES guests register state, so
> when booting an SEV-ES guest AP, the hypervisor is not allowed to set the
> RIP to the guest requested value. Instead, an SEV-ES AP must be transition
> from 64-bit long mode to 16-bit real mode in response to an INIT-SIPI-SIPI
> sequence. This requires a 16-bit code segment descriptor.
> For PEI, create this descriptor in the reset vector GDT table. For DXE, create
> this descriptor from the newly reserved entry at location 0x28.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Tom Lendacky 
> ---
>  UefiCpuPkg/CpuDxe/CpuGdt.h  | 4 ++--
>  UefiCpuPkg/CpuDxe/CpuGdt.c  | 8 
>  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 9 +
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.h b/UefiCpuPkg/CpuDxe/CpuGdt.h
> index 3a0210b2f172..1c94487cbee8 100644
> --- a/UefiCpuPkg/CpuDxe/CpuGdt.h
> +++ b/UefiCpuPkg/CpuDxe/CpuGdt.h
> @@ -36,7 +36,7 @@ struct _GDT_ENTRIES {
>GDT_ENTRY LinearCode;   GDT_ENTRY SysData;   GDT_ENTRY SysCode;-
> GDT_ENTRY Spare4;+  GDT_ENTRY SysCode16;   GDT_ENTRY LinearData64;
> GDT_ENTRY LinearCode64;   GDT_ENTRY Spare5;@@ -49,7 +49,7 @@ struct
> _GDT_ENTRIES {
>  #define LINEAR_CODE_SEL   OFFSET_OF (GDT_ENTRIES, LinearCode)
> #define SYS_DATA_SEL  OFFSET_OF (GDT_ENTRIES, SysData) #define
> SYS_CODE_SEL  OFFSET_OF (GDT_ENTRIES, SysCode)-#define SPARE4_SEL
> OFFSET_OF (GDT_ENTRIES, Spare4)+#define SYS_CODE16_SELOFFSET_OF
> (GDT_ENTRIES, SysCode16) #define LINEAR_DATA64_SEL OFFSET_OF
> (GDT_ENTRIES, LinearData64) #define LINEAR_CODE64_SEL OFFSET_OF
> (GDT_ENTRIES, LinearCode64) #define SPARE5_SELOFFSET_OF
> (GDT_ENTRIES, Spare5)diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c
> b/UefiCpuPkg/CpuDxe/CpuGdt.c
> index 64efadeba601..a1ab543f2da5 100644
> --- a/UefiCpuPkg/CpuDxe/CpuGdt.c
> +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c
> @@ -70,14 +70,14 @@ STATIC GDT_ENTRIES GdtTemplate = {
>  0x0,   },   //-  // SPARE4_SEL+  // SYS_CODE16_SEL   //   {-0x0, 
>// limit
> 15:0+0x0,// limit 15:0 0x0,// base 15:0 
> 0x0,// base
> 23:16-0x0,// type-0x0,// limit 19:16, flags+  
>   0x09A,  //
> present, ring 0, code, execute/read+0x08F,  // page-granular, 
> 16-bit
> 0x0,// base 31:24   },   //diff --git
> a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> index ce4ebfffb688..0e79a3984b16 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> @@ -129,5 +129,14 @@ LINEAR_CODE64_SEL   equ $-GDT_BASE
>  DB  0; base 31:24 %endif +; linear code segment
> descriptor+LINEAR_CODE16_SEL equ $-GDT_BASE+DW  0x   ; 
> limit
> 15:0+DW  0; base 15:0+DB  0; base 
> 23:16+DB
> PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(CODE32_TYPE)+
> DB
> GRANULARITY_FLAG(1)|DEFAULT_SIZE32(0)|CODE64_FLAG(0)|UPPER_LIMI
> T(0xf)+DB  0; base 31:24+ GDT_END: --
> 2.27.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61318): https://edk2.groups.io/g/devel/message/61318
Mute This Topic: https://groups.io/mt/74698711/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platform][PATCH v2 1/2] Platforms/RaspberryPi: Add RPi4 settings to Readme

2020-06-16 Thread Pete Batard


On 2020.06.15 22:43, Samer El-Haj-Mahmoud wrote:

Add a section to the the RPi4 Readme for 'Configuration Settings', with
instructions on scripting from the UEFI Shell.

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Signed-off-by: Samer El-Haj-Mahmoud 
---
  Platform/RaspberryPi/RPi4/Readme.md | 74 +++-
  1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi4/Readme.md 
b/Platform/RaspberryPi/RPi4/Readme.md
index 03eb6c391aca..98388e3caba1 100644
--- a/Platform/RaspberryPi/RPi4/Readme.md
+++ b/Platform/RaspberryPi/RPi4/Readme.md
@@ -143,4 +143,76 @@ all functionality may be available.
  
  - Network booting via onboard NIC.

  - SPCR hardcodes type to PL011, and thus will not expose correct
-  (miniUART) UART if DT overlays to switch UART are used on Pi 4B.
\ No newline at end of file
+  (miniUART) UART if DT overlays to switch UART are used on Pi 4B.
+
+# Configuration Settings
+The Raspberry Pi UEFI configuration settings can be viewed and changed using both 
the UI configuration menu (under `Device Manager` -> `Raspberry Pi 
Configuration`), as well as the UEFI Shell. To configure using the UEFI Shell, use 
`setvar` command to read/write the UEFI variables with GUID = 
`CD7CC258-31DB-22E6-9F22-63B0B8EED6B5`.
+
+The syntax to read a setting is:
+```
+setvar  -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5
+```
+
+The syntax to write a setting is:
+```
+setvar  -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5 -bs -rt -nv =
+```
+
+For string-type settings (e.g. Asset Tag), the syntax to write is:
+```
+setvar  -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5 -bs -rt -nv 
=L"" =0x
+```
+
+UEFI Setting |NAME   |  VALUE
+-|---|-
+**CPU Configuration**|
+CPU Clock| `CpuClock` | Low = `0x` Default = `0x0001` 
(default) Max = `0x0002` Custom = `0x0003`
+CPU Clock Rate (MHz) | `CustomCpuClock` | Hex numeric value, 
4-bytes (e.g. `0x05DC` for 1500 MHz)
+**Display Configuration**|
+Virtual 640x480  | `DisplayEnableScaledVModes` | Checked = Bit 0 set 
(i.e.  ` \| 0x01`)
+Virtual 800x600  | `DisplayEnableScaledVModes` | Checked = Bit 1 set 
(i.e.  ` \| 0x02`)
+Virtual 1024x768 | `DisplayEnableScaledVModes` | Checked = Bit 2 set 
(i.e.  ` \| 0x04`)
+Virtual 720p | `DisplayEnableScaledVModes` | Checked = Bit 3 set 
(i.e.  ` \| 0x08`)
+Virtual 1080p| `DisplayEnableScaledVModes` | Checked = Bit 4 set 
(i.e.  ` \| 0x10`)
+Native resolution| `DisplayEnableScaledVModes` | Checked = Bit 5 set 
(i.e.  ` \| 0x20`) (default)
+Screenshot support   | `DisplayEnableSShot` | Control-Alt-F12 = `0x0001` 
(default) Not Enabled = `0x`
+**Advanced Configuration**   |
+Limit RAM to 3 GB| `RamLimitTo3GB` | Disable = `0x`  
Enabled= `0x0001` (default)
+System Table Selection   | `SystemTableMode`| ACPI = `0x` (default) 
ACPI + Devicetree = `0x0001`  Devicetree = `0x0002`
+Asset Tag| `AssetTag` | String, 32 characters or less (e.g. 
`L"ABCD123"`) (default `L""`)
+**SD/MMC Configuration** |
+uSD/eMMC Routing | `SdIsArasan` | Arasan SDHC = `0x0001` (default) 
 eMMC2 SDHCI = `0x`
+Multi-Block Support  | `MmcDisableMulti` | Multi-block transfers = 
`0x` (default) Single block transfers = `0x0001`
+uSD Max Bus Width| `MmcForce1Bit` | 4-bit Mode = `0x`  
(default) 1-bit Mode = `0x0001`
+uSD Force Default Speed  | `MmcForceDefaultSpeed` | Allow High Speed = 
`0x` (default) Force Default Speed = `0x0001`
+SD Default Speed (MHz)   | `MmcSdDefaultSpeedMHz` | Hex numeric value, 4-bytes 
(e.g. `0x0019` for 25 MHz)(default 25)
+SD High Speed (MHz)  | `MmcSdHighSpeedMHz` | Hex numeric value, 4-bytes 
(e.g. `0x0032` for 50 MHz)(default 50)
+**Debugging Configuration**  |
+JTAG Routing | `DebugEnableJTAG` | Enable JTAG via GPIO = 
`0x0001` Disable JTAG= `0x` (default)
+
+**Examples:**
+
+- To read the 'System Table Selection' setting :
+```
+setvar SystemTableMode -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5
+```
+
+- To change the 'System Table Selection' setting to 'Devicetree' :
+```
+setvar SystemTableMode -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5 -bs -rt -nv 
=0x0002
+```
+
+- To read the 'Limit RAM to 3 GB' setting:
+```
+setvar RamLimitTo3GB -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5
+```
+
+- To change the 'Limit RAM to 3 GB' setting to 'Disabled':
+```
+setvar RamLimitTo3GB -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5 -bs -rt -nv 
=0x
+```
+
+- To change the Asset Tag to the string "ASSET-TAG-123" :
+```
+setvar AssetTag -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5 -bs -rt -nv 
=L"ASSET-TAG-123" =0x
+```

Re: [edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Tue, 06/16/2020 6:30pm-7:30pm #cal-reminder

2020-06-16 Thread Liming Gao

Few new bugs are this week.

2802
EDK2
Code
michael.kuba...@microsoft.com
UNCO
---
Extend usage of LastAttemptStatus in 
FmpDxe
18:48:53
2808
EDK2
Code
curtis.a.mac...@intel.com
UNCO
---
Implement authentication mechanism for UEFI BIOS 
sub-region
16:55:44
2807
EDK2
Tools
michael.d.kin...@intel.com
UNCO
---
Build intermittently breaks with "make: *** No rule to make target 'tbuild'. 
Stop."
14:47:55
2796
EDK2
Code
f...@gpiccoli.net
UNCO
---
PCI passthrough impaired due to very limiting PCI64 aperture - heuristic should 
be used instead of hardcoded 
window
Mon 01:37

This link lists the current owner & tracker status.
https://bugzilla.tianocore.org/report.cgi?x_axis_field=bug_status_axis_field=assigned_to_axis_field=_redirect=1_format=report-table_desc_type=allwordssubstr_desc==---_type=allwordssubstr=_file_loc_type=allwordssubstr_file_loc=_type=allwords===_id=_id_type=anyexact_to1=1=substring=_to2=1=1=1=substring==1=substringNow_top=AND=noop=noop==table=wrap

Thanks
Liming
From: devel@edk2.groups.io 
Sent: Tuesday, June 16, 2020 9:30 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Tue, 
06/16/2020 6:30pm-7:30pm #cal-reminder


Reminder: TianoCore Bug Triage - APAC / NAMO

When: Tuesday, 16 June 2020, 6:30pm to 7:30pm, (GMT-07:00) America/Los Angeles

Where:https://bluejeans.com/889357567?src=join_info

View Event

Organizer: Brian Richardson 
brian.richard...@intel.com

Description:
https://www.tianocore.org/bug-triage

Meeting URL
https://bluejeans.com/889357567?src=join_info

Meeting ID
889 357 567

Want to dial in from a phone?
Dial one of the following numbers:
+1.408.740.7256 (US (San Jose))
+1.408.317.9253 (US (Primary, San Jose))

(see all numbers - https://www.bluejeans.com/numbers)
Enter the meeting ID and passcode followed by #


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61316): https://edk2.groups.io/g/devel/message/61316
Mute This Topic: https://groups.io/mt/74908579/21656
Mute #cal-reminder: https://groups.io/g/edk2/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

58 matches

Mail list logo