[edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for NVME

2017-03-09 Thread Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Library/UefiBootManagerLib/BmBootDescription.c | 104 -
 .../Library/UefiBootManagerLib/InternalBm.h|   4 +-
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 3 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
index 050647d..501a0cc 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
@@ -1,7 +1,7 @@
 /** @file
   Library functions which relate with boot option description.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -501,6 +501,107 @@ BmGetLoadFileDescription (
 }
 
 /**
+  Return the boot description for NVME boot device.
+
+  @param HandleController handle.
+
+  @return  The description string.
+**/
+CHAR16 *
+BmGetNvmeDescription (
+  IN EFI_HANDLE  Handle
+  )
+{
+  EFI_STATUS   Status;
+  EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL   *NvmePassthru;
+  EFI_DEV_PATH_PTR DevicePath;
+  EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET CommandPacket;
+  EFI_NVM_EXPRESS_COMMAND  Command;
+  EFI_NVM_EXPRESS_COMPLETION   Completion;
+  NVME_ADMIN_CONTROLLER_DATA   ControllerData;
+  CHAR16   *Description;
+  CHAR16   *Char;
+  UINTNIndex;
+
+  Status = gBS->HandleProtocol (Handle, , (VOID **) 
);
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+
+  Status = gBS->LocateDevicePath (, 
, );
+  if (EFI_ERROR (Status) ||
+  (DevicePathType (DevicePath.DevPath) != MESSAGING_DEVICE_PATH) ||
+  (DevicePathSubType (DevicePath.DevPath) != MSG_NVME_NAMESPACE_DP)) {
+//
+// Do not return description when the Handle is not a child of NVME 
controller.
+//
+return NULL;
+  }
+
+  //
+  // Send ADMIN_IDENTIFY command to NVME controller to get the model and 
serial number.
+  //
+  Status = gBS->HandleProtocol (Handle, , 
(VOID **) );
+  ASSERT_EFI_ERROR (Status);
+
+  ZeroMem (, sizeof(EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET));
+  ZeroMem (, sizeof(EFI_NVM_EXPRESS_COMMAND));
+  ZeroMem (, sizeof(EFI_NVM_EXPRESS_COMPLETION));
+
+  Command.Cdw0.Opcode = NVME_ADMIN_IDENTIFY_CMD;
+  //
+  // According to Nvm Express 1.1 spec Figure 38, When not used, the field 
shall be cleared to 0h.
+  // For the Identify command, the Namespace Identifier is only used for the 
Namespace data structure.
+  //
+  Command.Nsid= 0;
+  CommandPacket.NvmeCmd= 
+  CommandPacket.NvmeCompletion = 
+  CommandPacket.TransferBuffer = 
+  CommandPacket.TransferLength = sizeof (ControllerData);
+  CommandPacket.CommandTimeout = EFI_TIMER_PERIOD_SECONDS (5);
+  CommandPacket.QueueType  = NVME_ADMIN_QUEUE;
+  //
+  // Set bit 0 (Cns bit) to 1 to identify a controller
+  //
+  Command.Cdw10= 1;
+  Command.Flags= CDW10_VALID;
+
+  Status = NvmePassthru->PassThru (
+   NvmePassthru,
+   0,
+   ,
+   NULL
+   );
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+
+  Description = AllocateZeroPool (
+  (ARRAY_SIZE (ControllerData.Mn) + 1
+   + ARRAY_SIZE (ControllerData.Sn) + 1
+   + MAXIMUM_VALUE_CHARACTERS + 1
+   ) * sizeof (CHAR16));
+  if (Description != NULL) {
+Char = Description;
+for (Index = 0; Index < ARRAY_SIZE (ControllerData.Mn); Index++) {
+  *(Char++) = (CHAR16) ControllerData.Mn[Index];
+}
+*(Char++) = L' ';
+for (Index = 0; Index < ARRAY_SIZE (ControllerData.Sn); Index++) {
+  *(Char++) = (CHAR16) ControllerData.Sn[Index];
+}
+*(Char++) = L' ';
+UnicodeValueToStringS (
+  Char, sizeof (CHAR16) * (MAXIMUM_VALUE_CHARACTERS + 1),
+  0, DevicePath.NvmeNamespace->NamespaceId, 0
+  );
+BmEliminateExtraSpaces (Description);
+  }
+
+  return Description;
+}
+
+/**
   Return the boot description for the controller based on the type.
 
   @param HandleController handle.
@@ -606,6 +707,7 @@ BM_GET_BOOT_DESCRIPTION mBmBootDescriptionHandlers[] = {
   BmGetDescriptionFromDiskInfo,
   BmGetNetworkDescription,
   BmGetLoadFileDescription,
+  BmGetNvmeDescription,
   BmGetMiscDescription
 };
 
diff --git 

Re: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Yao, Jiewen
Good. That is better.

From: Wu, Hao A
Sent: Friday, March 10, 2017 3:46 PM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors 
to handle unclosed events

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wu,
> Hao A
> Sent: Friday, March 10, 2017 3:40 PM
> To: Yao, Jiewen; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> destructors to handle unclosed events
>
> > -Original Message-
> > From: Yao, Jiewen
> > Sent: Friday, March 10, 2017 3:23 PM
> > To: Wu, Hao A; edk2-devel@lists.01.org
> > Cc: Wu, Hao A
> > Subject: RE: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> > destructors to handle unclosed events
> >
> > I just find it is unnecessary to put
> > mDxeRuntimeCapsuleLibVirtualAddressChangeEvent and
> > mIsVirtualAddrConverted to DxeCapsuleLib.c.
> > Can we just move them to DxeCapsuleLibRuntime.c?
> >
> > I think this is simple update. No need to send V3.
> > You can do that when check in.
> >
> > Reviewed-by: Jiewen Yao >
>
> Got it, I will move them before checking in the code.

I double checked the code, it seems that 'mEsrtTable' is only used in file
DxeCapsuleRuntime.c as well, I will move it from DxeCapsuleLib.c as well.

>
> Best Regards,
> Hao Wu
>
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Hao
> > > Wu
> > > Sent: Friday, March 10, 2017 3:16 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A >
> > > Subject: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> > destructors
> > > to handle unclosed events
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Hao Wu >
> > > Reviewed-by: Jiewen Yao 
> > > >
> > > ---
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37
> > > +---
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33
> > > ++---
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4
> > > ++-
> > >  4 files changed, 66 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > index 7f500a96eb..a892892ccd 100644
> > > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > @@ -47,9 +47,11 @@
> > >  #include 
> > >  #include 
> > >
> > > -EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
> > > -BOOLEAN   mIsVirtualAddrConverted  = FALSE;
> > > -BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
> > > +EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable
> > > = NULL;
> > > +BOOLEAN   mIsVirtualAddrConverted
> > > = FALSE;
> > > +BOOLEAN   mDxeCapsuleLibEndOfDxe
> > > = FALSE;
> > > +EFI_EVENT mDxeCapsuleLibEndOfDxeEvent
> > > = NULL;
> > > +EFI_EVENT
> > > mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
> > >
> > >  /**
> > >Initialize capsule related variables.
> > > @@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
> > >IN EFI_SYSTEM_TABLE   *SystemTable
> > >)
> > >  {
> > > -  EFI_EVENT EndOfDxeEvent;
> > >EFI_STATUSStatus;
> > >
> > >Status = gBS->CreateEventEx (
> > > @@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
> > >DxeCapsuleLibEndOfDxe,
> > >NULL,
> > >,
> > > -  
> > > +  
> > >);
> > >ASSERT_EFI_ERROR (Status);
> > >
> > > @@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
> > >
> > >return EFI_SUCCESS;
> > >  }
> > > +
> > > +/**
> > > +  The destructor function closes the End of DXE event.
> > > +
> > > +  @param  ImageHandle   The firmware allocated handle for the EFI
> image.
> > > +  @param  SystemTable   A pointer to the EFI System Table.
> > > +
> > > +  @retval EFI_SUCCESS   The destructor completed successfully.
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +DxeCapsuleLibDestructor (
> > > +  IN EFI_HANDLE ImageHandle,
> > > +  IN EFI_SYSTEM_TABLE   *SystemTable
> > > +  )
> > > +{
> > > +  EFI_STATUSStatus;
> > > +
> > > +  //
> > > +  // Close the End of DXE event.
> > > +  //
> > > +  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  return EFI_SUCCESS;
> > > +}
> 

Re: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Wu, Hao A
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wu,
> Hao A
> Sent: Friday, March 10, 2017 3:40 PM
> To: Yao, Jiewen; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> destructors to handle unclosed events
> 
> > -Original Message-
> > From: Yao, Jiewen
> > Sent: Friday, March 10, 2017 3:23 PM
> > To: Wu, Hao A; edk2-devel@lists.01.org
> > Cc: Wu, Hao A
> > Subject: RE: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> > destructors to handle unclosed events
> >
> > I just find it is unnecessary to put
> > mDxeRuntimeCapsuleLibVirtualAddressChangeEvent and
> > mIsVirtualAddrConverted to DxeCapsuleLib.c.
> > Can we just move them to DxeCapsuleLibRuntime.c?
> >
> > I think this is simple update. No need to send V3.
> > You can do that when check in.
> >
> > Reviewed-by: Jiewen Yao 
> 
> Got it, I will move them before checking in the code.

I double checked the code, it seems that 'mEsrtTable' is only used in file
DxeCapsuleRuntime.c as well, I will move it from DxeCapsuleLib.c as well.

> 
> Best Regards,
> Hao Wu
> 
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Hao
> > > Wu
> > > Sent: Friday, March 10, 2017 3:16 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A 
> > > Subject: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> > destructors
> > > to handle unclosed events
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Hao Wu 
> > > Reviewed-by: Jiewen Yao 
> > > ---
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37
> > > +---
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33
> > > ++---
> > >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4
> > > ++-
> > >  4 files changed, 66 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > index 7f500a96eb..a892892ccd 100644
> > > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > > @@ -47,9 +47,11 @@
> > >  #include 
> > >  #include 
> > >
> > > -EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
> > > -BOOLEAN   mIsVirtualAddrConverted  = FALSE;
> > > -BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
> > > +EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable
> > > = NULL;
> > > +BOOLEAN   mIsVirtualAddrConverted
> > > = FALSE;
> > > +BOOLEAN   mDxeCapsuleLibEndOfDxe
> > > = FALSE;
> > > +EFI_EVENT mDxeCapsuleLibEndOfDxeEvent
> > > = NULL;
> > > +EFI_EVENT
> > > mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
> > >
> > >  /**
> > >Initialize capsule related variables.
> > > @@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
> > >IN EFI_SYSTEM_TABLE   *SystemTable
> > >)
> > >  {
> > > -  EFI_EVENT EndOfDxeEvent;
> > >EFI_STATUSStatus;
> > >
> > >Status = gBS->CreateEventEx (
> > > @@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
> > >DxeCapsuleLibEndOfDxe,
> > >NULL,
> > >,
> > > -  
> > > +  
> > >);
> > >ASSERT_EFI_ERROR (Status);
> > >
> > > @@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
> > >
> > >return EFI_SUCCESS;
> > >  }
> > > +
> > > +/**
> > > +  The destructor function closes the End of DXE event.
> > > +
> > > +  @param  ImageHandle   The firmware allocated handle for the EFI
> image.
> > > +  @param  SystemTable   A pointer to the EFI System Table.
> > > +
> > > +  @retval EFI_SUCCESS   The destructor completed successfully.
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +DxeCapsuleLibDestructor (
> > > +  IN EFI_HANDLE ImageHandle,
> > > +  IN EFI_SYSTEM_TABLE   *SystemTable
> > > +  )
> > > +{
> > > +  EFI_STATUSStatus;
> > > +
> > > +  //
> > > +  // Close the End of DXE event.
> > > +  //
> > > +  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
> > > +  ASSERT_EFI_ERROR (Status);
> > > +
> > > +  return EFI_SUCCESS;
> > > +}
> > > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > > index 5e437dc418..a6cf54cb6b 100644
> > > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > > @@ -3,7 +3,7 @@
> > >  #
> > >  #  Capsule library instance for DXE_DRIVER module types.
> > >  #
> > > -#  Copyright (c) 2016, Intel 

Re: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Wu, Hao A
> -Original Message-
> From: Yao, Jiewen
> Sent: Friday, March 10, 2017 3:23 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: RE: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> destructors to handle unclosed events
> 
> I just find it is unnecessary to put
> mDxeRuntimeCapsuleLibVirtualAddressChangeEvent and
> mIsVirtualAddrConverted to DxeCapsuleLib.c.
> Can we just move them to DxeCapsuleLibRuntime.c?
> 
> I think this is simple update. No need to send V3.
> You can do that when check in.
> 
> Reviewed-by: Jiewen Yao 

Got it, I will move them before checking in the code.

Best Regards,
Hao Wu

> 
> Thank you
> Yao Jiewen
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao
> > Wu
> > Sent: Friday, March 10, 2017 3:16 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A 
> > Subject: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib
> destructors
> > to handle unclosed events
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hao Wu 
> > Reviewed-by: Jiewen Yao 
> > ---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37
> > +---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33
> > ++---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4
> > ++-
> >  4 files changed, 66 insertions(+), 11 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > index 7f500a96eb..a892892ccd 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > @@ -47,9 +47,11 @@
> >  #include 
> >  #include 
> >
> > -EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
> > -BOOLEAN   mIsVirtualAddrConverted  = FALSE;
> > -BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
> > +EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable
> > = NULL;
> > +BOOLEAN   mIsVirtualAddrConverted
> > = FALSE;
> > +BOOLEAN   mDxeCapsuleLibEndOfDxe
> > = FALSE;
> > +EFI_EVENT mDxeCapsuleLibEndOfDxeEvent
> > = NULL;
> > +EFI_EVENT
> > mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
> >
> >  /**
> >Initialize capsule related variables.
> > @@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
> >IN EFI_SYSTEM_TABLE   *SystemTable
> >)
> >  {
> > -  EFI_EVENT EndOfDxeEvent;
> >EFI_STATUSStatus;
> >
> >Status = gBS->CreateEventEx (
> > @@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
> >DxeCapsuleLibEndOfDxe,
> >NULL,
> >,
> > -  
> > +  
> >);
> >ASSERT_EFI_ERROR (Status);
> >
> > @@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
> >
> >return EFI_SUCCESS;
> >  }
> > +
> > +/**
> > +  The destructor function closes the End of DXE event.
> > +
> > +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> > +  @param  SystemTable   A pointer to the EFI System Table.
> > +
> > +  @retval EFI_SUCCESS   The destructor completed successfully.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +DxeCapsuleLibDestructor (
> > +  IN EFI_HANDLE ImageHandle,
> > +  IN EFI_SYSTEM_TABLE   *SystemTable
> > +  )
> > +{
> > +  EFI_STATUSStatus;
> > +
> > +  //
> > +  // Close the End of DXE event.
> > +  //
> > +  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
> > +  ASSERT_EFI_ERROR (Status);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > index 5e437dc418..a6cf54cb6b 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> > @@ -3,7 +3,7 @@
> >  #
> >  #  Capsule library instance for DXE_DRIVER module types.
> >  #
> > -#  Copyright (c) 2016, Intel Corporation. All rights reserved.
> > +#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
> >  #  This program and the accompanying materials
> >  #  are licensed and made available under the terms and conditions of the
> BSD
> > License
> >  #  which accompanies this distribution.  The full text of the license may 
> > be
> > found at
> > @@ -23,6 +23,7 @@
> >VERSION_STRING = 1.0
> >LIBRARY_CLASS  = CapsuleLib|DXE_DRIVER
> > UEFI_APPLICATION
> >CONSTRUCTOR= DxeCapsuleLibConstructor
> > +  DESTRUCTOR = DxeCapsuleLibDestructor
> >
> >  #
> >  # The following information is for reference only and not required by the
> 

[edk2] [PATCH 3/3] MdeModulePkg/SmmCore: Add Context in SmiHandlerProfileUnregister.

2017-03-09 Thread Jiewen Yao
The reason is that we observe that a platform may use same Handler
for different context.

In order to support Unregister such handler, we have to input
context information as well.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h |   8 +-
 MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c | 103 
 2 files changed, 89 insertions(+), 22 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h 
b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index 45b9d97..c12805a 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -1139,6 +1139,10 @@ SmiHandlerProfileRegisterHandler (
  For the SmmChildDispatch protocol, the HandlerGuid
  must be the GUID of SmmChildDispatch protocol.
   @param Handler The SMI handler.
+  @param Context The context of the SMI handler.
+ If it is NOT NULL, it will be used to check what is 
registered.
+  @param ContextSize The size of the context in bytes.
+ If Context is NOT NULL, it will be used to check what 
is registered.
 
   @retval EFI_SUCCESS   The original record is removed.
   @retval EFI_NOT_FOUND There is no record for the HandlerGuid and 
handler.
@@ -1148,7 +1152,9 @@ EFIAPI
 SmiHandlerProfileUnregisterHandler (
   IN SMI_HANDLER_PROFILE_PROTOCOL   *This,
   IN EFI_GUID   *HandlerGuid,
-  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler
+  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler,
+  IN VOID   *Context, OPTIONAL
+  IN UINTN  ContextSize OPTIONAL
   );
 
 extern UINTNmFullSmramRangeCount;
diff --git a/MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c 
b/MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c
index f85c0f0..1e36039 100644
--- a/MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c
+++ b/MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c
@@ -1089,6 +1089,40 @@ SmmCoreFindHardwareSmiEntry (
 }
 
 /**
+  Convert EFI_SMM_USB_REGISTER_CONTEXT to 
SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT.
+
+  @param UsbContext   A pointer to EFI_SMM_USB_REGISTER_CONTEXT
+  @param UsbContextSize   The size of EFI_SMM_USB_REGISTER_CONTEXT 
in bytes
+  @param SmiHandlerUsbContextSize The size of 
SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT in bytes
+
+  @return SmiHandlerUsbContext  A pointer to 
SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT
+**/
+SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT *
+ConvertSmiHandlerUsbContext (
+  IN EFI_SMM_USB_REGISTER_CONTEXT   *UsbContext,
+  IN UINTN  UsbContextSize,
+  OUT UINTN *SmiHandlerUsbContextSize
+  )
+{
+  UINTN DevicePathSize;
+  SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT  *SmiHandlerUsbContext;
+
+  ASSERT (UsbContextSize == sizeof(EFI_SMM_USB_REGISTER_CONTEXT));
+
+  DevicePathSize = GetDevicePathSize (UsbContext->Device);
+  SmiHandlerUsbContext = AllocatePool (sizeof 
(SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT) + DevicePathSize);
+  if (SmiHandlerUsbContext == NULL) {
+*SmiHandlerUsbContextSize = 0;
+return NULL;
+  }
+  SmiHandlerUsbContext->Type = UsbContext->Type;
+  SmiHandlerUsbContext->DevicePathSize = (UINT32)DevicePathSize;
+  CopyMem (SmiHandlerUsbContext + 1, UsbContext->Device, DevicePathSize);
+  *SmiHandlerUsbContextSize = sizeof 
(SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT) + DevicePathSize;
+  return SmiHandlerUsbContext;
+}
+
+/**
   This function is called by SmmChildDispatcher module to report
   a new SMI handler is registered, to SmmCore.
 
@@ -1123,6 +1157,11 @@ SmiHandlerProfileRegisterHandler (
   SMI_ENTRY*SmiEntry;
   LIST_ENTRY   *List;
 
+  if (((ContextSize == 0) && (Context != NULL)) ||
+  ((ContextSize != 0) && (Context == NULL))) {
+return EFI_INVALID_PARAMETER;
+  }
+
   SmiHandler = AllocateZeroPool (sizeof (SMI_HANDLER));
   if (SmiHandler == NULL) {
 return EFI_OUT_OF_RESOURCES;
@@ -1131,33 +1170,24 @@ SmiHandlerProfileRegisterHandler (
   SmiHandler->Signature = SMI_HANDLER_SIGNATURE;
   SmiHandler->Handler = Handler;
   SmiHandler->CallerAddr = (UINTN)CallerAddress;
-  if (ContextSize != 0 && Context != NULL) {
+  SmiHandler->Context = Context;
+  SmiHandler->ContextSize = ContextSize;
+
+  if (Context != NULL) {
 if (CompareGuid (HandlerGuid, )) {
-  EFI_SMM_USB_REGISTER_CONTEXT  *UsbContext;
-  UINTN DevicePathSize;
-  SMI_HANDLER_PROFILE_USB_REGISTER_CONTEXT  *SmiHandlerUsbContext;
-
-  ASSERT (ContextSize == sizeof(EFI_SMM_USB_REGISTER_CONTEXT));
-
-  UsbContext = 

[edk2] [PATCH 2/3] MdeModulePkg/SmiHandlerProfile: Add Context support in Unregister

2017-03-09 Thread Jiewen Yao
The reason is that we observe that a platform may use same Handler
for different context.

In order to support Unregister such handler, we have to input
context information as well.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Include/Guid/SmiHandlerProfile.h  | 41 
+++-
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c | 10 
-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/SmiHandlerProfile.h 
b/MdeModulePkg/Include/Guid/SmiHandlerProfile.h
index b81631d..c5d29e8 100644
--- a/MdeModulePkg/Include/Guid/SmiHandlerProfile.h
+++ b/MdeModulePkg/Include/Guid/SmiHandlerProfile.h
@@ -150,6 +150,26 @@ extern EFI_GUID gSmiHandlerProfileGuid;
 
 typedef struct _SMI_HANDLER_PROFILE_PROTOCOL  SMI_HANDLER_PROFILE_PROTOCOL;
 
+/**
+  This function is called by SmmChildDispatcher module to report
+  a new SMI handler is registered, to SmmCore.
+
+  @param ThisThe protocol instance
+  @param HandlerGuid The GUID to identify the type of the handler.
+ For the SmmChildDispatch protocol, the HandlerGuid
+ must be the GUID of SmmChildDispatch protocol.
+  @param Handler The SMI handler.
+  @param CallerAddress   The address of the module who registers the SMI 
handler.
+  @param Context The context of the SMI handler.
+ For the SmmChildDispatch protocol, the Context
+ must match the one defined for SmmChildDispatch 
protocol.
+  @param ContextSize The size of the context in bytes.
+ For the SmmChildDispatch protocol, the Context
+ must match the one defined for SmmChildDispatch 
protocol.
+
+  @retval EFI_SUCCESS   The information is recorded.
+  @retval EFI_OUT_OF_RESOURCES  There is no enough resource to record the 
information.
+**/
 typedef
 EFI_STATUS
 (EFIAPI  *SMI_HANDLER_PROFILE_REGISTER_HANDLER) (
@@ -161,12 +181,31 @@ EFI_STATUS
   IN UINTN  ContextSize OPTIONAL
   );
 
+/**
+  This function is called by SmmChildDispatcher module to report
+  an existing SMI handler is unregistered, to SmmCore.
+
+  @param ThisThe protocol instance
+  @param HandlerGuid The GUID to identify the type of the handler.
+ For the SmmChildDispatch protocol, the HandlerGuid
+ must be the GUID of SmmChildDispatch protocol.
+  @param Handler The SMI handler.
+  @param Context The context of the SMI handler.
+ If it is NOT NULL, it will be used to check what is 
registered.
+  @param ContextSize The size of the context in bytes.
+ If Context is NOT NULL, it will be used to check what 
is registered.
+
+  @retval EFI_SUCCESS   The original record is removed.
+  @retval EFI_NOT_FOUND There is no record for the HandlerGuid and 
handler.
+**/
 typedef
 EFI_STATUS
 (EFIAPI  *SMI_HANDLER_PROFILE_UNREGISTER_HANDLER) (
   IN SMI_HANDLER_PROFILE_PROTOCOL   *This,
   IN EFI_GUID   *HandlerGuid,
-  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler
+  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler,
+  IN VOID   *Context, OPTIONAL
+  IN UINTN  ContextSize OPTIONAL
   );
 
 struct _SMI_HANDLER_PROFILE_PROTOCOL {
diff --git 
a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c 
b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
index 2edc71b..2911619 100644
--- a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
+++ b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
@@ -63,6 +63,10 @@ SmiHandlerProfileRegisterHandler (
  For the SmmChildDispatch protocol, the HandlerGuid
  must be the GUID of SmmChildDispatch protocol.
   @param Handler The SMI handler.
+  @param Context The context of the SMI handler.
+ If it is NOT NULL, it will be used to check what is 
registered.
+  @param ContextSize The size of the context in bytes.
+ If Context is NOT NULL, it will be used to check what 
is registered.
 
   @retval EFI_SUCCESS   The original record is removed.
   @retval EFI_UNSUPPORTED   The feature is unsupported.
@@ -72,11 +76,13 @@ EFI_STATUS
 EFIAPI
 SmiHandlerProfileUnregisterHandler (
   IN EFI_GUID   *HandlerGuid,
-  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler
+  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler,
+  IN VOID   *Context, OPTIONAL
+  IN UINTN 

[edk2] [PATCH 1/3] MdePkg/SmiHandlerProfile: Add Context support in Unregister

2017-03-09 Thread Jiewen Yao
The reason is that we observe that a platform may use same Handler
for different context.

In order to support Unregister such handler, we have to input
context information as well.

Cc: Jeff Fan 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdePkg/Include/Library/SmiHandlerProfileLib.h  | 8 +++-
 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c | 8 +++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/SmiHandlerProfileLib.h 
b/MdePkg/Include/Library/SmiHandlerProfileLib.h
index 10b7323..77d19f9 100644
--- a/MdePkg/Include/Library/SmiHandlerProfileLib.h
+++ b/MdePkg/Include/Library/SmiHandlerProfileLib.h
@@ -66,6 +66,10 @@ SmiHandlerProfileRegisterHandler (
  For the SmmChildDispatch protocol, the HandlerGuid
  must be the GUID of SmmChildDispatch protocol.
   @param Handler The SMI handler.
+  @param Context The context of the SMI handler.
+ If it is NOT NULL, it will be used to check what is 
registered.
+  @param ContextSize The size of the context in bytes.
+ If Context is NOT NULL, it will be used to check what 
is registered.
 
   @retval EFI_SUCCESS   The original record is removed.
   @retval EFI_UNSUPPORTED   The feature is unsupported.
@@ -75,7 +79,9 @@ EFI_STATUS
 EFIAPI
 SmiHandlerProfileUnregisterHandler (
   IN EFI_GUID   *HandlerGuid,
-  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler
+  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler,
+  IN VOID   *Context, OPTIONAL
+  IN UINTN  ContextSize OPTIONAL
   );
 
 #endif
diff --git a/MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c 
b/MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c
index 6ae4718..7f4855b 100644
--- a/MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c
+++ b/MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c
@@ -56,6 +56,10 @@ SmiHandlerProfileRegisterHandler (
  For the SmmChildDispatch protocol, the HandlerGuid
  must be the GUID of SmmChildDispatch protocol.
   @param Handler The SMI handler.
+  @param Context The context of the SMI handler.
+ If it is NOT NULL, it will be used to check what is 
registered.
+  @param ContextSize The size of the context in bytes.
+ If Context is NOT NULL, it will be used to check what 
is registered.
 
   @retval EFI_SUCCESS   The original record is removed.
   @retval EFI_UNSUPPORTED   The feature is unsupported.
@@ -65,7 +69,9 @@ EFI_STATUS
 EFIAPI
 SmiHandlerProfileUnregisterHandler (
   IN EFI_GUID   *HandlerGuid,
-  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler
+  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler,
+  IN VOID   *Context, OPTIONAL
+  IN UINTN  ContextSize OPTIONAL
   )
 {
   return EFI_UNSUPPORTED;
-- 
2.7.4.windows.1

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


[edk2] [PATCH 0/3] Add Context in SmiHandlerProfileUnregister.

2017-03-09 Thread Jiewen Yao
This issue is reported by bret.barke...@microsoft.com.

We observe that a platform may use same Handler
for different context.

In order to support Unregister such handler, we have to input
context information as well.


The patch does not impact any platform with SmiHandlerProfile disabled.

Unit tests below:
1) register same handler with different context, and unregister each.
2) register and unregister UsbContext.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 

Jiewen Yao (3):
  MdePkg/SmiHandlerProfile: Add Context support in Unregister
  MdeModulePkg/SmiHandlerProfile: Add Context support in Unregister
  MdeModulePkg/SmmCore: Add Context in SmiHandlerProfileUnregister.

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   8 +-
 MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c| 103 

 MdeModulePkg/Include/Guid/SmiHandlerProfile.h  |  41 
+++-
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c |  10 +-
 MdePkg/Include/Library/SmiHandlerProfileLib.h  |   8 +-
 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c |   8 +-
 6 files changed, 151 insertions(+), 27 deletions(-)

-- 
2.7.4.windows.1

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


Re: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Yao, Jiewen
I just find it is unnecessary to put 
mDxeRuntimeCapsuleLibVirtualAddressChangeEvent and mIsVirtualAddrConverted to 
DxeCapsuleLib.c.
Can we just move them to DxeCapsuleLibRuntime.c?

I think this is simple update. No need to send V3.
You can do that when check in.

Reviewed-by: Jiewen Yao 

Thank you
Yao Jiewen


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao
> Wu
> Sent: Friday, March 10, 2017 3:16 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A 
> Subject: [edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors
> to handle unclosed events
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> Reviewed-by: Jiewen Yao 
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37
> +---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33
> ++---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4
> ++-
>  4 files changed, 66 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 7f500a96eb..a892892ccd 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -47,9 +47,11 @@
>  #include 
>  #include 
> 
> -EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
> -BOOLEAN   mIsVirtualAddrConverted  = FALSE;
> -BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
> +EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable
> = NULL;
> +BOOLEAN   mIsVirtualAddrConverted
> = FALSE;
> +BOOLEAN   mDxeCapsuleLibEndOfDxe
> = FALSE;
> +EFI_EVENT mDxeCapsuleLibEndOfDxeEvent
> = NULL;
> +EFI_EVENT
> mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
> 
>  /**
>Initialize capsule related variables.
> @@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
>IN EFI_SYSTEM_TABLE   *SystemTable
>)
>  {
> -  EFI_EVENT EndOfDxeEvent;
>EFI_STATUSStatus;
> 
>Status = gBS->CreateEventEx (
> @@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
>DxeCapsuleLibEndOfDxe,
>NULL,
>,
> -  
> +  
>);
>ASSERT_EFI_ERROR (Status);
> 
> @@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
> 
>return EFI_SUCCESS;
>  }
> +
> +/**
> +  The destructor function closes the End of DXE event.
> +
> +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS   The destructor completed successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeCapsuleLibDestructor (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUSStatus;
> +
> +  //
> +  // Close the End of DXE event.
> +  //
> +  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> index 5e437dc418..a6cf54cb6b 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  Capsule library instance for DXE_DRIVER module types.
>  #
> -#  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD
> License
>  #  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -23,6 +23,7 @@
>VERSION_STRING = 1.0
>LIBRARY_CLASS  = CapsuleLib|DXE_DRIVER
> UEFI_APPLICATION
>CONSTRUCTOR= DxeCapsuleLibConstructor
> +  DESTRUCTOR = DxeCapsuleLibDestructor
> 
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> index 880143905a..513aa533e1 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> @@ -1,7 +1,7 @@
>  /** @file
>Capsule library runtime support.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and 

Re: [edk2] [PATCH v2 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

2017-03-09 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Wu, Hao A
> Sent: Friday, March 10, 2017 3:16 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Yao, Jiewen 
> Subject: [PATCH v2 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> GetVariable2 API
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index fc0f8698a9..3fed8e06e4 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -342,6 +342,10 @@ InitCapsuleLastVariable (
>  0,
>  NULL
>  );
> +  } else {
> +if (CapsuleResult != NULL) {
> +  FreePool (CapsuleResult);
> +}
>}
>  }
> 
> --
> 2.12.0.windows.1

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


[edk2] [PATCH v2 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

2017-03-09 Thread Hao Wu
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
index fc0f8698a9..3fed8e06e4 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
@@ -342,6 +342,10 @@ InitCapsuleLastVariable (
 0,
 NULL
 );
+  } else {
+if (CapsuleResult != NULL) {
+  FreePool (CapsuleResult);
+}
   }
 }
 
-- 
2.12.0.windows.1

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


[edk2] [PATCH v2 0/2] MdeModulePkg/CapsuleLib: Code refinements

2017-03-09 Thread Hao Wu
V2 changes:
1) Check the return status of the GetVariable2 API before freeing the
returned buffer from this function.

V1 history:
The series makes the following refinements in DxeCapsuleLibFmp:
1) Add library destructors to close the events created in their relative
constructors.

2) Free the buffer returned by the GetVariable2 API.

Cc: Jiewen Yao 

Hao Wu (2):
  MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events
  MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37 
+---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c|  4 +++
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33 
++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4 ++-
 5 files changed, 70 insertions(+), 11 deletions(-)

-- 
2.12.0.windows.1

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


[edk2] [PATCH v2 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Hao Wu
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
Reviewed-by: Jiewen Yao 
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37 
+---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33 
++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4 ++-
 4 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 7f500a96eb..a892892ccd 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -47,9 +47,11 @@
 #include 
 #include 
 
-EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
-BOOLEAN   mIsVirtualAddrConverted  = FALSE;
-BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
+EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable = 
NULL;
+BOOLEAN   mIsVirtualAddrConverted = 
FALSE;
+BOOLEAN   mDxeCapsuleLibEndOfDxe  = 
FALSE;
+EFI_EVENT mDxeCapsuleLibEndOfDxeEvent = 
NULL;
+EFI_EVENT mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = 
NULL;
 
 /**
   Initialize capsule related variables.
@@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
-  EFI_EVENT EndOfDxeEvent;
   EFI_STATUSStatus;
 
   Status = gBS->CreateEventEx (
@@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
   DxeCapsuleLibEndOfDxe,
   NULL,
   ,
-  
+  
   );
   ASSERT_EFI_ERROR (Status);
 
@@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
 
   return EFI_SUCCESS;
 }
+
+/**
+  The destructor function closes the End of DXE event.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The destructor completed successfully.
+**/
+EFI_STATUS
+EFIAPI
+DxeCapsuleLibDestructor (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUSStatus;
+
+  //
+  // Close the End of DXE event.
+  //
+  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
index 5e437dc418..a6cf54cb6b 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
@@ -3,7 +3,7 @@
 #
 #  Capsule library instance for DXE_DRIVER module types.
 #
-#  Copyright (c) 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -23,6 +23,7 @@
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = CapsuleLib|DXE_DRIVER UEFI_APPLICATION
   CONSTRUCTOR= DxeCapsuleLibConstructor
+  DESTRUCTOR = DxeCapsuleLibDestructor
 
 #
 # The following information is for reference only and not required by the 
build tools.
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
index 880143905a..513aa533e1 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
@@ -1,7 +1,7 @@
 /** @file
   Capsule library runtime support.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -28,6 +28,7 @@
 
 extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
 extern BOOLEAN   mIsVirtualAddrConverted;
+extern EFI_EVENT 
mDxeRuntimeCapsuleLibVirtualAddressChangeEvent;
 
 /**
   Convert EsrtTable physical address to virtual address.
@@ -92,21 +93,45 @@ DxeRuntimeCapsuleLibConstructor (
   )
 {
   EFI_STATUS Status;
-  EFI_EVENT  Event;
 
   //
   // Make sure we can handle virtual address changes.
   //
-  Event = NULL;
   Status = gBS->CreateEventEx (
   EVT_NOTIFY_SIGNAL,
   TPL_NOTIFY,
   

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3] Change BIOS version

2017-03-09 Thread Guo, Mang
Change BIOS version to 0.62

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Guo Mang 
---
 Platform/BroxtonPlatformPkg/BiosId.env | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/BroxtonPlatformPkg/BiosId.env 
b/Platform/BroxtonPlatformPkg/BiosId.env
index bed8730..1e1992a 100644
--- a/Platform/BroxtonPlatformPkg/BiosId.env
+++ b/Platform/BroxtonPlatformPkg/BiosId.env
@@ -31,5 +31,5 @@ BOARD_ID  = APLKRVP
 BOARD_REV = 3
 OEM_ID = X64
 BUILD_TYPE= D
-VERSION_MAJOR = 0061
+VERSION_MAJOR = 0062
 VERSION_MINOR = 01
-- 
2.10.1.windows.1

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


Re: [edk2] [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

2017-03-09 Thread Wu, Hao A
> -Original Message-
> From: Yao, Jiewen
> Sent: Friday, March 10, 2017 2:17 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: RE: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned
> by GetVariable2 API
> 
> According to GetVariable2() API, there is no guarantee that Data will be NULL,
> if error is returned.
> 
> I suggest we use status check instead of data pointer check.
> 

Yes, you are right. I will send out a V2 version of the series.

Best Regards,
Hao Wu

> 
> /**
>   Returns the status whether get the variable success. The function retrieves
>   variable  through the UEFI Runtime Service GetVariable().  The
>   returned buffer is allocated using AllocatePool().  The caller is 
> responsible
>   for freeing this buffer with FreePool().
> 
>   If Name  is NULL, then ASSERT().
>   If Guid  is NULL, then ASSERT().
>   If Value is NULL, then ASSERT().
> 
>   @param[in]  Name  The pointer to a Null-terminated Unicode string.
>   @param[in]  Guid  The pointer to an EFI_GUID structure
>   @param[out] Value The buffer point saved the variable info.
>   @param[out] Size  The buffer size of the variable.
> 
>   @return EFI_OUT_OF_RESOURCES  Allocate buffer failed.
>   @return EFI_SUCCESS   Find the specified variable.
>   @return Others Errors Return errors from call to 
> gRT->GetVariable.
> 
> **/
> 
> 
> 
> > -Original Message-
> > From: Wu, Hao A
> > Sent: Friday, March 10, 2017 2:07 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A ; Yao, Jiewen 
> > Subject: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> > GetVariable2 API
> >
> > Cc: Jiewen Yao 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hao Wu 
> > ---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git
> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > index fc0f8698a9..191a432369 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > @@ -343,6 +343,9 @@ InitCapsuleLastVariable (
> >  NULL
> >  );
> >}
> > +  if (CapsuleResult != NULL) {
> > +FreePool (CapsuleResult);
> > +  }
> >  }
> >
> >  // Lock it in normal boot path per UEFI spec.
> > --
> > 2.12.0.windows.1

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


[edk2] [Patch] MdePkg/UefiDevicePathLib: Fix the wrong MAC address length

2017-03-09 Thread Jiaxin Wu
Network interface type should be checked before the conversion between
text device path node and MAC device path. Otherwise, the MAC text string
can't be converted to the representation of a device node, which leads to
the series failure of network HII configuration(e.g. IP, VLAN, HTTP Boot
configuration in Network Device List).

Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c 
b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
index ae38859..a52cbef 100644
--- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
@@ -1798,10 +1798,14 @@ DevPathFromTextMAC (
   );
 
   MACDevPath->IfType   = (UINT8) Strtoi (IfTypeStr);
 
   Length = sizeof (EFI_MAC_ADDRESS);
+  if (MACDevPath->IfType == 0x01 || MACDevPath->IfType == 0x00) {
+Length = 6;
+  }
+
   StrHexToBytes (AddressStr, Length * 2, MACDevPath->MacAddress.Addr, Length);
 
   return (EFI_DEVICE_PATH_PROTOCOL *) MACDevPath;
 }
 
-- 
1.9.5.msysgit.1

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


Re: [edk2] [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

2017-03-09 Thread Yao, Jiewen
According to GetVariable2() API, there is no guarantee that Data will be NULL, 
if error is returned.

I suggest we use status check instead of data pointer check.


/**
  Returns the status whether get the variable success. The function retrieves 
  variable  through the UEFI Runtime Service GetVariable().  The 
  returned buffer is allocated using AllocatePool().  The caller is responsible
  for freeing this buffer with FreePool().

  If Name  is NULL, then ASSERT().
  If Guid  is NULL, then ASSERT().
  If Value is NULL, then ASSERT().

  @param[in]  Name  The pointer to a Null-terminated Unicode string.
  @param[in]  Guid  The pointer to an EFI_GUID structure
  @param[out] Value The buffer point saved the variable info.
  @param[out] Size  The buffer size of the variable.

  @return EFI_OUT_OF_RESOURCES  Allocate buffer failed.
  @return EFI_SUCCESS   Find the specified variable.
  @return Others Errors Return errors from call to gRT->GetVariable.

**/



> -Original Message-
> From: Wu, Hao A
> Sent: Friday, March 10, 2017 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Yao, Jiewen 
> Subject: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> GetVariable2 API
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index fc0f8698a9..191a432369 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -343,6 +343,9 @@ InitCapsuleLastVariable (
>  NULL
>  );
>}
> +  if (CapsuleResult != NULL) {
> +FreePool (CapsuleResult);
> +  }
>  }
> 
>  // Lock it in normal boot path per UEFI spec.
> --
> 2.12.0.windows.1

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


[edk2] [PATCH] MdeModulePkg/PlatVarCleanupLib: Add lib destructor for unclosed event

2017-03-09 Thread Hao Wu
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c   | 33 
++--
 MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf |  3 +-
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c 
b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
index b96f8ddd83..c5fd30e219 100644
--- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
+++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
@@ -1,7 +1,7 @@
 /** @file
   Sample platform variable cleanup library implementation.
 
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -22,6 +22,8 @@ EDKII_VAR_CHECK_PROTOCOL*mVarCheck = NULL;
 ///
 BOOLEAN mEndOfDxe = FALSE;
 
+EFI_EVENT   mPlatVarCleanupLibEndOfDxeEvent = NULL;
+
 LIST_ENTRY  mUserVariableList = INITIALIZE_LIST_HEAD_VARIABLE 
(mUserVariableList);
 UINT16  mUserVariableCount = 0;
 UINT16  mMarkedUserVariableCount = 0;
@@ -1229,7 +1231,6 @@ PlatformVarCleanupLibConstructor (
   )
 {
   EFI_STATUSStatus;
-  EFI_EVENT Event;
 
   mLastVarErrorFlag = InternalGetVarErrorFlag ();
   DEBUG ((EFI_D_INFO, "mLastVarErrorFlag - 0x%02x\n", mLastVarErrorFlag));
@@ -1243,10 +1244,36 @@ PlatformVarCleanupLibConstructor (
   PlatformVarCleanupEndOfDxeEvent,
   NULL,
   ,
-  
+  
   );
   ASSERT_EFI_ERROR (Status);
 
   return EFI_SUCCESS;
 }
 
+/**
+  The destructor function closes the End of DXE event.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The destructor completed successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+PlatformVarCleanupLibDestructor (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUSStatus;
+
+  //
+  // Close the End of DXE event.
+  //
+  Status = gBS->CloseEvent (mPlatVarCleanupLibEndOfDxeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git 
a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf 
b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
index a3498cca50..6e7fcb6a5c 100644
--- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
+++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Sample platform variable cleanup library instance.
 #
-#  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions
@@ -23,6 +23,7 @@
   VERSION_STRING= 1.0
   LIBRARY_CLASS = PlatformVarCleanupLib|DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
   CONSTRUCTOR   = PlatformVarCleanupLibConstructor
+  DESTRUCTOR= PlatformVarCleanupLibDestructor
 
 #
 # The following information is for reference only and not required by the 
build tools.
-- 
2.12.0.windows.1

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


Re: [edk2] [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Wu, Hao A
> Sent: Friday, March 10, 2017 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Yao, Jiewen 
> Subject: [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle
> unclosed events
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37
> +---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33
> ++---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4
> ++-
>  4 files changed, 66 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index 7f500a96eb..a892892ccd 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -47,9 +47,11 @@
>  #include 
>  #include 
> 
> -EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
> -BOOLEAN   mIsVirtualAddrConverted  = FALSE;
> -BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
> +EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable
> = NULL;
> +BOOLEAN   mIsVirtualAddrConverted
> = FALSE;
> +BOOLEAN   mDxeCapsuleLibEndOfDxe
> = FALSE;
> +EFI_EVENT mDxeCapsuleLibEndOfDxeEvent
> = NULL;
> +EFI_EVENT
> mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
> 
>  /**
>Initialize capsule related variables.
> @@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
>IN EFI_SYSTEM_TABLE   *SystemTable
>)
>  {
> -  EFI_EVENT EndOfDxeEvent;
>EFI_STATUSStatus;
> 
>Status = gBS->CreateEventEx (
> @@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
>DxeCapsuleLibEndOfDxe,
>NULL,
>,
> -  
> +  
>);
>ASSERT_EFI_ERROR (Status);
> 
> @@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
> 
>return EFI_SUCCESS;
>  }
> +
> +/**
> +  The destructor function closes the End of DXE event.
> +
> +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS   The destructor completed successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeCapsuleLibDestructor (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUSStatus;
> +
> +  //
> +  // Close the End of DXE event.
> +  //
> +  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> index 5e437dc418..a6cf54cb6b 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  Capsule library instance for DXE_DRIVER module types.
>  #
> -#  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD
> License
>  #  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -23,6 +23,7 @@
>VERSION_STRING = 1.0
>LIBRARY_CLASS  = CapsuleLib|DXE_DRIVER
> UEFI_APPLICATION
>CONSTRUCTOR= DxeCapsuleLibConstructor
> +  DESTRUCTOR = DxeCapsuleLibDestructor
> 
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> index 880143905a..513aa533e1 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> @@ -1,7 +1,7 @@
>  /** @file
>Capsule library runtime support.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -28,6 +28,7 @@
> 
>  extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
>  extern BOOLEAN   mIsVirtualAddrConverted;
> +extern EFI_EVENT
> 

[edk2] [PATCH 0/2] MdeModulePkg/CapsuleLib: Code refinements

2017-03-09 Thread Hao Wu
The series makes the following refinements in DxeCapsuleLibFmp:
1) Add library destructors to close the events created in their relative
constructors.

2) Free the buffer returned by the GetVariable2 API.

Cc: Jiewen Yao 

Hao Wu (2):
  MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events
  MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37 
+---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c|  3 ++
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33 
++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4 ++-
 5 files changed, 69 insertions(+), 11 deletions(-)

-- 
2.12.0.windows.1

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


[edk2] [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

2017-03-09 Thread Hao Wu
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
index fc0f8698a9..191a432369 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
@@ -343,6 +343,9 @@ InitCapsuleLastVariable (
 NULL
 );
   }
+  if (CapsuleResult != NULL) {
+FreePool (CapsuleResult);
+  }
 }
 
 // Lock it in normal boot path per UEFI spec.
-- 
2.12.0.windows.1

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


[edk2] [PATCH 1/2] MdeModulePkg/CapsuleLib: Add lib destructors to handle unclosed events

2017-03-09 Thread Hao Wu
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c  | 37 
+---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf|  3 +-
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c  | 33 
++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  4 ++-
 4 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 7f500a96eb..a892892ccd 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -47,9 +47,11 @@
 #include 
 #include 
 
-EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable  = NULL;
-BOOLEAN   mIsVirtualAddrConverted  = FALSE;
-BOOLEAN   mDxeCapsuleLibEndOfDxe   = FALSE;
+EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable = 
NULL;
+BOOLEAN   mIsVirtualAddrConverted = 
FALSE;
+BOOLEAN   mDxeCapsuleLibEndOfDxe  = 
FALSE;
+EFI_EVENT mDxeCapsuleLibEndOfDxeEvent = 
NULL;
+EFI_EVENT mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = 
NULL;
 
 /**
   Initialize capsule related variables.
@@ -1654,7 +1656,6 @@ DxeCapsuleLibConstructor (
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
-  EFI_EVENT EndOfDxeEvent;
   EFI_STATUSStatus;
 
   Status = gBS->CreateEventEx (
@@ -1663,7 +1664,7 @@ DxeCapsuleLibConstructor (
   DxeCapsuleLibEndOfDxe,
   NULL,
   ,
-  
+  
   );
   ASSERT_EFI_ERROR (Status);
 
@@ -1671,3 +1672,29 @@ DxeCapsuleLibConstructor (
 
   return EFI_SUCCESS;
 }
+
+/**
+  The destructor function closes the End of DXE event.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The destructor completed successfully.
+**/
+EFI_STATUS
+EFIAPI
+DxeCapsuleLibDestructor (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUSStatus;
+
+  //
+  // Close the End of DXE event.
+  //
+  Status = gBS->CloseEvent (mDxeCapsuleLibEndOfDxeEvent);
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
index 5e437dc418..a6cf54cb6b 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
@@ -3,7 +3,7 @@
 #
 #  Capsule library instance for DXE_DRIVER module types.
 #
-#  Copyright (c) 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -23,6 +23,7 @@
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = CapsuleLib|DXE_DRIVER UEFI_APPLICATION
   CONSTRUCTOR= DxeCapsuleLibConstructor
+  DESTRUCTOR = DxeCapsuleLibDestructor
 
 #
 # The following information is for reference only and not required by the 
build tools.
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c 
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
index 880143905a..513aa533e1 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
@@ -1,7 +1,7 @@
 /** @file
   Capsule library runtime support.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -28,6 +28,7 @@
 
 extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
 extern BOOLEAN   mIsVirtualAddrConverted;
+extern EFI_EVENT 
mDxeRuntimeCapsuleLibVirtualAddressChangeEvent;
 
 /**
   Convert EsrtTable physical address to virtual address.
@@ -92,21 +93,45 @@ DxeRuntimeCapsuleLibConstructor (
   )
 {
   EFI_STATUS Status;
-  EFI_EVENT  Event;
 
   //
   // Make sure we can handle virtual address changes.
   //
-  Event = NULL;
   Status = gBS->CreateEventEx (
   EVT_NOTIFY_SIGNAL,
   TPL_NOTIFY,
   

[edk2] [Patch][edk2-platforms] Vlv2TbltDevicePkg: Add BGRT table.

2017-03-09 Thread lushifex
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: lushifex 
---
 Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c |  2 +-
 Vlv2TbltDevicePkg/PlatformPkg.fdf  | 12 +++-
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf   | 12 +++-
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|  3 ++-
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |  3 ++-
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |  3 ++-
 6 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c 
b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
index 7f91777..73ad0bd 100644
--- a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -1821,7 +1821,7 @@ PlatformBdsPolicyBehavior (
 // console directly.
 //
 BdsLibConnectAllDefaultConsoles ();
-PlatformBdsDiagnostics (IGNORE, TRUE, BaseMemoryTest);
+PlatformBdsDiagnostics (IGNORE, FALSE, BaseMemoryTest);
 
 //
 // Perform some platform specific connect sequence
diff --git a/Vlv2TbltDevicePkg/PlatformPkg.fdf 
b/Vlv2TbltDevicePkg/PlatformPkg.fdf
index cb5b6b7..e5c6525 100644
--- a/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -1,7 +1,7 @@
 #/** @file
 # FDF file of Platform.
 #
-# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License that accompanies this 
distribution.
@@ -447,6 +447,10 @@ APRIORI DXE {
 FILE FREEFORM = C3E36D09-8294-4b97-A857-D5288FE33E28 {
 SECTION RAW = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(DXE_ARCHITECTURE)/BiosId.bin
   }
+  
+FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
+SECTION RAW = MdeModulePkg/Logo/Logo.bmp
+  }
 
   #
   # EDK II Related Platform codes
@@ -461,6 +465,7 @@ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 !if $(ACPI50_ENABLE) == TRUE
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
+INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 !endif
 
 
@@ -1097,6 +1102,11 @@ INF  
SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf
 RAW ASL   Optional|.aml
   }
 
+[Rule.Common.USER_DEFINED.LOGO]
+  FILE FREEFORM = $(NAMED_GUID) {
+RAW BIN   |.bmp
+  }
+
 [Rule.Common.ACPITABLE]
   FILE FREEFORM = $(NAMED_GUID) {
 RAW ACPI  Optional|.acpi
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index 4243b07..84e416e 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -1,7 +1,7 @@
 #/** @file
 # FDF file of Platform.
 #
-# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License that accompanies this 
distribution.
@@ -404,6 +404,10 @@ APRIORI DXE {
 FILE FREEFORM = C3E36D09-8294-4b97-A857-D5288FE33E28 {
 SECTION RAW = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(DXE_ARCHITECTURE)/BiosId.bin
   }
+  
+FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
+SECTION RAW = MdeModulePkg/Logo/Logo.bmp
+  }
 
   #
   # EDK II Related Platform codes
@@ -418,6 +422,7 @@ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 !if $(ACPI50_ENABLE) == TRUE
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
+INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 !endif
 
 
@@ -1058,6 +1063,11 @@ INF  
SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf
 RAW ASL   Optional|.aml
   }
 
+[Rule.Common.USER_DEFINED.LOGO]
+  FILE FREEFORM = $(NAMED_GUID) {
+RAW BIN   |.bmp
+  }
+
 [Rule.Common.ACPITABLE]
   FILE FREEFORM = $(NAMED_GUID) {
 RAW ACPI  Optional|.acpi
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index cdde337..f27f294 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -1,7 +1,7 @@
 #/** @file
 # Platform description.
 #
-# Copyright (c) 2012  - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2012  - 2017, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying 

Re: [edk2] [Patch][edk2-platforms] Vlv2TbltDevicePkg: Add BGRT table.

2017-03-09 Thread Wei, David
Reviewed-by: zwei4  


Thanks,
David  Wei 

-Original Message-
From: Lu, ShifeiX A 
Sent: Thursday, March 09, 2017 5:28 PM
To: edk2-devel@lists.01.org
Cc: Wei, David 
Subject: [Patch][edk2-platforms] Vlv2TbltDevicePkg: Add BGRT table.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: lushifex 
---
 Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c |  4 ++--
 Vlv2TbltDevicePkg/PlatformPkg.fdf  | 12 +++-
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf   | 12 +++-
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|  3 ++-
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |  3 ++-
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |  3 ++-
 6 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c 
b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
index 6e8364b..38eaeb2 100644
--- a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2004  - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2004  - 2017, Intel Corporation. All rights reserved.



   This program and the accompanying materials are licensed and made available 
under

   the terms and conditions of the BSD License that accompanies this 
distribution.  

@@ -1891,7 +1891,7 @@ PlatformBdsPolicyBehavior (
 // console directly.
 //
 BdsLibConnectAllDefaultConsoles ();
-PlatformBdsDiagnostics (IGNORE, TRUE, BaseMemoryTest);
+PlatformBdsDiagnostics (IGNORE, FALSE, BaseMemoryTest);
 
 //
 // Perform some platform specific connect sequence
diff --git a/Vlv2TbltDevicePkg/PlatformPkg.fdf 
b/Vlv2TbltDevicePkg/PlatformPkg.fdf
index ae4ee2d..b795d60 100644
--- a/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -1,7 +1,7 @@
 #/** @file
 # FDF file of Platform.
 #
-# Copyright (c) 2008  - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2008  - 2017, Intel Corporation. All rights reserved.
 #  

 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License that accompanies this 
distribution.  
@@ -404,6 +404,10 @@ APRIORI DXE {
 FILE FREEFORM = C3E36D09-8294-4b97-A857-D5288FE33E28 {
 SECTION RAW = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(DXE_ARCHITECTURE)/BiosId.bin
   }
+  
+FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
+SECTION RAW = MdeModulePkg/Logo/Logo.bmp
+  }
 
   #
   # EDK II Related Platform codes
@@ -418,6 +422,7 @@ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 !if $(ACPI50_ENABLE) == TRUE
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
+INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 !endif
 
 
@@ -1128,6 +1133,11 @@ FV = BiosUpdate
 RAW ASL   Optional|.aml
   }
 
+[Rule.Common.USER_DEFINED.LOGO]
+  FILE FREEFORM = $(NAMED_GUID) {
+RAW BIN   |.bmp
+  }
+
 [Rule.Common.ACPITABLE]
   FILE FREEFORM = $(NAMED_GUID) {
 RAW ACPI  Optional|.acpi
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index 43d20ee..334a5fc 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -1,7 +1,7 @@
 #/** @file
 # FDF file of Platform.
 #
-# Copyright (c) 2008  - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2008  - 2017, Intel Corporation. All rights reserved.
 #  

 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License that accompanies this 
distribution.  
@@ -357,6 +357,10 @@ APRIORI DXE {
 FILE FREEFORM = C3E36D09-8294-4b97-A857-D5288FE33E28 {
 SECTION RAW = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(DXE_ARCHITECTURE)/BiosId.bin
   }
+  
+FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
+SECTION RAW = MdeModulePkg/Logo/Logo.bmp
+  }
 
   #
   # EDK II Related Platform codes
@@ -371,6 +375,7 @@ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 !if $(ACPI50_ENABLE) == TRUE
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
+INF  

Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.

2017-03-09 Thread Fan, Jeff
Anthony,

MSR 0x1A0 is architectural MSR defined in IA32 SDM.

Have you tried AMD real platform?

Thanks!
Jeff

-Original Message-
From: Anthony PERARD [mailto:anthony.per...@citrix.com] 
Sent: Thursday, March 09, 2017 7:53 PM
To: Yao, Jiewen
Cc: edk2-devel@lists.01.org; Kinney, Michael D; Fan, Jeff
Subject: Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute 
setting.

On Tue, Feb 21, 2017 at 02:57:07PM +0800, Jiewen Yao wrote:
> Add memory attribute setting in CpuArch protocol.
> Previous SetMemoryAttributes() API only supports cache attribute setting.
> 
> This patch updated SetMemoryAttributes() API to support memory 
> attribute setting by updating CPU page table.
> 
> Cc: Jeff Fan 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao 

Hi,

This patch doesn't work on AMD (when running OVMF on Xen). With an AMD cpu, 
reading MSR 0x1a0 cause a General Protection Fault.

> +  AsmCpuid (0x8000, , NULL, NULL, NULL);  if (RegEax > 
> + 0x8000) {
> +AsmCpuid (0x8001, NULL, NULL, NULL, );
> +if ((RegEdx & BIT20) != 0) {
> +  // XD supported

This next read is where the fault is taken.

> +  if ((AsmReadMsr64 (0x01A0) & BIT34) == 0) {
> +// XD enabled
> +if ((AsmReadMsr64 (0xC080) & BIT11) != 0) {
> +  // XD activated
> +  PagingContext->ContextData.Ia32.Attributes |= 
> PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED;
> +}
> +  }
> +}

>From OVMF output:

 X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID -  

RIP  - 2F323ABE, CS  - 0018, RFLAGS - 00010002 
ExceptionData -  RAX  - 8001, RCX - 
01A0, RDX - 2FD3FBFF RBX  - 2F422CB8, RSP - 
2F422C68, RBP - 2EE0AD98 RSI  - 2F3AF018, RDI - 
2F3229E4
R8   - , R9  - , R10 - 
R11  - 2F422BAD, R12 - , R13 - 
R14  - , R15 - 2F44A980
DS   - 0008, ES  - 0008, FS  - 0008
GS   - 0008, SS  - 0008
CR0  - C0010033, CR2 - , CR3 - 2F3C1000
CR4  - 0668, CR8 - 
DR0  - , DR1 - , DR2 - 
DR3  - , DR6 - 0FF0, DR7 - 0400 GDTR - 
FF80 001F, LDTR - 
IDTR - 2B68DD90 021F,   TR - 
FXSAVE_STATE - 2F4228C0
 Find PE image 
/home/osstest/build.106538.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe/DEBUG/CpuDxe.dll
 (ImageBase=2F321000, EntryPoint=2F321271) 


Thanks,

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


Re: [edk2] [PATCH] MdePkg/Pci22.h: Remove deprecated macros

2017-03-09 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu 
> Ni
> Sent: Monday, March 6, 2017 2:40 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [PATCH] MdePkg/Pci22.h: Remove deprecated macros
> 
> The following deprecated macros are removed.The removal
> doesn't cause any build failure to existing packages.
> 
> #define DEVICE_ID_NOCARE0x
> #define PCI_BAR_OLD_ALIGN   0xULL
> #define PCI_BAR_EVEN_ALIGN  0xFFFEULL
> #define PCI_BAR_SQUAD_ALIGN 0xFFFDULL
> #define PCI_BAR_DQUAD_ALIGN 0xFFFCULL
> #define PCI_BAR_ALL 0xFF
> #define PCI_ACPI_UNUSED 0
> #define PCI_BAR_NOCHANGE0
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Liming Gao 
> ---
>  MdePkg/Include/IndustryStandard/Pci22.h | 19 ---
>  1 file changed, 19 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h 
> b/MdePkg/Include/IndustryStandard/Pci22.h
> index 07718dc..96a7680 100644
> --- a/MdePkg/Include/IndustryStandard/Pci22.h
> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
> @@ -780,25 +780,6 @@ typedef struct {
>///
>  } EFI_PCI_CAPABILITY_HOTPLUG;
> 
> -///
> -/// Below macros (till PCI_BAR_NOCHANGE) were used by 
> EfiIncompatiblePciDeviceSupport Protocol.
> -///
> -#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
> -
> -///
> -/// [ATTENTION] These macros are deprecated because they don't match Spec or 
> not defined in Spec.
> -///
> -#define DEVICE_ID_NOCARE0x ///< Deprecated. Value 
> doesn't match Spec.
> -#define PCI_BAR_OLD_ALIGN   0xULL  ///< Deprecated. Value 
> isn't defined in Spec.
> -#define PCI_BAR_EVEN_ALIGN  0xFFFEULL  ///< Deprecated. Value 
> isn't defined in Spec.
> -#define PCI_BAR_SQUAD_ALIGN 0xFFFDULL  ///< Deprecated. Value 
> isn't defined in Spec.
> -#define PCI_BAR_DQUAD_ALIGN 0xFFFCULL  ///< Deprecated. Value 
> isn't defined in Spec.
> -#define PCI_BAR_ALL 0xFF   ///< Deprecated. Value 
> doesn't match Spec.
> -#define PCI_ACPI_UNUSED 0  ///< Deprecated. Macro 
> name is too general.
> -#define PCI_BAR_NOCHANGE0  ///< Deprecated. Macro 
> name is too general.
> -
> -#endif
> -
>  #define PCI_BAR_IDX00x00
>  #define PCI_BAR_IDX10x01
>  #define PCI_BAR_IDX20x02
> --
> 2.9.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [patch] BaseTools/UPT: Fix an issue in subst command

2017-03-09 Thread hesschen
UPT used to use "B:" as the temp directory which may cause conflict,
now it will choose a valid volume. Also UPT now accepts empty sections.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: hesschen 
---
 .../UPT/Parser/InfGuidPpiProtocolSectionParser.py|  7 ---
 BaseTools/Source/Python/UPT/UPT.py   | 20 +++-
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git 
a/BaseTools/Source/Python/UPT/Parser/InfGuidPpiProtocolSectionParser.py 
b/BaseTools/Source/Python/UPT/Parser/InfGuidPpiProtocolSectionParser.py
index 10a82cb..12ffeda 100644
--- a/BaseTools/Source/Python/UPT/Parser/InfGuidPpiProtocolSectionParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/InfGuidPpiProtocolSectionParser.py
@@ -1,7 +1,7 @@
 ## @file
 # This file contained the parser for [Guids], [Ppis], [Protocols] sections in 
INF file 
 #
-# Copyright (c) 2011, Intel Corporation. All rights reserved.
+# Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
 # under the terms and conditions of the BSD License which accompanies this 
@@ -217,8 +217,9 @@ class InfGuidPpiProtocolSectionParser(InfParserSectionRoot):
 for Line in SectionString:
 LineContent = Line[0]
 
-if LineContent.strip() == '':
-continue
+# Comment the code to support user extension without any statement just the 
section header in []
+# if LineContent.strip() == '':
+# continue
 
 UserExtensionContent += LineContent + DT.END_OF_LINE
 continue
diff --git a/BaseTools/Source/Python/UPT/UPT.py 
b/BaseTools/Source/Python/UPT/UPT.py
index 8dd949a..873492d 100644
--- a/BaseTools/Source/Python/UPT/UPT.py
+++ b/BaseTools/Source/Python/UPT/UPT.py
@@ -2,7 +2,7 @@
 #
 # This file is the main entry for UPT 
 #
-# Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
 # under the terms and conditions of the BSD License which accompanies this 
@@ -179,15 +179,16 @@ def Main():
 Logger.Quiet(ST.MSG_PYTHON_ON % (python_version(), platform) + 
format_exc())
 return XExcept.args[0]
 
-# Start *
 # Support WORKSPACE is a long path
-# Only work well on windows
-# Linux Solution TBD
+# Only works for windows system
 if pf.system() == 'Windows':
-os.system('@echo off\nsubst b: /D')
-os.system('subst b: "%s"' % GlobalData.gWORKSPACE)
-GlobalData.gWORKSPACE = 'B:\\'
-# End ***
+Vol = 'B:'
+for Index in range(90, 65, -1):
+Vol = chr(Index) + ':'
+if not os.path.isdir(Vol):
+os.system('subst %s "%s"' % (Vol, GlobalData.gWORKSPACE))
+break
+GlobalData.gWORKSPACE = '%s\\' % Vol
 
 WorkspaceDir = GlobalData.gWORKSPACE
 
@@ -304,8 +305,9 @@ def Main():
 except StandardError:
 Logger.Quiet(ST.MSG_RECOVER_FAIL)
 GlobalData.gDB.CloseDb()
+
 if pf.system() == 'Windows':
-os.system('subst b: /D')
+os.system('subst %s /D' % GlobalData.gWORKSPACE.replace('\\',''))
 
 return ReturnCode
 
-- 
2.7.2.windows.1

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


[edk2] [edk2-staging][PATCH 1/2] ShellPkg: acpiview branch on edk2-staging

2017-03-09 Thread evan . lloyd
From: Evan Lloyd 

Add a Readme file to initiate a branch for acpiview on edk2-staging.
The branch is intended to allow collaborative updating of acpiview for
platforms beyond the initial ARM implementation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Evan Lloyd 
---
 Readme.md | 53 
 1 file changed, 53 insertions(+)

diff --git a/Readme.md b/Readme.md
new file mode 100755
index 
..7a64b941cee1aed7cd51b5db06843535a359c057
--- /dev/null
+++ b/Readme.md
@@ -0,0 +1,53 @@
+
+# ReadMe.MD for acpiview staging branch..
+#  Contents:
+#1. Introduction
+#2. Branch Owner
+#3. Feature Summary
+#4. Timeline
+#5. Related Modules
+#6. Related Links
+#7. Misc
+#
+###
+
+1. Introduction
+   This acpiview staging branch is requested by Evan Lloyd 
+   following suggestions from Jiewen Yao  in response
+   to a patch submitted to the edk2-devel mailing list.
+   The aim is to allow collaborative development of acpiview for all platforms.
+
+2. Branch Owners
+   Evan Lloyd 
+   Jiewen Yao 
+
+3. Feature Summary
+   Acpiview allows examination of ACPI table contents from the UEFI Shell.
+   This can help with investigations, especially at that stage where the tables
+   are not enabling an OS to boot.  The program is not exhaustive, and only
+   encapsulates detailed knowledge of a limited number of table types.
+   There is no immediate intent to provide disassembly of AML tables.
+
+   Default behaviour is to display the content of all tables installed.
+   'Known' table types will be parsed and displayed with descriptions and
+   field values.  Where appropriate a degree of consistency checking is
+   done and errors may be reported in the output.
+   Other table types will be displayed as an array of Hexadecimal bytes.
+
+   To facilitate debugging, the -t and -b options can be used to generate a
+   binary file image of a table that can be copied elsewhere for
+   investigation using tools such as those provided by acpica.org.  This is
+   especially relevant for AML type tables like DSDT and SSDT.
+
+4. Timeline
+   N/A
+
+5. Related Modules
+   ShellPkg
+
+6. Related Links
+   N/A
+
+7. Misc
+   N/A
+
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

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


[edk2] [edk2-staging][PATCH 0/2] Create new acpiview branch

2017-03-09 Thread evan . lloyd
From: Evan Lloyd 

   I am requesting this acpiview staging branch  following suggestions from
   Jiewen Yao  in response to a patch submitted 
previously.
   The aim is to allow collaborative development of acpiview for all platforms.

Evan Lloyd (1):
  ShellPkg: acpiview branch on edk2-staging

Sami Mujawar (1):
  ShellPkg: Add acpiview tool to dump ACPI tables

 ShellPkg/ShellPkg.dec| 
  2 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf | 
 73 +++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h| 
418 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h   | 
347 +++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h  | 
 84 +++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.h   | 
 34 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c| 
628 +++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c   | 
173 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c  | 
595 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/BgrtParser.c| 
 61 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Dbg2Parser.c| 
244 
 ShellPkg/Library/UefiShellAcpiViewCommandLib/DsdtParser.c| 
 45 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/FadtParser.c| 
151 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/GtdtParser.c| 
295 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/IortParser.c| 
643 
 ShellPkg/Library/UefiShellAcpiViewCommandLib/MadtParser.c| 
280 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/McfgParser.c| 
 88 +++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/RsdpParser.c| 
183 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/SlitParser.c| 
142 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/SpcrParser.c| 
148 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/SratParser.c| 
307 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/SsdtParser.c| 
 43 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c   | 
106 
 ShellPkg/Library/UefiShellAcpiViewCommandLib/XsdtParser.c| 
145 +
 Readme.md| 
 53 ++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni | 
125 
 26 files changed, 5413 insertions(+)
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.h
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.h
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/BgrtParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/Dbg2Parser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/DsdtParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/FadtParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/GtdtParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/IortParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/MadtParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/McfgParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/RsdpParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/SlitParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/SpcrParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/SratParser.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/SsdtParser.c
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/XsdtParser.c
 create mode 100755 Readme.md
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

___
edk2-devel mailing list
edk2-devel@lists.01.org

Re: [edk2] [PATCH v2 0/4] ArmVirtPkg: make DT vs ACPI support mutually exclusive

2017-03-09 Thread Ard Biesheuvel
On 9 March 2017 at 18:21, Ard Biesheuvel  wrote:
> Instead of supplying both ACPI and DT hw descriptions, and allow the latter
> to be inihibited by setting a compile time define, make DT table installation
> dependent on the absence of a ACPI 2.0 table when the ReadyToBoot even fires.
>
> Changes since v1:
> - add missing includes
> - cosmetic coding style fixes
> - reorder event registration with protocol installation (#2)
> - add Laszlo's patch to add missing EFIAPI specifiers
>
> As Laszlo has pointed out, this affects the Xen port as well as the QEMU/KVM
> one, which I consider to be an advantage. And of course, I am happy to keep
> both halves if it turns out I ended up breaking it :-)
>

Thanks guys, all pushed now
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v2 1/4] ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv specifiers

2017-03-09 Thread Laszlo Ersek
On 03/09/17 18:21, Ard Biesheuvel wrote:
> From: Laszlo Ersek 
> 
> Add missing EFIAPI calling convention specifiers to STATIC function
> that are exposed via the FdtClientProtocol interface.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 7cc0c44ca12a..547a29fce62c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -29,6 +29,7 @@ STATIC VOID  *mDeviceTreeBase;
>  
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  GetNodeProperty (
>IN  FDT_CLIENT_PROTOCOL *This,
>IN  INT32   Node,
> @@ -55,6 +56,7 @@ GetNodeProperty (
>  
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  SetNodeProperty (
>IN  FDT_CLIENT_PROTOCOL *This,
>IN  INT32   Node,
> @@ -267,6 +269,7 @@ FindMemoryNodeReg (
>  
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  GetOrInsertChosenNode (
>IN  FDT_CLIENT_PROTOCOL *This,
>OUT INT32   *Node
> 

Well, I can't exactly "review" or "ACK" my own patch (or patches mostly
derived from my patches). Leif, can you pls R-b this?

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


Re: [edk2] [PATCH v2 4/4] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

2017-03-09 Thread Laszlo Ersek
On 03/09/17 18:21, Ard Biesheuvel wrote:
> Instead of having a build time switch to prevent the FDT configuration
> table from being installed, make this behavior dependent on whether we
> are passing ACPI tables to the OS. This is done by looking for the
> ACPI 2.0 configuration table, and only installing the FDT one if the
> ACPI one cannot be found.

I would have preferred if, rather than updating just the blurb, you had
mentioned Xen by name in this commit message. But, I don't want to
obsess about it.

Reviewed-by: Laszlo Ersek 

Thanks,
Laszlo

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 10 --
>  ArmVirtPkg/ArmVirtQemu.dsc   |  5 -
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 16 +++-
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 ++---
>  4 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index a5ec42166445..efe83a383d55 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
># EFI_VT_100_GUID.
>#
>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
> 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
> 0x4D}|VOID*|0x0007
> -
> -[PcdsFeatureFlag]
> -  #
> -  # Pure ACPI boot
> -  #
> -  # Inhibit installation of the FDT as a configuration table if this feature
> -  # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
> -  # description of the platform, and it is up to the OS to choose.
> -  #
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x000a
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 477dfdcfc764..7b266b98b949 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -34,7 +34,6 @@ [Defines]
># -D FLAG=VALUE
>#
>DEFINE SECURE_BOOT_ENABLE  = FALSE
> -  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
>  
>  !include ArmVirtPkg/ArmVirt.dsc.inc
>  
> @@ -94,10 +93,6 @@ [PcdsFeatureFlag.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>  
> -!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
> -!endif
> -
>  [PcdsFixedAtBuild.common]
>gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>  !if $(ARCH) == AARCH64
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 4cf79f70cb2a..21c1074e331c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -17,9 +17,11 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -316,12 +318,16 @@ OnReadyToBoot (
>)
>  {
>EFI_STATUS  Status;
> +  VOID*Table;
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// Only install the FDT as a configuration table if we want to leave it 
> up
> -// to the OS to decide whether it prefers ACPI over DT.
> -//
> +  //
> +  // Only install the FDT as a configuration table if we are not exposing
> +  // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has
> +  // no meaning on ARM since we need at least ACPI 5.0 support, and the
> +  // 64-bit ACPI 2.0 table GUID is mandatory in that case.
> +  //
> +  Status = EfiGetSystemConfigurationTable (, );
> +  if (EFI_ERROR (Status) || Table == NULL) {
>  Status = gBS->InstallConfigurationTable (, 
> mDeviceTreeBase);
>  ASSERT_EFI_ERROR (Status);
>}
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 00017727c32c..9861f41e968b 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -37,17 +37,16 @@ [LibraryClasses]
>HobLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
> +  UefiLib
>  
>  [Protocols]
>gFdtClientProtocolGuid  ## PRODUCES
>  
>  [Guids]
> +  gEfiAcpi20TableGuid
>gEfiEventReadyToBootGuid
>gFdtHobGuid
>gFdtTableGuid
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>TRUE
> 

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


Re: [edk2] [PATCH v2 3/4] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

2017-03-09 Thread Laszlo Ersek
On 03/09/17 18:21, Ard Biesheuvel wrote:
> Defer FDT configuration table installation until ReadyToBoot is signaled.
> This allows any driver to make modifications in the mean time, and will
> also allow us to defer the decision of whether to install it in the first
> place to later on in the boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Leif Lindholm 
> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 49 
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
>  2 files changed, 41 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 547a29fce62c..4cf79f70cb2a 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -306,6 +307,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
>GetOrInsertChosenNode,
>  };
>  
> +STATIC
> +VOID
> +EFIAPI
> +OnReadyToBoot (
> +  EFI_EVENT   Event,
> +  VOID*Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> +//
> +// Only install the FDT as a configuration table if we want to leave it 
> up
> +// to the OS to decide whether it prefers ACPI over DT.
> +//
> +Status = gBS->InstallConfigurationTable (, 
> mDeviceTreeBase);
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  gBS->CloseEvent (Event);
> +}
> +
> +STATIC EFI_EVENT mReadyToBootEvent;
> +
>  EFI_STATUS
>  EFIAPI
>  InitializeFdtClientDxe (
> @@ -333,15 +358,21 @@ InitializeFdtClientDxe (
>  
>DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// Only install the FDT as a configuration table if we want to leave it 
> up
> -// to the OS to decide whether it prefers ACPI over DT.
> -//
> -Status = gBS->InstallConfigurationTable (, DeviceTreeBase);
> -ASSERT_EFI_ERROR (Status);
> +  Status = gBS->InstallProtocolInterface (, 
> ,
> +  EFI_NATIVE_INTERFACE, );
> +  if (EFI_ERROR (Status)) {
> +return Status;
>}
>  
> -  return gBS->InstallProtocolInterface (, 
> ,
> -EFI_NATIVE_INTERFACE, );
> +  Status = gBS->CreateEventEx (
> +  EVT_NOTIFY_SIGNAL,
> +  TPL_CALLBACK,
> +  OnReadyToBoot,
> +  NULL,
> +  ,
> +  
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
>  }
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 3a0cd37040eb..00017727c32c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -42,6 +42,7 @@ [Protocols]
>gFdtClientProtocolGuid  ## PRODUCES
>  
>  [Guids]
> +  gEfiEventReadyToBootGuid
>gFdtHobGuid
>gFdtTableGuid
>  
> 

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


Re: [edk2] [PATCH v2 1/4] ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv specifiers

2017-03-09 Thread Leif Lindholm
Reviewed-by: Leif Lindholm 

On 9 Mar 2017 6:29 pm, "Laszlo Ersek"  wrote:

> On 03/09/17 18:21, Ard Biesheuvel wrote:
> > From: Laszlo Ersek 
> >
> > Add missing EFIAPI calling convention specifiers to STATIC function
> > that are exposed via the FdtClientProtocol interface.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Laszlo Ersek 
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> > index 7cc0c44ca12a..547a29fce62c 100644
> > --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> > +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> > @@ -29,6 +29,7 @@ STATIC VOID  *mDeviceTreeBase;
> >
> >  STATIC
> >  EFI_STATUS
> > +EFIAPI
> >  GetNodeProperty (
> >IN  FDT_CLIENT_PROTOCOL *This,
> >IN  INT32   Node,
> > @@ -55,6 +56,7 @@ GetNodeProperty (
> >
> >  STATIC
> >  EFI_STATUS
> > +EFIAPI
> >  SetNodeProperty (
> >IN  FDT_CLIENT_PROTOCOL *This,
> >IN  INT32   Node,
> > @@ -267,6 +269,7 @@ FindMemoryNodeReg (
> >
> >  STATIC
> >  EFI_STATUS
> > +EFIAPI
> >  GetOrInsertChosenNode (
> >IN  FDT_CLIENT_PROTOCOL *This,
> >OUT INT32   *Node
> >
>
> Well, I can't exactly "review" or "ACK" my own patch (or patches mostly
> derived from my patches). Leif, can you pls R-b this?
>
> Thanks
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v2 3/4] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

2017-03-09 Thread Ard Biesheuvel
Defer FDT configuration table installation until ReadyToBoot is signaled.
This allows any driver to make modifications in the mean time, and will
also allow us to defer the decision of whether to install it in the first
place to later on in the boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 49 
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 547a29fce62c..4cf79f70cb2a 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -306,6 +307,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
   GetOrInsertChosenNode,
 };
 
+STATIC
+VOID
+EFIAPI
+OnReadyToBoot (
+  EFI_EVENT   Event,
+  VOID*Context
+  )
+{
+  EFI_STATUS  Status;
+
+  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
+//
+// Only install the FDT as a configuration table if we want to leave it up
+// to the OS to decide whether it prefers ACPI over DT.
+//
+Status = gBS->InstallConfigurationTable (, mDeviceTreeBase);
+ASSERT_EFI_ERROR (Status);
+  }
+
+  gBS->CloseEvent (Event);
+}
+
+STATIC EFI_EVENT mReadyToBootEvent;
+
 EFI_STATUS
 EFIAPI
 InitializeFdtClientDxe (
@@ -333,15 +358,21 @@ InitializeFdtClientDxe (
 
   DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// Only install the FDT as a configuration table if we want to leave it up
-// to the OS to decide whether it prefers ACPI over DT.
-//
-Status = gBS->InstallConfigurationTable (, DeviceTreeBase);
-ASSERT_EFI_ERROR (Status);
+  Status = gBS->InstallProtocolInterface (, 
,
+  EFI_NATIVE_INTERFACE, );
+  if (EFI_ERROR (Status)) {
+return Status;
   }
 
-  return gBS->InstallProtocolInterface (, ,
-EFI_NATIVE_INTERFACE, );
+  Status = gBS->CreateEventEx (
+  EVT_NOTIFY_SIGNAL,
+  TPL_CALLBACK,
+  OnReadyToBoot,
+  NULL,
+  ,
+  
+  );
+  ASSERT_EFI_ERROR (Status);
+
+  return EFI_SUCCESS;
 }
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
index 3a0cd37040eb..00017727c32c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
@@ -42,6 +42,7 @@ [Protocols]
   gFdtClientProtocolGuid  ## PRODUCES
 
 [Guids]
+  gEfiEventReadyToBootGuid
   gFdtHobGuid
   gFdtTableGuid
 
-- 
2.7.4

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


[edk2] [PATCH v2 2/4] ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node

2017-03-09 Thread Ard Biesheuvel
Disable the PL031 RTC DT node unconditionally rather than only when
the DT will be exposed to the OS. This allows us to defer the decision
whether to expose it to the OS to a later time without creating an
additional dependency on the FDT client code by the RTC driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Reviewed-by: Laszlo Ersek 
---
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 
+---
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 
---
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git 
a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c 
b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
index 82de7a51b32e..d168424a52f5 100644
--- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
@@ -66,18 +66,16 @@ ArmVirtPL031FdtClientLibConstructor (
 
   DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// UEFI takes ownership of the RTC hardware, and exposes its functionality
-// through the UEFI Runtime Services GetTime, SetTime, etc. This means we
-// need to disable it in the device tree to prevent the OS from attaching
-// its device driver as well.
-//
-Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
-  "disabled", sizeof ("disabled"));
-if (EFI_ERROR (Status)) {
-DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
-}
+  //
+  // UEFI takes ownership of the RTC hardware, and exposes its functionality
+  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
+  // need to disable it in the device tree to prevent the OS from attaching
+  // its device driver as well.
+  //
+  Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
+"disabled", sizeof ("disabled"));
+  if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
   }
 
   return EFI_SUCCESS;
diff --git 
a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf 
b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
index 32dbff6f0852..342193651a86 100644
--- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
@@ -42,8 +42,5 @@ [Protocols]
 [Pcd]
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
 
-[FeaturePcd]
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
-
 [Depex]
   gFdtClientProtocolGuid
-- 
2.7.4

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


[edk2] [PATCH v2 4/4] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

2017-03-09 Thread Ard Biesheuvel
Instead of having a build time switch to prevent the FDT configuration
table from being installed, make this behavior dependent on whether we
are passing ACPI tables to the OS. This is done by looking for the
ACPI 2.0 configuration table, and only installing the FDT one if the
ACPI one cannot be found.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtPkg.dec| 10 --
 ArmVirtPkg/ArmVirtQemu.dsc   |  5 -
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 16 +++-
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 ++---
 4 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index a5ec42166445..efe83a383d55 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # EFI_VT_100_GUID.
   #
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
0x4D}|VOID*|0x0007
-
-[PcdsFeatureFlag]
-  #
-  # Pure ACPI boot
-  #
-  # Inhibit installation of the FDT as a configuration table if this feature
-  # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
-  # description of the platform, and it is up to the OS to choose.
-  #
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x000a
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 477dfdcfc764..7b266b98b949 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -34,7 +34,6 @@ [Defines]
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE  = FALSE
-  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
 
 !include ArmVirtPkg/ArmVirt.dsc.inc
 
@@ -94,10 +93,6 @@ [PcdsFeatureFlag.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
 
-!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
-!endif
-
 [PcdsFixedAtBuild.common]
   gArmPlatformTokenSpaceGuid.PcdCoreCount|1
 !if $(ARCH) == AARCH64
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 4cf79f70cb2a..21c1074e331c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -17,9 +17,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -316,12 +318,16 @@ OnReadyToBoot (
   )
 {
   EFI_STATUS  Status;
+  VOID*Table;
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// Only install the FDT as a configuration table if we want to leave it up
-// to the OS to decide whether it prefers ACPI over DT.
-//
+  //
+  // Only install the FDT as a configuration table if we are not exposing
+  // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has
+  // no meaning on ARM since we need at least ACPI 5.0 support, and the
+  // 64-bit ACPI 2.0 table GUID is mandatory in that case.
+  //
+  Status = EfiGetSystemConfigurationTable (, );
+  if (EFI_ERROR (Status) || Table == NULL) {
 Status = gBS->InstallConfigurationTable (, mDeviceTreeBase);
 ASSERT_EFI_ERROR (Status);
   }
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
index 00017727c32c..9861f41e968b 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
@@ -37,17 +37,16 @@ [LibraryClasses]
   HobLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
+  UefiLib
 
 [Protocols]
   gFdtClientProtocolGuid  ## PRODUCES
 
 [Guids]
+  gEfiAcpi20TableGuid
   gEfiEventReadyToBootGuid
   gFdtHobGuid
   gFdtTableGuid
 
-[FeaturePcd]
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
-
 [Depex]
   TRUE
-- 
2.7.4

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


[edk2] [PATCH v2 1/4] ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv specifiers

2017-03-09 Thread Ard Biesheuvel
From: Laszlo Ersek 

Add missing EFIAPI calling convention specifiers to STATIC function
that are exposed via the FdtClientProtocol interface.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 

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

diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 7cc0c44ca12a..547a29fce62c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -29,6 +29,7 @@ STATIC VOID  *mDeviceTreeBase;
 
 STATIC
 EFI_STATUS
+EFIAPI
 GetNodeProperty (
   IN  FDT_CLIENT_PROTOCOL *This,
   IN  INT32   Node,
@@ -55,6 +56,7 @@ GetNodeProperty (
 
 STATIC
 EFI_STATUS
+EFIAPI
 SetNodeProperty (
   IN  FDT_CLIENT_PROTOCOL *This,
   IN  INT32   Node,
@@ -267,6 +269,7 @@ FindMemoryNodeReg (
 
 STATIC
 EFI_STATUS
+EFIAPI
 GetOrInsertChosenNode (
   IN  FDT_CLIENT_PROTOCOL *This,
   OUT INT32   *Node
-- 
2.7.4

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


[edk2] [PATCH v2 0/4] ArmVirtPkg: make DT vs ACPI support mutually exclusive

2017-03-09 Thread Ard Biesheuvel
Instead of supplying both ACPI and DT hw descriptions, and allow the latter
to be inihibited by setting a compile time define, make DT table installation
dependent on the absence of a ACPI 2.0 table when the ReadyToBoot even fires.

Changes since v1:
- add missing includes
- cosmetic coding style fixes
- reorder event registration with protocol installation (#2)
- add Laszlo's patch to add missing EFIAPI specifiers

As Laszlo has pointed out, this affects the Xen port as well as the QEMU/KVM
one, which I consider to be an advantage. And of course, I am happy to keep
both halves if it turns out I ended up breaking it :-)

Ard Biesheuvel (3):
  ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node
  ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot
  ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

Laszlo Ersek (1):
  ArmVirtPkg/FdtClientDxe: supplement missing EFIAPI calling conv
specifiers

 ArmVirtPkg/ArmVirtPkg.dec| 10 

 ArmVirtPkg/ArmVirtQemu.dsc   |  5 
--
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 58 
+---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  6 
+-
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 

 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 -
 6 files changed, 62 insertions(+), 42 deletions(-)

-- 
2.7.4

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


Re: [edk2] [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

2017-03-09 Thread Laszlo Ersek
On 03/09/17 18:00, Leif Lindholm wrote:
> Hi Laszlo,
> 
> Apologies, I didn't ignore this set, I just missed it

No wonder you missed it, I didn't CC you on it :) I wondered if I
should. In general I don't want to increase other people's email load,
and you aren't (yet?) listed as an ArmVirtPkg co-maintainer, so I didn't
CC you in the end.

> (and felt Ard's
> set was a clean solution to this behaviour change).

It is a simple solution, yes; I think its simplicity is deceptive
though, to an extent. To me, ReadyToBoot callbacks are a last resort.

If you grep the UEFI spec for EFI_EVENT_GROUP_READY_TO_BOOT, among other
locations, you find

  34.1.1 User Identify

  [...] When the UEFI Boot Manager signals the
  EFI_EVENT_GROUP_READY_TO_BOOT event group, the User Identity Manager
  publishes the current user profile information in the EFI System
  Configuration Table. [...]

The rest of the language is irrelevant here; my point is that there is
"prior art" for installing sysconfig tables at Ready To Boot. That's
entirely fine. What worries me is that the dependency we evaluate in the
callback is *generally* something that could be satisfied by *another*
Ready To Boot callback, and the ordering between those is unspecified.

Given the current platform, this is not a real issue in practice (we
don't install any of our ACPI tables at Ready To Boot), which is why I'm
open to Ard's solution. I just want us to be aware of this risk.

> 
> A few comments below.
> 
> On Thu, Mar 09, 2017 at 04:30:19PM +0100, Laszlo Ersek wrote:
>> On 03/09/17 13:26, Ard Biesheuvel wrote:
> Hi Laszlo,
>
> This looks complicated to me. Given that it is arguably a policy to
> only expose on h/w description or the other, couldn't we simply remove
> the FDT config table in BDS if an ACPI/ACPI2.0 config table is
> present?

 Technically we could do that, but I dislike it for two reasons:

 - BDS is often the first victim found when looking for a driver to add
 new code to that doesn't seem to fit very well elsewhere. That doesn't
 make BDS any better a recipient, however. "For lack of a better driver"
 is not a strong enough argument to dump code into BDS. If there's really
 no better "topical" driver, then the code usually goes to PlatformDxe.

 - Installing a sysconfig table (or any other system-wide resource) in
 one driver, then undoing it in another driver, should be avoided as much
 as possible, because it leads to non-trivial lifecycles and boggles our
 minds over the longer term. If we can come to a decision that the table
 shouldn't be installed in the first place, we should pursue that.

 Another approach we could look into is: move the installation of the
 sysconfig table from FdtClientDxe to AcpiPlatformDxe. Look for the ACPI
 payload first, and fall back to installing DT (from within
 AcpiPlatformDxe). However, DT should be installed even in builds (like
 ARM32) that don't contain AcpiPlatformDxe at all.

>>>
>>> Or we could hook to the ReadyToBoot event in FdtClientDxe, and install
>>> the DT config table if there is no ACPI/ACPI2.0 table registered.
>>
>> Yes, that's doable in our case, because we control the full platform.
>>
>> Installing tables (any kinds of tables) in ReadyToBoot and similar event
>> handlers is generally a bad idea, because everyone thinks, "okay I'll
>> wait until the rest of the system is done setting up, and I'll just add
>> my stuff afterwards". Obviously, this results in much of the logic being
>> simply moved to such event callbacks, and the invocation order of
>> callbacks remains unspecified.
>>
>> In more precise terms, if the ACPI tables too were installed in a
>> ReadyToBoot callback, your suggestion above would not work. And our ACPI
>> tables are not installed in a ReadyToBoot callback partly because I
>> ultimately introduced a separate event group for "PCI bridges have been
>> connected", and we signal it now explicitly from BDS (device connections
>> are BDS jurisdiction, and QEMU's ACPI generation depends on PCI state).
> 
> All you say above is clearly correct.
> But I am still not clear on why this is a problem.
> This is a _very_ specific case, that applies only to virtual machines
> (which we are in complete control of).

Yes, the "complete control of the platform" argument is not lost on me.

> 
> For hardware platforms wanting the ability to switch between different
> hardware description types, as we discussed, we need a configuration
> setting based on a dynamic Pcd or environment variable - so they won't
> need to wait until the end.
> 
>> I just want to point out that we have a kind of "capital" here. By
>> carefully coding stuff we build capital, and by hooking stuff into
>> ReadyToBoot callbacks we spend (hopefully not "squander") that capital.
> 
> I fully agree with this as a strong default position. But I am
> suggesting that in this case, the callback sort order 

Re: [edk2] [PATCH 3/3] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

2017-03-09 Thread Laszlo Ersek
On 03/09/17 17:04, Ard Biesheuvel wrote:
> Instead of having a build time switch to prevent the FDT configuration
> table from being installed, make this behavior dependent on whether we
> are passing ACPI tables to the OS. This is done by looking for the
> ACPI 2.0 configuration table, and only installing the FDT one if the
> ACPI one cannot be found.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 10 --
>  ArmVirtPkg/ArmVirtQemu.dsc   |  5 -
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 15 ++-
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 ++---
>  4 files changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index a5ec42166445..efe83a383d55 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
># EFI_VT_100_GUID.
>#
>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
> 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
> 0x4D}|VOID*|0x0007
> -
> -[PcdsFeatureFlag]
> -  #
> -  # Pure ACPI boot
> -  #
> -  # Inhibit installation of the FDT as a configuration table if this feature
> -  # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
> -  # description of the platform, and it is up to the OS to choose.
> -  #
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x000a
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 477dfdcfc764..7b266b98b949 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -34,7 +34,6 @@ [Defines]
># -D FLAG=VALUE
>#
>DEFINE SECURE_BOOT_ENABLE  = FALSE
> -  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
>  
>  !include ArmVirtPkg/ArmVirt.dsc.inc
>  
> @@ -94,10 +93,6 @@ [PcdsFeatureFlag.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>  
> -!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
> -!endif
> -
>  [PcdsFixedAtBuild.common]
>gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>  !if $(ARCH) == AARCH64
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 0327af5739f2..2981977f3d20 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 

Can you also #include  for completeness? (For
gEfiAcpi20TableGuid's sake.)

>  
> @@ -312,12 +313,16 @@ OnReadyToBoot (
>)
>  {
>EFI_STATUS  Status;
> +  VOID*Table;
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// Only install the FDT as a configuration table if we want to leave it 
> up
> -// to the OS to decide whether it prefers ACPI over DT.
> -//
> +  //
> +  // Only install the FDT as a configuration table if we are not exposing
> +  // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has
> +  // no meaning on ARM since we need at least ACPI 5.0 support, and the
> +  // 64-bit ACPI 2.0 table GUID is mandatory in that case.
> +  //
> +  Status = EfiGetSystemConfigurationTable (, );
> +  if (EFI_ERROR (Status) || Table == NULL) {
>  Status = gBS->InstallConfigurationTable (, 
> mDeviceTreeBase);
>  ASSERT_EFI_ERROR (Status);
>}

This change will affect Xen too (unlike -D PURE_ACPI_BOOT_ENABLE, which
does not affect Xen -- my patch set also didn't affect Xen, on purpose,
because I have no idea what Xen people want).

Xen installs its ACPI tables in "ArmVirtPkg/XenAcpiPlatformDxe".

I guess it's okay to strive for uniformity here, but the commit message
should mention Xen is included in the change. (The blurb subject is
currently "ArmVirtQemu: make DT vs ACPI support mutually exclusive".)

> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 00017727c32c..9861f41e968b 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -37,17 +37,16 @@ [LibraryClasses]
>HobLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
> +  UefiLib
>  
>  [Protocols]
>gFdtClientProtocolGuid  ## PRODUCES
>  
>  [Guids]
> +  gEfiAcpi20TableGuid
>gEfiEventReadyToBootGuid
>gFdtHobGuid
>gFdtTableGuid
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>TRUE
> 

Looks good to me generally. Two more comments:

- Can you include the first patch from my series, that adds the missing
EFIAPI specifiers to the protocol members in FdtClientDxe? Since we're
already touching the code.

- I agree this is simpler than my approach, but I think it's also less

Re: [edk2] [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

2017-03-09 Thread Leif Lindholm
Hi Laszlo,

Apologies, I didn't ignore this set, I just missed it (and felt Ard's
set was a clean solution to this behaviour change).

A few comments below.

On Thu, Mar 09, 2017 at 04:30:19PM +0100, Laszlo Ersek wrote:
> On 03/09/17 13:26, Ard Biesheuvel wrote:
> >>> Hi Laszlo,
> >>>
> >>> This looks complicated to me. Given that it is arguably a policy to
> >>> only expose on h/w description or the other, couldn't we simply remove
> >>> the FDT config table in BDS if an ACPI/ACPI2.0 config table is
> >>> present?
> >>
> >> Technically we could do that, but I dislike it for two reasons:
> >>
> >> - BDS is often the first victim found when looking for a driver to add
> >> new code to that doesn't seem to fit very well elsewhere. That doesn't
> >> make BDS any better a recipient, however. "For lack of a better driver"
> >> is not a strong enough argument to dump code into BDS. If there's really
> >> no better "topical" driver, then the code usually goes to PlatformDxe.
> >>
> >> - Installing a sysconfig table (or any other system-wide resource) in
> >> one driver, then undoing it in another driver, should be avoided as much
> >> as possible, because it leads to non-trivial lifecycles and boggles our
> >> minds over the longer term. If we can come to a decision that the table
> >> shouldn't be installed in the first place, we should pursue that.
> >>
> >> Another approach we could look into is: move the installation of the
> >> sysconfig table from FdtClientDxe to AcpiPlatformDxe. Look for the ACPI
> >> payload first, and fall back to installing DT (from within
> >> AcpiPlatformDxe). However, DT should be installed even in builds (like
> >> ARM32) that don't contain AcpiPlatformDxe at all.
> >>
> > 
> > Or we could hook to the ReadyToBoot event in FdtClientDxe, and install
> > the DT config table if there is no ACPI/ACPI2.0 table registered.
> 
> Yes, that's doable in our case, because we control the full platform.
> 
> Installing tables (any kinds of tables) in ReadyToBoot and similar event
> handlers is generally a bad idea, because everyone thinks, "okay I'll
> wait until the rest of the system is done setting up, and I'll just add
> my stuff afterwards". Obviously, this results in much of the logic being
> simply moved to such event callbacks, and the invocation order of
> callbacks remains unspecified.
> 
> In more precise terms, if the ACPI tables too were installed in a
> ReadyToBoot callback, your suggestion above would not work. And our ACPI
> tables are not installed in a ReadyToBoot callback partly because I
> ultimately introduced a separate event group for "PCI bridges have been
> connected", and we signal it now explicitly from BDS (device connections
> are BDS jurisdiction, and QEMU's ACPI generation depends on PCI state).

All you say above is clearly correct.
But I am still not clear on why this is a problem.
This is a _very_ specific case, that applies only to virtual machines
(which we are in complete control of).

For hardware platforms wanting the ability to switch between different
hardware description types, as we discussed, we need a configuration
setting based on a dynamic Pcd or environment variable - so they won't
need to wait until the end.

> I just want to point out that we have a kind of "capital" here. By
> carefully coding stuff we build capital, and by hooking stuff into
> ReadyToBoot callbacks we spend (hopefully not "squander") that capital.

I fully agree with this as a strong default position. But I am
suggesting that in this case, the callback sort order genuinely does
not matter for this feature. At which point I prefer the simpler
solution of Ard's set.

That said, there are two maintainers of ArmVirtPkg, and I'm neither of
them :)

> Originally, the APM Mustang firmware (open source, so I can talk about
> it) would first install ACPI tables with constant, platform-tailored
> contents (built from *.asl / *.aslc files), but reusing the stock
> AcpiPlatformDxe C code without customization. Then it would install a
> ReadyToBoot callback which looked up the right DSDT or SSDT, by walking
> the table tree manually, and then it would poke data into the installed
> table (DSDT or SSDT) in-place, using the ACPI SDT protocol.
> 
> Of course it was completely bogus and unreliable, and I changed the
> constant table to contain external references, and I provided those
> external references in a minimal, hand- and runtime- built SSDT, right
> in AcpiPlatformDxe.
> 
> I'm not trying to carefully compose a strawman argument here, just
> presenting why I'm nervous about ReadyToBoot callbacks that try to rely
> on ordering between system-wide resources.
> 
> 
> Also, please don't forget about the other (current) consumer of the
> feature PCD, ArmVirtPL031FdtClientLib, which is plugged into
> RealTimeClockRuntimeDxe. How do you suggest to rewrite the PCD test in
> that driver under the ReadyToBoot callback scenario?
>
> RealTimeClockRuntimeDxe's dispatch order is unspecified relative 

Re: [edk2] [PATCH 2/3] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

2017-03-09 Thread Laszlo Ersek
On 03/09/17 17:04, Ard Biesheuvel wrote:
> Wait for ReadyToBoot to install the FDT configuration table. This allows
> any driver to make modifications in the mean time, and will also allow us
> to defer the decision of whether to install it in the first place to later
> on in the boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 41 
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 7cc0c44ca12a..0327af5739f2 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -303,6 +303,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
>GetOrInsertChosenNode,
>  };
>  
> +STATIC
> +VOID
> +EFIAPI
> +OnReadyToBoot (
> +  EFI_EVENT   Event,
> +  VOID*Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> +//
> +// Only install the FDT as a configuration table if we want to leave it 
> up
> +// to the OS to decide whether it prefers ACPI over DT.
> +//
> +Status = gBS->InstallConfigurationTable (, 
> mDeviceTreeBase);
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  gBS->CloseEvent (Event);

OK, you didn't forget to close the event. :)

> +}
> +
> +STATIC EFI_EVENT ReadyToBootEvent;

This should be called "mReadyToBootEvent".

> +
>  EFI_STATUS
>  EFIAPI
>  InitializeFdtClientDxe (
> @@ -330,14 +354,15 @@ InitializeFdtClientDxe (
>  
>DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// Only install the FDT as a configuration table if we want to leave it 
> up
> -// to the OS to decide whether it prefers ACPI over DT.
> -//
> -Status = gBS->InstallConfigurationTable (, DeviceTreeBase);
> -ASSERT_EFI_ERROR (Status);
> -  }
> +  Status = gBS->CreateEventEx (
> +  EVT_NOTIFY_SIGNAL,
> +  TPL_CALLBACK,
> +  OnReadyToBoot,
> +  NULL,
> +  ,
> +  
> +  );
> +  ASSERT_EFI_ERROR (Status);

I guess it's OK to insist that this succeed.

Also, this is done after the assignment to mDeviceTreeBase, so that's good.

>  
>return gBS->InstallProtocolInterface (, 
> ,
>  EFI_NATIVE_INTERFACE, );

However, if the protocol installation fails for any reason, we exit the
driver with an error code, and the driver will be unloaded. We'll have a
dangling callback pointer

In case of failure, the event should be closed (== CreateEventEx()
should be undone).

> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 3a0cd37040eb..00017727c32c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -42,6 +42,7 @@ [Protocols]
>gFdtClientProtocolGuid  ## PRODUCES
>  
>  [Guids]
> +  gEfiEventReadyToBootGuid
>gFdtHobGuid
>gFdtTableGuid
>  
> 

For completeness, can you please also

#include 

for gEfiEventReadyToBootGuid's sake, in the C file? (It is already
pulled in by something else, but, again, for completenes...)

Looks okay to me otherwise.

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


Re: [edk2] [PATCH 1/3] ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node

2017-03-09 Thread Laszlo Ersek
On 03/09/17 17:03, Ard Biesheuvel wrote:
> Disable the PL031 RTC DT node unconditionally rather than only when
> the DT will be exposed to the OS. This allows us to defer the decision
> whether to expose it to the OS to a later time without creating an
> additional dependency on the FDT client code by the RTC driver.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 
> 22 +---
>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  
> 3 ---
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git 
> a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c 
> b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> index 82de7a51b32e..d168424a52f5 100644
> --- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> @@ -66,18 +66,16 @@ ArmVirtPL031FdtClientLibConstructor (
>  
>DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// UEFI takes ownership of the RTC hardware, and exposes its 
> functionality
> -// through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> -// need to disable it in the device tree to prevent the OS from attaching
> -// its device driver as well.
> -//
> -Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> -  "disabled", sizeof ("disabled"));
> -if (EFI_ERROR (Status)) {
> -DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
> -}
> +  //
> +  // UEFI takes ownership of the RTC hardware, and exposes its functionality
> +  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> +  // need to disable it in the device tree to prevent the OS from attaching
> +  // its device driver as well.
> +  //
> +  Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> +"disabled", sizeof ("disabled"));
> +  if (EFI_ERROR (Status)) {
> +  DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
>}
>  
>return EFI_SUCCESS;
> diff --git 
> a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf 
> b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> index 32dbff6f0852..342193651a86 100644
> --- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> @@ -42,8 +42,5 @@ [Protocols]
>  [Pcd]
>gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>gFdtClientProtocolGuid
> 

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


Re: [edk2] [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import BaseIoLibIntrinsic package

2017-03-09 Thread Laszlo Ersek
On 03/09/17 16:36, Duran, Leo wrote:
> OK, what I'm hearing is:
> - Let's leave the Fifo routines in BaseIoLib (as we currently have)
> - And let's add the SEV checks to BaseIoLib, so that OvmPkg (et al) can 
> consume it as-is
> 
> If so, I could put a patch-set together for that... Please confirm.

Confirmed from my side.

> BTW, I'd try to minimize the need for CPUID (e.g., check it once and cache 
> the result)

In a general purpose BASE library instance, you can't rely on memory
(writeable memory) being available. The library instance could be linked
into SEC or PEI phase modules that (generally speaking) have no
writeable global variables.

It's only an OvmfPkg specialty that PEI has writeable global variables.
(And it comes with a non-intuitive downside: in the SMM-less build of
OVMF, on S3 resume, the PEI global variables are not re-initialized to
the values that the C language would require. This is something we
always have to keep in mind.)

I don't think the CPUID checks will have a huge performance impact,
especially because (IIUC) you only have to customize the Fifo routines.
The cost of the CPUID will be amortized over all the individual IO port
accesses (--> traps) that you will perform individually anyway.

IMO the FeaturePCD check (which will be evaluated at compile time) is a
valid requirement, the CPUID result caching is not -- not until you can
measure the CPUID's impact to be "grave" under "general" workloads. Even
then, the separate DxeSmm instance can be added incrementally (as
suggested by others).

Thanks!
Laszlo

> 
> Leo
> 
>> -Original Message-
>> From: Gao, Liming [mailto:liming@intel.com]
>> Sent: Wednesday, March 08, 2017 7:48 PM
>> To: Justen, Jordan L ; Kinney, Michael D
>> ; edk2-devel@lists.01.org; Brijesh Singh
>> ; Laszlo Ersek 
>> Cc: Lendacky, Thomas ; Duran, Leo
>> ; brijesh.s...@amd.com
>> Subject: RE: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import
>> BaseIoLibIntrinsic package
>>
>>
>>
>>> -Original Message-
>>> From: Justen, Jordan L
>>> Sent: Thursday, March 9, 2017 2:59 AM
>>> To: Gao, Liming ; Kinney, Michael D
>>> ; edk2-devel@lists.01.org; Brijesh Singh
>>> ; Laszlo Ersek 
>>> Cc: thomas.lenda...@amd.com; leo.du...@amd.com;
>> brijesh.s...@amd.com
>>> Subject: RE: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import
>>> BaseIoLibIntrinsic package
>>>
>>> On 2017-03-08 07:41:58, Gao, Liming wrote:

> -Original Message-
> From: Justen, Jordan L

> One other thought is, should we consider a DxeSmm alternative .inf
> for BaseIoLibIntrinsic.inf? In that case we could use a global
> variable to help out. Maybe this could prevent the concern that
> might drive a new PCD to be added?
>
> -Jordan
 Current patch has stored the check state into data section. In PEI
 phase, the data section can't be written. So, every call will check
 CpuId. In DXE and SMM phase, the data section can be written. The
 first call will cache the check state. So, no DxeSmm INF is
 required.

>>>
>>> I don't think we can attempt to write a variable to memory in generic
>>> SEC/PEI code. Some flash memory treat memory writes as an I/O for
>>> programming purposes. I think we added
>>> PcdGuidedExtractHandlerTableAddress for this reason. This is why I
>>> suggested that maybe we could add a DXE/SMM .inf where we could
>> assume
>>> writeable global variables exit.
>>>
>>> -Jordan
>>
>> I get your point. So, I suggest to always check SEV in BaseIoLib. If people
>> meet with the performance issue, DXE/SMM version IoLib can be added
>> later.
>>
> 

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


Re: [edk2] [PATCH 3/3] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

2017-03-09 Thread Leif Lindholm
On Thu, Mar 09, 2017 at 05:04:01PM +0100, Ard Biesheuvel wrote:
> Instead of having a build time switch to prevent the FDT configuration
> table from being installed, make this behavior dependent on whether we
> are passing ACPI tables to the OS. This is done by looking for the
> ACPI 2.0 configuration table, and only installing the FDT one if the
> ACPI one cannot be found.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 10 --
>  ArmVirtPkg/ArmVirtQemu.dsc   |  5 -
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 15 ++-
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 ++---
>  4 files changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index a5ec42166445..efe83a383d55 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
># EFI_VT_100_GUID.
>#
>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
> 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
> 0x4D}|VOID*|0x0007
> -
> -[PcdsFeatureFlag]
> -  #
> -  # Pure ACPI boot
> -  #
> -  # Inhibit installation of the FDT as a configuration table if this feature
> -  # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
> -  # description of the platform, and it is up to the OS to choose.
> -  #
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x000a
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 477dfdcfc764..7b266b98b949 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -34,7 +34,6 @@ [Defines]
># -D FLAG=VALUE
>#
>DEFINE SECURE_BOOT_ENABLE  = FALSE
> -  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
>  
>  !include ArmVirtPkg/ArmVirt.dsc.inc
>  
> @@ -94,10 +93,6 @@ [PcdsFeatureFlag.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>  
> -!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
> -!endif
> -
>  [PcdsFixedAtBuild.common]
>gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>  !if $(ARCH) == AARCH64
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 0327af5739f2..2981977f3d20 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -312,12 +313,16 @@ OnReadyToBoot (
>)
>  {
>EFI_STATUS  Status;
> +  VOID*Table;
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// Only install the FDT as a configuration table if we want to leave it 
> up
> -// to the OS to decide whether it prefers ACPI over DT.
> -//
> +  //
> +  // Only install the FDT as a configuration table if we are not exposing
> +  // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has
> +  // no meaning on ARM since we need at least ACPI 5.0 support, and the
> +  // 64-bit ACPI 2.0 table GUID is mandatory in that case.
> +  //
> +  Status = EfiGetSystemConfigurationTable (, );
> +  if (EFI_ERROR (Status) || Table == NULL) {
>  Status = gBS->InstallConfigurationTable (, 
> mDeviceTreeBase);
>  ASSERT_EFI_ERROR (Status);
>}
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 00017727c32c..9861f41e968b 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -37,17 +37,16 @@ [LibraryClasses]
>HobLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
> +  UefiLib
>  
>  [Protocols]
>gFdtClientProtocolGuid  ## PRODUCES
>  
>  [Guids]
> +  gEfiAcpi20TableGuid
>gEfiEventReadyToBootGuid
>gFdtHobGuid
>gFdtTableGuid
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>TRUE
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/3] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

2017-03-09 Thread Leif Lindholm
On Thu, Mar 09, 2017 at 05:04:00PM +0100, Ard Biesheuvel wrote:
> Wait for ReadyToBoot to install the FDT configuration table. This allows

Semantic nitpicking: "Wait for" sounds a little like a delay.
"Defer FDT configuration table installation until ReadyToBoot is
signaled."?

Fold in if you agree that's an improvement.

> any driver to make modifications in the mean time, and will also allow us
> to defer the decision of whether to install it in the first place to later
> on in the boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 41 
>  ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> index 7cc0c44ca12a..0327af5739f2 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
> @@ -303,6 +303,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
>GetOrInsertChosenNode,
>  };
>  
> +STATIC
> +VOID
> +EFIAPI
> +OnReadyToBoot (
> +  EFI_EVENT   Event,
> +  VOID*Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> +//
> +// Only install the FDT as a configuration table if we want to leave it 
> up
> +// to the OS to decide whether it prefers ACPI over DT.
> +//

I was going to complain about how it felt pointless to introduce this
only to delete it in 2/3, but I really like how it improves the
clarity of 3/3.

Reviewed-by: Leif Lindholm 

> +Status = gBS->InstallConfigurationTable (, 
> mDeviceTreeBase);
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  gBS->CloseEvent (Event);
> +}
> +
> +STATIC EFI_EVENT ReadyToBootEvent;
> +
>  EFI_STATUS
>  EFIAPI
>  InitializeFdtClientDxe (
> @@ -330,14 +354,15 @@ InitializeFdtClientDxe (
>  
>DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// Only install the FDT as a configuration table if we want to leave it 
> up
> -// to the OS to decide whether it prefers ACPI over DT.
> -//
> -Status = gBS->InstallConfigurationTable (, DeviceTreeBase);
> -ASSERT_EFI_ERROR (Status);
> -  }
> +  Status = gBS->CreateEventEx (
> +  EVT_NOTIFY_SIGNAL,
> +  TPL_CALLBACK,
> +  OnReadyToBoot,
> +  NULL,
> +  ,
> +  
> +  );
> +  ASSERT_EFI_ERROR (Status);
>  
>return gBS->InstallProtocolInterface (, 
> ,
>  EFI_NATIVE_INTERFACE, );
> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
> b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> index 3a0cd37040eb..00017727c32c 100644
> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
> @@ -42,6 +42,7 @@ [Protocols]
>gFdtClientProtocolGuid  ## PRODUCES
>  
>  [Guids]
> +  gEfiEventReadyToBootGuid
>gFdtHobGuid
>gFdtTableGuid
>  
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 1/3] ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node

2017-03-09 Thread Leif Lindholm
On Thu, Mar 09, 2017 at 05:03:59PM +0100, Ard Biesheuvel wrote:
> Disable the PL031 RTC DT node unconditionally rather than only when
> the DT will be exposed to the OS. This allows us to defer the decision
> whether to expose it to the OS to a later time without creating an
> additional dependency on the FDT client code by the RTC driver.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

This seems like an improvement even without the rest of the series.
Reviewed-by: Leif Lindholm 

> ---
>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 
> 22 +---
>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  
> 3 ---
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git 
> a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c 
> b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> index 82de7a51b32e..d168424a52f5 100644
> --- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
> @@ -66,18 +66,16 @@ ArmVirtPL031FdtClientLibConstructor (
>  
>DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
>  
> -  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
> -//
> -// UEFI takes ownership of the RTC hardware, and exposes its 
> functionality
> -// through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> -// need to disable it in the device tree to prevent the OS from attaching
> -// its device driver as well.
> -//
> -Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> -  "disabled", sizeof ("disabled"));
> -if (EFI_ERROR (Status)) {
> -DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
> -}
> +  //
> +  // UEFI takes ownership of the RTC hardware, and exposes its functionality
> +  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
> +  // need to disable it in the device tree to prevent the OS from attaching
> +  // its device driver as well.
> +  //
> +  Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> +"disabled", sizeof ("disabled"));
> +  if (EFI_ERROR (Status)) {
> +  DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
>}
>  
>return EFI_SUCCESS;
> diff --git 
> a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf 
> b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> index 32dbff6f0852..342193651a86 100644
> --- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
> @@ -42,8 +42,5 @@ [Protocols]
>  [Pcd]
>gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
>  
> -[FeaturePcd]
> -  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
> -
>  [Depex]
>gFdtClientProtocolGuid
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 3/3] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

2017-03-09 Thread Ard Biesheuvel
Instead of having a build time switch to prevent the FDT configuration
table from being installed, make this behavior dependent on whether we
are passing ACPI tables to the OS. This is done by looking for the
ACPI 2.0 configuration table, and only installing the FDT one if the
ACPI one cannot be found.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtPkg.dec| 10 --
 ArmVirtPkg/ArmVirtQemu.dsc   |  5 -
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 15 ++-
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  5 ++---
 4 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index a5ec42166445..efe83a383d55 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # EFI_VT_100_GUID.
   #
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
0x4D}|VOID*|0x0007
-
-[PcdsFeatureFlag]
-  #
-  # Pure ACPI boot
-  #
-  # Inhibit installation of the FDT as a configuration table if this feature
-  # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
-  # description of the platform, and it is up to the OS to choose.
-  #
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x000a
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 477dfdcfc764..7b266b98b949 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -34,7 +34,6 @@ [Defines]
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE  = FALSE
-  DEFINE PURE_ACPI_BOOT_ENABLE   = FALSE
 
 !include ArmVirtPkg/ArmVirt.dsc.inc
 
@@ -94,10 +93,6 @@ [PcdsFeatureFlag.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
 
-!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
-!endif
-
 [PcdsFixedAtBuild.common]
   gArmPlatformTokenSpaceGuid.PcdCoreCount|1
 !if $(ARCH) == AARCH64
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 0327af5739f2..2981977f3d20 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -312,12 +313,16 @@ OnReadyToBoot (
   )
 {
   EFI_STATUS  Status;
+  VOID*Table;
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// Only install the FDT as a configuration table if we want to leave it up
-// to the OS to decide whether it prefers ACPI over DT.
-//
+  //
+  // Only install the FDT as a configuration table if we are not exposing
+  // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has
+  // no meaning on ARM since we need at least ACPI 5.0 support, and the
+  // 64-bit ACPI 2.0 table GUID is mandatory in that case.
+  //
+  Status = EfiGetSystemConfigurationTable (, );
+  if (EFI_ERROR (Status) || Table == NULL) {
 Status = gBS->InstallConfigurationTable (, mDeviceTreeBase);
 ASSERT_EFI_ERROR (Status);
   }
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
index 00017727c32c..9861f41e968b 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
@@ -37,17 +37,16 @@ [LibraryClasses]
   HobLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
+  UefiLib
 
 [Protocols]
   gFdtClientProtocolGuid  ## PRODUCES
 
 [Guids]
+  gEfiAcpi20TableGuid
   gEfiEventReadyToBootGuid
   gFdtHobGuid
   gFdtTableGuid
 
-[FeaturePcd]
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
-
 [Depex]
   TRUE
-- 
2.7.4

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


[edk2] [PATCH 1/3] ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node

2017-03-09 Thread Ard Biesheuvel
Disable the PL031 RTC DT node unconditionally rather than only when
the DT will be exposed to the OS. This allows us to defer the decision
whether to expose it to the OS to a later time without creating an
additional dependency on the FDT client code by the RTC driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 
+---
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 
---
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git 
a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c 
b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
index 82de7a51b32e..d168424a52f5 100644
--- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
@@ -66,18 +66,16 @@ ArmVirtPL031FdtClientLibConstructor (
 
   DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// UEFI takes ownership of the RTC hardware, and exposes its functionality
-// through the UEFI Runtime Services GetTime, SetTime, etc. This means we
-// need to disable it in the device tree to prevent the OS from attaching
-// its device driver as well.
-//
-Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
-  "disabled", sizeof ("disabled"));
-if (EFI_ERROR (Status)) {
-DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
-}
+  //
+  // UEFI takes ownership of the RTC hardware, and exposes its functionality
+  // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
+  // need to disable it in the device tree to prevent the OS from attaching
+  // its device driver as well.
+  //
+  Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
+"disabled", sizeof ("disabled"));
+  if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
   }
 
   return EFI_SUCCESS;
diff --git 
a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf 
b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
index 32dbff6f0852..342193651a86 100644
--- a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
@@ -42,8 +42,5 @@ [Protocols]
 [Pcd]
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
 
-[FeaturePcd]
-  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
-
 [Depex]
   gFdtClientProtocolGuid
-- 
2.7.4

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


[edk2] [PATCH 2/3] ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot

2017-03-09 Thread Ard Biesheuvel
Wait for ReadyToBoot to install the FDT configuration table. This allows
any driver to make modifications in the mean time, and will also allow us
to defer the decision of whether to install it in the first place to later
on in the boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 41 
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  1 +
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
index 7cc0c44ca12a..0327af5739f2 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c
@@ -303,6 +303,30 @@ STATIC FDT_CLIENT_PROTOCOL mFdtClientProtocol = {
   GetOrInsertChosenNode,
 };
 
+STATIC
+VOID
+EFIAPI
+OnReadyToBoot (
+  EFI_EVENT   Event,
+  VOID*Context
+  )
+{
+  EFI_STATUS  Status;
+
+  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
+//
+// Only install the FDT as a configuration table if we want to leave it up
+// to the OS to decide whether it prefers ACPI over DT.
+//
+Status = gBS->InstallConfigurationTable (, mDeviceTreeBase);
+ASSERT_EFI_ERROR (Status);
+  }
+
+  gBS->CloseEvent (Event);
+}
+
+STATIC EFI_EVENT ReadyToBootEvent;
+
 EFI_STATUS
 EFIAPI
 InitializeFdtClientDxe (
@@ -330,14 +354,15 @@ InitializeFdtClientDxe (
 
   DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, mDeviceTreeBase));
 
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// Only install the FDT as a configuration table if we want to leave it up
-// to the OS to decide whether it prefers ACPI over DT.
-//
-Status = gBS->InstallConfigurationTable (, DeviceTreeBase);
-ASSERT_EFI_ERROR (Status);
-  }
+  Status = gBS->CreateEventEx (
+  EVT_NOTIFY_SIGNAL,
+  TPL_CALLBACK,
+  OnReadyToBoot,
+  NULL,
+  ,
+  
+  );
+  ASSERT_EFI_ERROR (Status);
 
   return gBS->InstallProtocolInterface (, ,
 EFI_NATIVE_INTERFACE, );
diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf 
b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
index 3a0cd37040eb..00017727c32c 100644
--- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
@@ -42,6 +42,7 @@ [Protocols]
   gFdtClientProtocolGuid  ## PRODUCES
 
 [Guids]
+  gEfiEventReadyToBootGuid
   gFdtHobGuid
   gFdtTableGuid
 
-- 
2.7.4

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


[edk2] [PATCH 0/3] ArmVirtQemu: make DT vs ACPI support mutually exclusive

2017-03-09 Thread Ard Biesheuvel
Instead of supplying both ACPI and DT hw descriptions, and allow the latter
to be inihibited by setting a compile time define, make DT table installation
dependent on the absence of a ACPI 2.0 table when the ReadyToBoot even fires.

Ard Biesheuvel (3):
  ArmVirtPkg/ArmVirtPL031FdtClientLib: unconditionally disable DT node
  ArmVirtPkg/FdtClientDxe: install DT configuration table at ReadyToBoot
  ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent

 ArmVirtPkg/ArmVirtPkg.dec| 10 
-
 ArmVirtPkg/ArmVirtQemu.dsc   |  5 
---
 ArmVirtPkg/FdtClientDxe/FdtClientDxe.c   | 46 

 ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf |  6 
+--
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 22 
+-
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf |  3 
--
 6 files changed, 51 insertions(+), 41 deletions(-)

-- 
2.7.4

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


Re: [edk2] [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import BaseIoLibIntrinsic package

2017-03-09 Thread Duran, Leo
OK, what I'm hearing is:
- Let's leave the Fifo routines in BaseIoLib (as we currently have)
- And let's add the SEV checks to BaseIoLib, so that OvmPkg (et al) can consume 
it as-is

If so, I could put a patch-set together for that... Please confirm.
BTW, I'd try to minimize the need for CPUID (e.g., check it once and cache the 
result)

Leo

> -Original Message-
> From: Gao, Liming [mailto:liming@intel.com]
> Sent: Wednesday, March 08, 2017 7:48 PM
> To: Justen, Jordan L ; Kinney, Michael D
> ; edk2-devel@lists.01.org; Brijesh Singh
> ; Laszlo Ersek 
> Cc: Lendacky, Thomas ; Duran, Leo
> ; brijesh.s...@amd.com
> Subject: RE: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import
> BaseIoLibIntrinsic package
> 
> 
> 
> > -Original Message-
> > From: Justen, Jordan L
> > Sent: Thursday, March 9, 2017 2:59 AM
> > To: Gao, Liming ; Kinney, Michael D
> > ; edk2-devel@lists.01.org; Brijesh Singh
> > ; Laszlo Ersek 
> > Cc: thomas.lenda...@amd.com; leo.du...@amd.com;
> brijesh.s...@amd.com
> > Subject: RE: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import
> > BaseIoLibIntrinsic package
> >
> > On 2017-03-08 07:41:58, Gao, Liming wrote:
> > >
> > > >-Original Message-
> > > >From: Justen, Jordan L
> > >
> > > >One other thought is, should we consider a DxeSmm alternative .inf
> > > >for BaseIoLibIntrinsic.inf? In that case we could use a global
> > > >variable to help out. Maybe this could prevent the concern that
> > > >might drive a new PCD to be added?
> > > >
> > > >-Jordan
> > > Current patch has stored the check state into data section. In PEI
> > > phase, the data section can't be written. So, every call will check
> > > CpuId. In DXE and SMM phase, the data section can be written. The
> > > first call will cache the check state. So, no DxeSmm INF is
> > > required.
> > >
> >
> > I don't think we can attempt to write a variable to memory in generic
> > SEC/PEI code. Some flash memory treat memory writes as an I/O for
> > programming purposes. I think we added
> > PcdGuidedExtractHandlerTableAddress for this reason. This is why I
> > suggested that maybe we could add a DXE/SMM .inf where we could
> assume
> > writeable global variables exit.
> >
> > -Jordan
> 
> I get your point. So, I suggest to always check SEV in BaseIoLib. If people
> meet with the performance issue, DXE/SMM version IoLib can be added
> later.
> 

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


Re: [edk2] [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

2017-03-09 Thread Laszlo Ersek
On 03/09/17 13:26, Ard Biesheuvel wrote:
> On 9 March 2017 at 12:01, Laszlo Ersek  wrote:
>> On 03/09/17 09:16, Ard Biesheuvel wrote:
>>> On 8 March 2017 at 20:05, Laszlo Ersek  wrote:
 This series replaces the PURE_ACPI_BOOT_ENABLE build option with dynamic
 behavior, matching QEMU's (inverse sense) "-no-acpi" switch. In
 particular, DT and ACPI are no longer exposed to the guest at the same
 time. (DT is exposed with "-no-acpi", or else ACPI is exposed without
 "-no-acpi".)

 Repo:   https://github.com/lersek/edk2.git
 Branch: dynamic_pure_acpi
 RHBZ:   https://bugzilla.redhat.com/show_bug.cgi?id=1430262

 Tested with RHEL-7.3 for ARM and Fedora 24 guests (DT vs. ACPI).

 Cc: Ard Biesheuvel 

>>>
>>> Hi Laszlo,
>>>
>>> This looks complicated to me. Given that it is arguably a policy to
>>> only expose on h/w description or the other, couldn't we simply remove
>>> the FDT config table in BDS if an ACPI/ACPI2.0 config table is
>>> present?
>>
>> Technically we could do that, but I dislike it for two reasons:
>>
>> - BDS is often the first victim found when looking for a driver to add
>> new code to that doesn't seem to fit very well elsewhere. That doesn't
>> make BDS any better a recipient, however. "For lack of a better driver"
>> is not a strong enough argument to dump code into BDS. If there's really
>> no better "topical" driver, then the code usually goes to PlatformDxe.
>>
>> - Installing a sysconfig table (or any other system-wide resource) in
>> one driver, then undoing it in another driver, should be avoided as much
>> as possible, because it leads to non-trivial lifecycles and boggles our
>> minds over the longer term. If we can come to a decision that the table
>> shouldn't be installed in the first place, we should pursue that.
>>
>> Another approach we could look into is: move the installation of the
>> sysconfig table from FdtClientDxe to AcpiPlatformDxe. Look for the ACPI
>> payload first, and fall back to installing DT (from within
>> AcpiPlatformDxe). However, DT should be installed even in builds (like
>> ARM32) that don't contain AcpiPlatformDxe at all.
>>
> 
> Or we could hook to the ReadyToBoot event in FdtClientDxe, and install
> the DT config table if there is no ACPI/ACPI2.0 table registered.

Yes, that's doable in our case, because we control the full platform.

Installing tables (any kinds of tables) in ReadyToBoot and similar event
handlers is generally a bad idea, because everyone thinks, "okay I'll
wait until the rest of the system is done setting up, and I'll just add
my stuff afterwards". Obviously, this results in much of the logic being
simply moved to such event callbacks, and the invocation order of
callbacks remains unspecified.

In more precise terms, if the ACPI tables too were installed in a
ReadyToBoot callback, your suggestion above would not work. And our ACPI
tables are not installed in a ReadyToBoot callback partly because I
ultimately introduced a separate event group for "PCI bridges have been
connected", and we signal it now explicitly from BDS (device connections
are BDS jurisdiction, and QEMU's ACPI generation depends on PCI state).

I just want to point out that we have a kind of "capital" here. By
carefully coding stuff we build capital, and by hooking stuff into
ReadyToBoot callbacks we spend (hopefully not "squander") that capital.

Originally, the APM Mustang firmware (open source, so I can talk about
it) would first install ACPI tables with constant, platform-tailored
contents (built from *.asl / *.aslc files), but reusing the stock
AcpiPlatformDxe C code without customization. Then it would install a
ReadyToBoot callback which looked up the right DSDT or SSDT, by walking
the table tree manually, and then it would poke data into the installed
table (DSDT or SSDT) in-place, using the ACPI SDT protocol.

Of course it was completely bogus and unreliable, and I changed the
constant table to contain external references, and I provided those
external references in a minimal, hand- and runtime- built SSDT, right
in AcpiPlatformDxe.

I'm not trying to carefully compose a strawman argument here, just
presenting why I'm nervous about ReadyToBoot callbacks that try to rely
on ordering between system-wide resources.


Also, please don't forget about the other (current) consumer of the
feature PCD, ArmVirtPL031FdtClientLib, which is plugged into
RealTimeClockRuntimeDxe. How do you suggest to rewrite the PCD test in
that driver under the ReadyToBoot callback scenario?

RealTimeClockRuntimeDxe's dispatch order is unspecified relative to the
installation of ACPI tables, so you couldn't look at the latter's
presence to see if the DTB needs an update. (In fact, because
AcpiPlatformDxe's main actions are cued in from BDS in practice, the
tables would be guaranteed not to exist when RealTimeClockRuntimeDxe looks.)

So ArmVirtPL031FdtClientLib would 

Re: [edk2] [PATCH v2 0/2] MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT, X_DSDT} mutual exclusion

2017-03-09 Thread Laszlo Ersek
On 03/08/17 20:58, Laszlo Ersek wrote:
> This is version 2 of the series originally posted at
> 
> and further discussed at
> .
> 
> This version contains minor updates (noted on each patch) that don't
> affect functionality.
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: fadt_dsdt_v2
> 
> Cc: Al Stone 
> Cc: Ard Biesheuvel 
> Cc: Feng Tian 
> Cc: Igor Mammedov 
> Cc: Jiewen Yao 
> Cc: Leo Duran 
> Cc: Michael Tsirkin 
> Cc: Phil Dennis-Jordan 
> Cc: Star Zeng 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   MdeModulePkg/AcpiTableDxe: condense whitespace around
> FADT.{DSDT,X_DSDT}
>   MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT,X_DSDT} mutual exclusion
> 
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 78 
> 
>  1 file changed, 63 insertions(+), 15 deletions(-)
> 

Thanks All, this is now pushed (commit range e0e1cfcbbb24..198a46d768fb).

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


Re: [edk2] [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

2017-03-09 Thread Ard Biesheuvel
On 9 March 2017 at 12:01, Laszlo Ersek  wrote:
> On 03/09/17 09:16, Ard Biesheuvel wrote:
>> On 8 March 2017 at 20:05, Laszlo Ersek  wrote:
>>> This series replaces the PURE_ACPI_BOOT_ENABLE build option with dynamic
>>> behavior, matching QEMU's (inverse sense) "-no-acpi" switch. In
>>> particular, DT and ACPI are no longer exposed to the guest at the same
>>> time. (DT is exposed with "-no-acpi", or else ACPI is exposed without
>>> "-no-acpi".)
>>>
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: dynamic_pure_acpi
>>> RHBZ:   https://bugzilla.redhat.com/show_bug.cgi?id=1430262
>>>
>>> Tested with RHEL-7.3 for ARM and Fedora 24 guests (DT vs. ACPI).
>>>
>>> Cc: Ard Biesheuvel 
>>>
>>
>> Hi Laszlo,
>>
>> This looks complicated to me. Given that it is arguably a policy to
>> only expose on h/w description or the other, couldn't we simply remove
>> the FDT config table in BDS if an ACPI/ACPI2.0 config table is
>> present?
>
> Technically we could do that, but I dislike it for two reasons:
>
> - BDS is often the first victim found when looking for a driver to add
> new code to that doesn't seem to fit very well elsewhere. That doesn't
> make BDS any better a recipient, however. "For lack of a better driver"
> is not a strong enough argument to dump code into BDS. If there's really
> no better "topical" driver, then the code usually goes to PlatformDxe.
>
> - Installing a sysconfig table (or any other system-wide resource) in
> one driver, then undoing it in another driver, should be avoided as much
> as possible, because it leads to non-trivial lifecycles and boggles our
> minds over the longer term. If we can come to a decision that the table
> shouldn't be installed in the first place, we should pursue that.
>
> Another approach we could look into is: move the installation of the
> sysconfig table from FdtClientDxe to AcpiPlatformDxe. Look for the ACPI
> payload first, and fall back to installing DT (from within
> AcpiPlatformDxe). However, DT should be installed even in builds (like
> ARM32) that don't contain AcpiPlatformDxe at all.
>

Or we could hook to the ReadyToBoot event in FdtClientDxe, and install
the DT config table if there is no ACPI/ACPI2.0 table registered.

> This series indeed turned out a bit more complex than I had expected,
> but it was the one I could post with a good conscience. Can you perhaps
> identify the part(s) in more detail that seem overly complex to you?
>

Building the same library in two different ways, having to call a
library constructor explicitly in some cases and muck about with TPL
levels to prevent a protocol notify from triggering are all things I
would really like to avoid tbh
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.

2017-03-09 Thread Anthony PERARD
On Tue, Feb 21, 2017 at 02:57:07PM +0800, Jiewen Yao wrote:
> Add memory attribute setting in CpuArch protocol.
> Previous SetMemoryAttributes() API only supports cache attribute setting.
> 
> This patch updated SetMemoryAttributes() API to support memory attribute
> setting by updating CPU page table.
> 
> Cc: Jeff Fan 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao 

Hi,

This patch doesn't work on AMD (when running OVMF on Xen). With an AMD
cpu, reading MSR 0x1a0 cause a General Protection Fault.

> +  AsmCpuid (0x8000, , NULL, NULL, NULL);
> +  if (RegEax > 0x8000) {
> +AsmCpuid (0x8001, NULL, NULL, NULL, );
> +if ((RegEdx & BIT20) != 0) {
> +  // XD supported

This next read is where the fault is taken.

> +  if ((AsmReadMsr64 (0x01A0) & BIT34) == 0) {
> +// XD enabled
> +if ((AsmReadMsr64 (0xC080) & BIT11) != 0) {
> +  // XD activated
> +  PagingContext->ContextData.Ia32.Attributes |= 
> PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED;
> +}
> +  }
> +}

>From OVMF output:

 X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID -  

RIP  - 2F323ABE, CS  - 0018, RFLAGS - 00010002
ExceptionData - 
RAX  - 8001, RCX - 01A0, RDX - 2FD3FBFF
RBX  - 2F422CB8, RSP - 2F422C68, RBP - 2EE0AD98
RSI  - 2F3AF018, RDI - 2F3229E4
R8   - , R9  - , R10 - 
R11  - 2F422BAD, R12 - , R13 - 
R14  - , R15 - 2F44A980
DS   - 0008, ES  - 0008, FS  - 0008
GS   - 0008, SS  - 0008
CR0  - C0010033, CR2 - , CR3 - 2F3C1000
CR4  - 0668, CR8 - 
DR0  - , DR1 - , DR2 - 
DR3  - , DR6 - 0FF0, DR7 - 0400
GDTR - FF80 001F, LDTR - 
IDTR - 2B68DD90 021F,   TR - 
FXSAVE_STATE - 2F4228C0
 Find PE image 
/home/osstest/build.106538.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe/DEBUG/CpuDxe.dll
 (ImageBase=2F321000, EntryPoint=2F321271) 


Thanks,

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


Re: [edk2] [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

2017-03-09 Thread Laszlo Ersek
On 03/09/17 09:16, Ard Biesheuvel wrote:
> On 8 March 2017 at 20:05, Laszlo Ersek  wrote:
>> This series replaces the PURE_ACPI_BOOT_ENABLE build option with dynamic
>> behavior, matching QEMU's (inverse sense) "-no-acpi" switch. In
>> particular, DT and ACPI are no longer exposed to the guest at the same
>> time. (DT is exposed with "-no-acpi", or else ACPI is exposed without
>> "-no-acpi".)
>>
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: dynamic_pure_acpi
>> RHBZ:   https://bugzilla.redhat.com/show_bug.cgi?id=1430262
>>
>> Tested with RHEL-7.3 for ARM and Fedora 24 guests (DT vs. ACPI).
>>
>> Cc: Ard Biesheuvel 
>>
> 
> Hi Laszlo,
> 
> This looks complicated to me. Given that it is arguably a policy to
> only expose on h/w description or the other, couldn't we simply remove
> the FDT config table in BDS if an ACPI/ACPI2.0 config table is
> present?

Technically we could do that, but I dislike it for two reasons:

- BDS is often the first victim found when looking for a driver to add
new code to that doesn't seem to fit very well elsewhere. That doesn't
make BDS any better a recipient, however. "For lack of a better driver"
is not a strong enough argument to dump code into BDS. If there's really
no better "topical" driver, then the code usually goes to PlatformDxe.

- Installing a sysconfig table (or any other system-wide resource) in
one driver, then undoing it in another driver, should be avoided as much
as possible, because it leads to non-trivial lifecycles and boggles our
minds over the longer term. If we can come to a decision that the table
shouldn't be installed in the first place, we should pursue that.

Another approach we could look into is: move the installation of the
sysconfig table from FdtClientDxe to AcpiPlatformDxe. Look for the ACPI
payload first, and fall back to installing DT (from within
AcpiPlatformDxe). However, DT should be installed even in builds (like
ARM32) that don't contain AcpiPlatformDxe at all.

This series indeed turned out a bit more complex than I had expected,
but it was the one I could post with a good conscience. Can you perhaps
identify the part(s) in more detail that seem overly complex to you?

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


[edk2] [Patch][edk2-platforms] Vlv2TbltDevicePkg: Add BGRT table.

2017-03-09 Thread lushifex
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: lushifex 
---
 Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c |  4 ++--
 Vlv2TbltDevicePkg/PlatformPkg.fdf  | 12 +++-
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf   | 12 +++-
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|  3 ++-
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |  3 ++-
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |  3 ++-
 6 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c 
b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
index 6e8364b..38eaeb2 100644
--- a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2004  - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2004  - 2017, Intel Corporation. All rights reserved.



   This program and the accompanying materials are licensed and made available 
under

   the terms and conditions of the BSD License that accompanies this 
distribution.  

@@ -1891,7 +1891,7 @@ PlatformBdsPolicyBehavior (
 // console directly.
 //
 BdsLibConnectAllDefaultConsoles ();
-PlatformBdsDiagnostics (IGNORE, TRUE, BaseMemoryTest);
+PlatformBdsDiagnostics (IGNORE, FALSE, BaseMemoryTest);
 
 //
 // Perform some platform specific connect sequence
diff --git a/Vlv2TbltDevicePkg/PlatformPkg.fdf 
b/Vlv2TbltDevicePkg/PlatformPkg.fdf
index ae4ee2d..b795d60 100644
--- a/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -1,7 +1,7 @@
 #/** @file
 # FDF file of Platform.
 #
-# Copyright (c) 2008  - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2008  - 2017, Intel Corporation. All rights reserved.
 #  

 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License that accompanies this 
distribution.  
@@ -404,6 +404,10 @@ APRIORI DXE {
 FILE FREEFORM = C3E36D09-8294-4b97-A857-D5288FE33E28 {
 SECTION RAW = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(DXE_ARCHITECTURE)/BiosId.bin
   }
+  
+FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
+SECTION RAW = MdeModulePkg/Logo/Logo.bmp
+  }
 
   #
   # EDK II Related Platform codes
@@ -418,6 +422,7 @@ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 !if $(ACPI50_ENABLE) == TRUE
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
+INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 !endif
 
 
@@ -1128,6 +1133,11 @@ FV = BiosUpdate
 RAW ASL   Optional|.aml
   }
 
+[Rule.Common.USER_DEFINED.LOGO]
+  FILE FREEFORM = $(NAMED_GUID) {
+RAW BIN   |.bmp
+  }
+
 [Rule.Common.ACPITABLE]
   FILE FREEFORM = $(NAMED_GUID) {
 RAW ACPI  Optional|.acpi
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index 43d20ee..334a5fc 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -1,7 +1,7 @@
 #/** @file
 # FDF file of Platform.
 #
-# Copyright (c) 2008  - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2008  - 2017, Intel Corporation. All rights reserved.
 #  

 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License that accompanies this 
distribution.  
@@ -357,6 +357,10 @@ APRIORI DXE {
 FILE FREEFORM = C3E36D09-8294-4b97-A857-D5288FE33E28 {
 SECTION RAW = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(DXE_ARCHITECTURE)/BiosId.bin
   }
+  
+FILE FREEFORM = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) {
+SECTION RAW = MdeModulePkg/Logo/Logo.bmp
+  }
 
   #
   # EDK II Related Platform codes
@@ -371,6 +375,7 @@ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 !if $(ACPI50_ENABLE) == TRUE
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
+INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 !endif
 
 
@@ -1087,6 +1092,11 @@ FV = BiosUpdate
 RAW ASL   Optional|.aml
   }
 
+[Rule.Common.USER_DEFINED.LOGO]
+  FILE FREEFORM = $(NAMED_GUID) {
+RAW BIN   |.bmp
+  }
+
 [Rule.Common.ACPITABLE]
   FILE FREEFORM = $(NAMED_GUID) {
 RAW ACPI  

[edk2] [PATCH 02/11] UefiCpuPkg/CpuS3DataDxe: Consume the existing PcdCpuS3DataAddress

2017-03-09 Thread Jeff Fan
If PCD PcdCpuS3DataAddress is set before, CpuS3DataDxe should get RegisterTable
and PreSmmRegisterTable from existing PCD pointed buffer and needn't to allocate
new buffer for RegisterTable and PreSmmRegisterTable.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 54 ++---
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c 
b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
index 07c7102..dccb406 100644
--- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
@@ -160,12 +160,18 @@ CpuS3DataInitialize (
   VOID   *Gdt;
   VOID   *Idt;
   EFI_EVENT  Event;
+  ACPI_CPU_DATA  *OldAcpiCpuData;
 
   if (!PcdGetBool (PcdAcpiS3Enable)) {
 return EFI_UNSUPPORTED;
   }
 
   //
+  // Set PcdCpuS3DataAddress to the base address of the ACPI_CPU_DATA structure
+  //
+  OldAcpiCpuData = (ACPI_CPU_DATA *) (UINTN) PcdGet64 (PcdCpuS3DataAddress);
+
+  //
   // Allocate ACPI NVS memory below 4G memory for use on ACPI S3 resume.
   //
   AcpiCpuDataEx = AllocateAcpiNvsMemoryBelow4G (sizeof (ACPI_CPU_DATA_EX));
@@ -229,32 +235,38 @@ CpuS3DataInitialize (
   AcpiCpuDataEx->GdtrProfile.Base = (UINTN)Gdt;
   AcpiCpuDataEx->IdtrProfile.Base = (UINTN)Idt;
 
-  //
-  // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for 
all CPUs
-  //
-  TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
-  RegisterTable = (CPU_REGISTER_TABLE *)AllocateAcpiNvsMemoryBelow4G 
(TableSize);
-  ASSERT (RegisterTable != NULL);
-  for (Index = 0; Index < NumberOfCpus; Index++) {
-Status = MpServices->GetProcessorInfo (
+  if (OldAcpiCpuData != NULL) {
+AcpiCpuData->RegisterTable   = OldAcpiCpuData->RegisterTable;
+AcpiCpuData->PreSmmInitRegisterTable = 
OldAcpiCpuData->PreSmmInitRegisterTable;
+  } else {
+//
+// Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for 
all CPUs
+//
+TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
+RegisterTable = (CPU_REGISTER_TABLE *)AllocateAcpiNvsMemoryBelow4G 
(TableSize);
+ASSERT (RegisterTable != NULL);
+
+for (Index = 0; Index < NumberOfCpus; Index++) {
+  Status = MpServices->GetProcessorInfo (
MpServices,
Index,

);
-ASSERT_EFI_ERROR (Status);
-
-RegisterTable[Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
-RegisterTable[Index].TableLength= 0;
-RegisterTable[Index].AllocatedSize  = 0;
-RegisterTable[Index].RegisterTableEntry = 0;
-
-RegisterTable[NumberOfCpus + Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
-RegisterTable[NumberOfCpus + Index].TableLength= 0;
-RegisterTable[NumberOfCpus + Index].AllocatedSize  = 0;
-RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
+  ASSERT_EFI_ERROR (Status);
+
+  RegisterTable[Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
+  RegisterTable[Index].TableLength= 0;
+  RegisterTable[Index].AllocatedSize  = 0;
+  RegisterTable[Index].RegisterTableEntry = 0;
+
+  RegisterTable[NumberOfCpus + Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
+  RegisterTable[NumberOfCpus + Index].TableLength= 0;
+  RegisterTable[NumberOfCpus + Index].AllocatedSize  = 0;
+  RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
+}
+AcpiCpuData->RegisterTable   = 
(EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
+AcpiCpuData->PreSmmInitRegisterTable = 
(EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
   }
-  AcpiCpuData->RegisterTable   = 
(EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
-  AcpiCpuData->PreSmmInitRegisterTable = 
(EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
 
   //
   // Set PcdCpuS3DataAddress to the base address of the ACPI_CPU_DATA structure
-- 
2.9.3.windows.2

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


[edk2] [PATCH 04/11] UefiCpuPkg/Msr: Add CPUID signature check MACROs

2017-03-09 Thread Jeff Fan
All model-specific MSRs are related to processor signatures that are defined in
each section in Chapter 35 Model-Specific-Registers (MSR), Intel(R) 64 and
IA-32 Architectures Software Developer's Manual, Volume 3, September 2016.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Include/Register/Msr/AtomMsr.h| 22 +-
 UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h   | 21 -
 UefiCpuPkg/Include/Register/Msr/Core2Msr.h   | 19 ++-
 UefiCpuPkg/Include/Register/Msr/CoreMsr.h| 18 +-
 UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h| 18 +-
 UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h| 18 +-
 UefiCpuPkg/Include/Register/Msr/HaswellMsr.h | 20 +++-
 UefiCpuPkg/Include/Register/Msr/IvyBridgeMsr.h   | 18 +-
 UefiCpuPkg/Include/Register/Msr/NehalemMsr.h | 21 -
 UefiCpuPkg/Include/Register/Msr/P6Msr.h  | 23 ++-
 UefiCpuPkg/Include/Register/Msr/Pentium4Msr.h| 15 ++-
 UefiCpuPkg/Include/Register/Msr/PentiumMMsr.h| 18 +-
 UefiCpuPkg/Include/Register/Msr/PentiumMsr.h | 20 +++-
 UefiCpuPkg/Include/Register/Msr/SandyBridgeMsr.h | 19 ++-
 UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h  | 22 +-
 UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h | 19 ++-
 UefiCpuPkg/Include/Register/Msr/Xeon5600Msr.h| 19 ++-
 UefiCpuPkg/Include/Register/Msr/XeonDMsr.h   | 19 ++-
 UefiCpuPkg/Include/Register/Msr/XeonE7Msr.h  | 18 +-
 UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h | 18 +-
 20 files changed, 365 insertions(+), 20 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/AtomMsr.h 
b/UefiCpuPkg/Include/Register/Msr/AtomMsr.h
index c314195..b276469 100644
--- a/UefiCpuPkg/Include/Register/Msr/AtomMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/AtomMsr.h
@@ -6,7 +6,7 @@
   returned is a single 32-bit or 64-bit value, then a data structure is not
   provided for that MSR.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -27,6 +27,26 @@
 #include 
 
 /**
+  Is Intel(R) Atom(TM) Processor Family?
+
+  @param   DisplayFamily  Display Family ID
+  @param   DisplayModel   Display Model ID
+
+  @retval  TRUE   Yes, it is.
+  @retval  FALSE  No, it isn't.
+**/
+#define IS_ATOM_PROCESSOR(DisplayFamily, DisplayModel) \
+  (DisplayFamily == 0x06 && \
+   (\
+DisplayModel == 0x1C || \
+DisplayModel == 0x26 || \
+DisplayModel == 0x27 || \
+DisplayModel == 0x35 || \
+DisplayModel == 0x36\
+)   \
+   )
+
+/**
   Shared. Model Specific Platform ID (R).
 
   @param  ECX  MSR_ATOM_PLATFORM_ID (0x0017)
diff --git a/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h 
b/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h
index 1c3c2dc..90bd523 100644
--- a/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h
@@ -6,7 +6,7 @@
   returned is a single 32-bit or 64-bit value, then a data structure is not
   provided for that MSR.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -27,6 +27,25 @@
 #include 
 
 /**
+  Is Intel processors based on the Broadwell microarchitecture?
+
+  @param   DisplayFamily  Display Family ID
+  @param   DisplayModel   Display Model ID
+
+  @retval  TRUE   Yes, it is.
+  @retval  FALSE  No, it isn't.
+**/
+#define IS_BROADWELL_PROCESSOR(DisplayFamily, DisplayModel) \
+  (DisplayFamily == 0x06 && \
+   (\
+DisplayModel == 0x3D || \
+DisplayModel == 0x47 || \
+DisplayModel == 0x4F || \
+DisplayModel == 0x56\
+)   \
+   )
+
+/**
   Thread. See Table 35-2. See Section 18.4.2, "Global Counter Control
   Facilities.".
 
diff --git a/UefiCpuPkg/Include/Register/Msr/Core2Msr.h 
b/UefiCpuPkg/Include/Register/Msr/Core2Msr.h
index 9f0e790..9ebca5e 100644
--- a/UefiCpuPkg/Include/Register/Msr/Core2Msr.h
+++ b/UefiCpuPkg/Include/Register/Msr/Core2Msr.h
@@ -6,7 +6,7 @@
   returned is a single 32-bit or 64-bit value, 

[edk2] [PATCH 03/11] UefiCpuPkg/PiSmmCpuDxeSmm: Skip if AllocatedSize is 0

2017-03-09 Thread Jeff Fan
Needn't to copy register table if AllocatedSize is 0.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 12efc1f..f24d3d7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -826,18 +826,20 @@ CopyRegisterTable (
 
   CopyMem (DestinationRegisterTableList, SourceRegisterTableList, NumberOfCpus 
* sizeof (CPU_REGISTER_TABLE));
   for (Index = 0; Index < NumberOfCpus; Index++) {
-RegisterTableEntry = AllocatePool 
(DestinationRegisterTableList[Index].AllocatedSize);
-ASSERT (RegisterTableEntry != NULL);
-CopyMem (RegisterTableEntry, (VOID 
*)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry, 
DestinationRegisterTableList[Index].AllocatedSize);
-//
-// Go though all MSRs in register table to initialize MSR spin lock
-//
-for (Index1 = 0; Index1 < DestinationRegisterTableList[Index].TableLength; 
Index1++, RegisterTableEntry++) {
-  if ((RegisterTableEntry->RegisterType == Msr) && 
(RegisterTableEntry->ValidBitLength < 64)) {
-//
-// Initialize MSR spin lock only for those MSRs need bit field writing
-//
-InitMsrSpinLockByIndex (RegisterTableEntry->Index);
+if (DestinationRegisterTableList[Index].AllocatedSize != 0) {
+  RegisterTableEntry = AllocatePool 
(DestinationRegisterTableList[Index].AllocatedSize);
+  ASSERT (RegisterTableEntry != NULL);
+  CopyMem (RegisterTableEntry, (VOID 
*)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry, 
DestinationRegisterTableList[Index].AllocatedSize);
+  //
+  // Go though all MSRs in register table to initialize MSR spin lock
+  //
+  for (Index1 = 0; Index1 < 
DestinationRegisterTableList[Index].TableLength; Index1++, 
RegisterTableEntry++) {
+if ((RegisterTableEntry->RegisterType == Msr) && 
(RegisterTableEntry->ValidBitLength < 64)) {
+  //
+  // Initialize MSR spin lock only for those MSRs need bit field 
writing
+  //
+  InitMsrSpinLockByIndex (RegisterTableEntry->Index);
+}
   }
 }
 DestinationRegisterTableList[Index].RegisterTableEntry = 
(EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTableEntry;
-- 
2.9.3.windows.2

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


[edk2] [PATCH 10/11] UefiCpuPkg: Add NULL CPU Common Features Library instance

2017-03-09 Thread Jeff Fan
This NULL CPU common Features Library instance will register some CPU features
defined in Intel(R) 64 and IA-32 Architectures Software Developer's Manual,
Volume 3, September 2016, Chapter 35 Model-Specific-Registers (MSR).

Add PCD PcdCpuClockModulationDutyCycle and PcdIsPowerOnReset consumed by NULL
CPU Common Features Library instance.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c|  94 +++
 UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c  |  79 ++
 .../Library/CpuCommonFeaturesLib/ClockModulation.c | 106 +++
 .../CpuCommonFeaturesLib/CpuCommonFeatures.h   | 852 +
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.c| 227 ++
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf  |  68 ++
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.uni  |  25 +
 UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c |  81 ++
 .../Library/CpuCommonFeaturesLib/ExecuteDisable.c  |  91 +++
 .../Library/CpuCommonFeaturesLib/FastStrings.c |  52 ++
 .../Library/CpuCommonFeaturesLib/FeatureControl.c  | 315 
 .../CpuCommonFeaturesLib/LimitCpuIdMaxval.c|  82 ++
 .../Library/CpuCommonFeaturesLib/MachineCheck.c| 231 ++
 .../Library/CpuCommonFeaturesLib/MonitorMwait.c|  79 ++
 .../Library/CpuCommonFeaturesLib/PendingBreak.c|  90 +++
 UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c   |  81 ++
 UefiCpuPkg/UefiCpuPkg.dec  |  11 +
 17 files changed, 2564 insertions(+)
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/ClockModulation.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeatures.h
 create mode 100644 
UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c
 create mode 100644 
UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
 create mode 100644 
UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.uni
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/LimitCpuIdMaxval.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/MonitorMwait.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/PendingBreak.c
 create mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c 
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
new file mode 100644
index 000..6aebf0d
--- /dev/null
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
@@ -0,0 +1,94 @@
+/** @file
+  AESNI feature.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "CpuCommonFeatures.h"
+
+/**
+  Detects if AESNI feature supported on current processor.
+
+  @param[in]  ProcessorNumber  The index of the CPU executing this function.
+  @param[in]  CpuInfo  A pointer to the 
REGISTER_CPU_FEATURE_INFORMATION
+   structure for the CPU executing this function.
+  @param[in]  ConfigData   A pointer to the configuration buffer returned
+   by CPU_FEATURE_GET_CONFIG_DATA.  NULL if
+   CPU_FEATURE_GET_CONFIG_DATA was not provided in
+   RegisterCpuFeature().
+
+  @retval TRUE AESNI feature is supported.
+  @retval FALSEAESNI feature is not supported.
+
+  @note This service could be called by BSP/APs.
+**/
+BOOLEAN
+EFIAPI
+AesniSupport (
+  IN UINTN ProcessorNumber,
+  IN REGISTER_CPU_FEATURE_INFORMATION  *CpuInfo,
+  IN VOID  *ConfigData  OPTIONAL
+  )
+{
+  if (IS_SANDY_BRIDGE_PROCESSOR (CpuInfo->DisplayFamily, 
CpuInfo->DisplayModel) ||
+  IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) 
||
+  IS_XEON_5600_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
+  IS_XEON_PHI_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) {
+return 

[edk2] [PATCH 07/11] UefiCpuPkg: Add GUID gEdkiiCpuFeaturesInitDoneGuid

2017-03-09 Thread Jeff Fan
GUID gEdkiiCpuFeaturesInitDoneGuid is used to indicate if CPU features have been
initialized.

On PEI phase, one gEdkiiCpuFeaturesInitDoneGuid PPI will be installed after CPU
features initialized.
On DXE phase, one gEdkiiCpuFeaturesInitDoneGuid Protocol will be installed after
CPU features initialized.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Include/Guid/CpuFeaturesInitDone.h | 26 ++
 UefiCpuPkg/UefiCpuPkg.dec |  3 +++
 2 files changed, 29 insertions(+)
 create mode 100644 UefiCpuPkg/Include/Guid/CpuFeaturesInitDone.h

diff --git a/UefiCpuPkg/Include/Guid/CpuFeaturesInitDone.h 
b/UefiCpuPkg/Include/Guid/CpuFeaturesInitDone.h
new file mode 100644
index 000..ef17da5
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/CpuFeaturesInitDone.h
@@ -0,0 +1,26 @@
+/** @file
+  CPU Features Init Done PPI/Protocol should be installed after CPU features
+  are initialized.
+
+Copyright (c) 2016, Intel Corporation. All rights reserved.
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD 
License
+which accompanies this distribution.  The full text of the license may be 
found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef _CPU_FEATURES_INIT_DONE_H_
+#define _CPU_FEATURES_INIT_DONE_H_
+
+#define EDKII_CPU_FEATURES_INIT_DONE_GUID \
+  { \
+{ 0xc77c3a41, 0x61ab, 0x4143, { 0x98, 0x3e, 0x33, 0x39, 0x28, 0x6, 0x28, 
0xe5 } \
+  }
+
+extern EFI_GUID gEdkiiCpuFeaturesInitDoneGuid;
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 1443e82..f7abe1c 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -68,6 +68,9 @@
   ## Include/Guid/CpuFeaturesSetDone.h
   gEdkiiCpuFeaturesSetDoneGuid   = { 0xa82485ce, 0xad6b, 0x4101, { 0x99, 0xd3, 
0xe1, 0x35, 0x8c, 0x9e, 0x7e, 0x37 }}
 
+  ## Include/Guid/CpuFeaturesInitDone.h
+  gEdkiiCpuFeaturesInitDoneGuid  = { 0xc77c3a41, 0x61ab, 0x4143, { 0x98, 0x3e, 
0x33, 0x39, 0x28, 0x6, 0x28, 0xe5 }}
+
 [Protocols]
   ## Include/Protocol/SmmCpuService.h
   gEfiSmmCpuServiceProtocolGuid  = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 
0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
-- 
2.9.3.windows.2

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


[edk2] [PATCH 11/11] UefiCpuPkg: Add CPU Features PEI/DXE drivers

2017-03-09 Thread Jeff Fan
They will consume Register CPU Features library to detect and initialize CPU
features.

CpuFeaturesPei driver is used to initialize CPU features in PEI phase.
CpuFeaturesDxe driver is used to initialize CPU features in DXE phase.

Add PcdCpuFeaturesInitAfterSmmRelocation and PcdCpuFeaturesInitOnS3Resume
that consumed by CpuFeaturesPei and CpuFeaturesDxe.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c| 122 +
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf  |  53 +++
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.uni  |  22 +
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxeExtra.uni |  20 
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c|  75 +++
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf  |  49 ++
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.uni  |  22 +
 UefiCpuPkg/CpuFeatures/CpuFeaturesPeiExtra.uni |  20 
 UefiCpuPkg/UefiCpuPkg.dec  |   8 ++
 UefiCpuPkg/UefiCpuPkg.dsc  |   8 ++
 10 files changed, 399 insertions(+)
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.uni
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesDxeExtra.uni
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.uni
 create mode 100644 UefiCpuPkg/CpuFeatures/CpuFeaturesPeiExtra.uni

diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c 
b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c
new file mode 100644
index 000..f4f70cf
--- /dev/null
+++ b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c
@@ -0,0 +1,122 @@
+/** @file
+  CPU Features DXE driver to initialize CPU features.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+
+/**
+  Worker function to perform CPU feature initialization.
+
+**/
+VOID
+CpuFeaturesInitializeWorker (
+  VOID
+  )
+{
+  EFI_STATUS Status;
+  EFI_HANDLE Handle;
+
+  CpuFeaturesDetect ();
+
+  CpuFeaturesInitialize ();
+
+  //
+  // Install CPU Features Init Done Protocol
+  //
+  Handle = NULL;
+  Status = gBS->InstallProtocolInterface (
+  ,
+  ,
+  EFI_NATIVE_INTERFACE,
+  NULL
+  );
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  Event notification that initialize CPU features when 
gEfiSmmConfigurationProtocol installs.
+
+  @param[in]  Event The Event that is being processed, not 
used.
+  @param[in]  Context   Event Context, not used.
+**/
+VOID
+EFIAPI
+SmmConfigurationEventNotify (
+  IN EFI_EVENT Event,
+  IN VOID  *Context
+  )
+{
+  EFI_STATUS   Status;
+  EFI_SMM_CONFIGURATION_PROTOCOL   *SmmConfiguration;
+
+  //
+  // Make sure this notification is for this handler
+  //
+  Status = gBS->LocateProtocol (, NULL, (VOID 
**));
+  if (EFI_ERROR (Status)) {
+return;
+  }
+
+  CpuFeaturesInitializeWorker ();
+}
+
+/**
+  CPU Features driver entry point function.
+
+  If PcdCpuFeaturesInitAfterSmmRelocation is TRUE, it will register one
+  SMM Configuration Protocol notify function to perform CPU features
+  initialization. Otherwise, it will perform CPU features initialization
+  directly.
+
+  @param ImageHandle Image handle this driver.
+  @param SystemTable Pointer to the System Table.
+
+  @retval EFI_SUCCESS   CPU Features is initialized successfully.
+**/
+EFI_STATUS
+EFIAPI
+CpuFeaturesDxeInitialize (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  VOID*Registration;
+
+  if (PcdGetBool (PcdCpuFeaturesInitAfterSmmRelocation)) {
+//
+// Install notification callback on SMM Configuration Protocol
+//
+EfiCreateProtocolNotifyEvent (
+  ,
+  TPL_CALLBACK,
+  SmmConfigurationEventNotify,
+  NULL,
+  
+  );
+  } else {
+CpuFeaturesInitializeWorker ();
+  }
+
+  return EFI_SUCCESS;
+}
+
diff --git a/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf 
b/UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf
new file mode 

[edk2] [PATCH 08/11] UefiCpuPkg/Include/Library: Add Register CPU Features Library

2017-03-09 Thread Jeff Fan
Register CPU Features Library is used to register/manage/program CPU features.
NULL CPU features library instance could consume it register CPU features
functions.
CPU Feature module could consume this library to detect/analysis/program CPU
features on BSP/APs.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 .../Include/Library/RegisterCpuFeaturesLib.h   | 516 +
 UefiCpuPkg/UefiCpuPkg.dec  |   5 +
 2 files changed, 521 insertions(+)
 create mode 100644 UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
new file mode 100644
index 000..5b974e7
--- /dev/null
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -0,0 +1,516 @@
+/** @file
+  Register CPU Features Library to register and manage CPU features.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __REGISTER_CPU_FEATURES_LIB_H__
+#define __REGISTER_CPU_FEATURES_LIB_H__
+
+#include 
+#include 
+#include 
+
+///
+/// Defines used to identify a CPU feature.  The lower 16-bits are used to
+/// identify a unique CPU feature and the value represents a bit number in
+/// a bit mask.  The upper 16-bits are bit mask values that are used as
+/// modifiers of a CPU feature.  When used in a list, the define value
+/// CPU_FEATURE_END is used to terminate a list of CPU feature values.
+/// @{
+#define CPU_FEATURE_AESNI   0
+#define CPU_FEATURE_TURBO_MODE  1
+#define CPU_FEATURE_MWAIT   2
+#define CPU_FEATURE_ACPI3
+#define CPU_FEATURE_EIST4
+#define CPU_FEATURE_XD  5
+#define CPU_FEATURE_FASTSTRINGS 6
+#define CPU_FEATURE_VMX 7
+#define CPU_FEATURE_SMX 8
+#define CPU_FEATURE_SENTER  9
+#define CPU_FEATURE_LOCK_FEATURE_CONTROL_REGISTER   10
+#define CPU_FEATURE_LIMIT_CPUID_MAX_VAL 11
+#define CPU_FEATURE_MCE 12
+#define CPU_FEATURE_MCA 13
+#define CPU_FEATURE_MCG_CTL 14
+#define CPU_FEATURE_PENDING_BREAK   15
+#define CPU_FEATURE_C1E 16
+#define CPU_FEATURE_C1_AUTO_DEMOTION17
+#define CPU_FEATURE_C3_AUTO_DEMOTION18
+#define CPU_FEATURE_C1_AUTO_UNDEMOTION  19
+#define CPU_FEATURE_C3_AUTO_UNDEMOTION  20
+#define CPU_FEATURE_C_STATE 21
+#define CPU_FEATURE_TM  22
+#define CPU_FEATURE_TM2 23
+#define CPU_FEATURE_X2APIC  24
+#define CPU_FEATURE_RESERVED_25 25
+#define CPU_FEATURE_RESERVED_26 26
+#define CPU_FEATURE_RESERVED_27 27
+#define CPU_FEATURE_RESERVED_28 28
+#define CPU_FEATURE_RESERVED_29 29
+#define CPU_FEATURE_RESERVED_30 30
+#define CPU_FEATURE_RESERVED_31 31
+
+#define CPU_FEATURE_L2_PREFETCHER   (32+0)
+#define CPU_FEATURE_L1_DATA_PREFETCHER  (32+1)
+#define CPU_FEATURE_HARDWARE_PREFETCHER (32+2)
+#define CPU_FEATURE_ADJACENT_CACHE_LINE_PREFETCH(32+3)
+#define CPU_FEATURE_DCU_PREFETCHER  (32+4)
+#define CPU_FEATURE_IP_PREFETCHER   (32+5)
+#define CPU_FEATURE_MLC_STREAMER_PREFETCHER (32+6)
+#define CPU_FEATURE_MLC_SPATIAL_PREFETCHER  (32+7)
+#define CPU_FEATURE_THREE_STRICK_COUNTER(32+8)
+#define CPU_FEATURE_APIC_TPR_UPDATE_MESSAGE (32+9)
+#define CPU_FEATURE_ENERGY_PERFORMANCE_BIAS (32+10)
+
+#define CPU_FEATURE_BEFORE_ALL  BIT27
+#define CPU_FEATURE_AFTER_ALL   BIT28
+#define CPU_FEATURE_BEFORE  BIT29
+#define CPU_FEATURE_AFTER   BIT30
+#define CPU_FEATURE_END MAX_UINT32
+/// @}
+
+///
+/// CPU Information passed into the SupportFunc and InitializeFunc of the
+/// RegisterCpuFeature() library function.  This structure contains information
+/// that is commonly used during CPU 

[edk2] [PATCH 09/11] UefiCpuPkg: Add PEI/DXE Register CPU Features Library instances

2017-03-09 Thread Jeff Fan
PEI Register CPU Features Library instance is used to register/manager/program
CPU features on PEI phase.
DXE Register CPU Features Library instance is used to register/manager/program
CPU features on DXE phase.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 700 +++
 .../DxeRegisterCpuFeaturesLib.c| 266 +++
 .../DxeRegisterCpuFeaturesLib.inf  |  62 ++
 .../PeiRegisterCpuFeaturesLib.c| 390 +++
 .../PeiRegisterCpuFeaturesLib.inf  |  64 ++
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   | 193 ++
 .../RegisterCpuFeaturesDxe.uni |  22 +
 .../RegisterCpuFeaturesLib.c   | 770 +
 UefiCpuPkg/UefiCpuPkg.dsc  |   6 +-
 9 files changed, 2472 insertions(+), 1 deletion(-)
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.inf
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesDxe.uni
 create mode 100644 
UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
new file mode 100644
index 000..30dc0b8
--- /dev/null
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -0,0 +1,700 @@
+/** @file
+  CPU Features Initialize functions.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "RegisterCpuFeatures.h"
+
+/**
+  Worker function to save PcdCpuFeaturesCapability. 
+  
+  @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
+*/
+VOID
+SetCapabilityPcd (
+  IN UINT8   *SupportedFeatureMask
+  )
+{ 
+  EFI_STATUS Status;
+  UINTN  BitMaskSize;
+  
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
+  Status = PcdSetPtrS (PcdCpuFeaturesCapability, , 
SupportedFeatureMask);
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  Worker function to save PcdCpuFeaturesSetting. 
+  
+  @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask buffer
+**/
+VOID
+SetSettingPcd (
+  IN UINT8   *SupportedFeatureMask
+  )
+{ 
+  EFI_STATUS Status;
+  UINTN  BitMaskSize;
+  
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
+  Status = PcdSetPtrS (PcdCpuFeaturesSetting, , 
SupportedFeatureMask);
+  ASSERT_EFI_ERROR (Status);
+}
+
+/**
+  Worker function to get PcdCpuFeaturesSupport. 
+  
+  @return  The pointer to CPU feature bits mask buffer.
+**/
+UINT8 *
+GetSupportPcds (
+  VOID
+  )
+{
+  UINTN  BitMaskSize;
+  UINT8  *SupportBitMask;
+  
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
+  SupportBitMask = AllocateZeroPool (BitMaskSize);
+  SupportBitMask = (UINT8 *) PcdGetPtr (PcdCpuFeaturesSupport);
+
+  return SupportBitMask;
+}
+
+/**
+  Worker function to get PcdCpuFeaturesUserConfiguration. 
+  
+  @return  The pointer to CPU feature bits mask buffer.
+**/
+UINT8 *
+GetConfigurationPcds (
+  VOID
+  )
+{
+  UINTN  BitMaskSize;
+  UINT8  *SupportBitMask;
+  
+  BitMaskSize = PcdGetSize (PcdCpuFeaturesUserConfiguration);
+  SupportBitMask = AllocateZeroPool (BitMaskSize);
+  SupportBitMask = (UINT8 *) PcdGetPtr (PcdCpuFeaturesUserConfiguration);
+
+  return SupportBitMask;
+}
+
+/**
+  Collects CPU type and feature information.
+
+  @param[in, out]  CpuInfo  The pointer to CPU feature information
+**/
+VOID
+FillProcessorInfo (
+  IN OUT REGISTER_CPU_FEATURE_INFORMATION*CpuInfo
+  )
+{
+  CPUID_VERSION_INFO_EAX Eax;
+  CPUID_VERSION_INFO_ECX Ecx;
+  CPUID_VERSION_INFO_EDX Edx;
+  UINT32 DisplayedFamily;
+  UINT32 DisplayedModel;
+
+  AsmCpuid (CPUID_VERSION_INFO, , NULL, , );
+
+  

[edk2] [PATCH 06/11] UefiCpuPkg: Add GUID gEdkiiCpuFeaturesSetDoneGuid

2017-03-09 Thread Jeff Fan
GUID gEdkiiCpuFeaturesSetDoneGuid is used to indicate if CPU feature related
setting are set finished. For example, PCD PcdCpuFeaturesUserConfiguration.

On PEI phase, one gEdkiiCpuFeaturesSetDoneGuid PPI will be installed after
platform set CPU feature setting.
On DXE phase, one gEdkiiCpuFeaturesSetDoneGuid Protocol will be installed after
platform set CPU feature setting.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Include/Guid/CpuFeaturesSetDone.h | 26 ++
 UefiCpuPkg/UefiCpuPkg.dec|  3 +++
 2 files changed, 29 insertions(+)
 create mode 100644 UefiCpuPkg/Include/Guid/CpuFeaturesSetDone.h

diff --git a/UefiCpuPkg/Include/Guid/CpuFeaturesSetDone.h 
b/UefiCpuPkg/Include/Guid/CpuFeaturesSetDone.h
new file mode 100644
index 000..8b1592e
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/CpuFeaturesSetDone.h
@@ -0,0 +1,26 @@
+/** @file
+  CPU Features Set Done PPI/Protocol should be installed after CPU features
+  configuration are set.
+
+Copyright (c) 2016, Intel Corporation. All rights reserved.
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD 
License
+which accompanies this distribution.  The full text of the license may be 
found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef _CPU_FEATURES_INIT_DONE_H_
+#define _CPU_FEATURES_INIT_DONE_H_
+
+#define EDKII_CPU_FEATURES_SET_DONE_GUID \
+  { \
+{ 0xa82485ce, 0xad6b, 0x4101, { 0x99, 0xd3, 0xe1, 0x35, 0x8c, 0x9e, 0x7e, 
0x37 } \
+  }
+
+extern EFI_GUID gEdkiiCpuFeaturesSetDoneGuid;
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index c4b43e2..1443e82 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -65,6 +65,9 @@
   ## Include/Guid/MicrocodeFmp.h
   gMicrocodeFmpImageTypeIdGuid  = { 0x96d4fdcd, 0x1502, 0x424d, { 0x9d, 
0x4c, 0x9b, 0x12, 0xd2, 0xdc, 0xae, 0x5c } }
 
+  ## Include/Guid/CpuFeaturesSetDone.h
+  gEdkiiCpuFeaturesSetDoneGuid   = { 0xa82485ce, 0xad6b, 0x4101, { 0x99, 0xd3, 
0xe1, 0x35, 0x8c, 0x9e, 0x7e, 0x37 }}
+
 [Protocols]
   ## Include/Protocol/SmmCpuService.h
   gEfiSmmCpuServiceProtocolGuid  = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 
0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
-- 
2.9.3.windows.2

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


[edk2] [PATCH 01/11] UefiCpuPkg/AcpiCpuData: Update RegisterTableEntry type

2017-03-09 Thread Jeff Fan
Current RegisterTableEntry filed in CPU_REGISTER_TABLE is one pointer to
CPU_REGISTER_TABLE_ENTRY. If CPU register table wants to be passed from 32bit
PEI to x64 DXE/SMM, x64 DXE/SMM cannot get the correct RegisterTableEntry.

This update is to update RegisterTableEntry type to EFI_PHYSICAL_ADDRESS and
make RegisterTableEntry is fixed length.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c |  6 +++---
 UefiCpuPkg/Include/AcpiCpuData.h|  6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c   | 10 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c 
b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
index 3489b95..07c7102 100644
--- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
+++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
@@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, so this 
module does not
 support hot plug CPUs.  This module can be copied into a CPU specific package
 and customized if these additional features are required.
 
-Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.
 Copyright (c) 2015, Red Hat, Inc.
 
 This program and the accompanying materials
@@ -246,12 +246,12 @@ CpuS3DataInitialize (
 RegisterTable[Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
 RegisterTable[Index].TableLength= 0;
 RegisterTable[Index].AllocatedSize  = 0;
-RegisterTable[Index].RegisterTableEntry = NULL;
+RegisterTable[Index].RegisterTableEntry = 0;
 
 RegisterTable[NumberOfCpus + Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
 RegisterTable[NumberOfCpus + Index].TableLength= 0;
 RegisterTable[NumberOfCpus + Index].AllocatedSize  = 0;
-RegisterTable[NumberOfCpus + Index].RegisterTableEntry = NULL;
+RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
   }
   AcpiCpuData->RegisterTable   = 
(EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
   AcpiCpuData->PreSmmInitRegisterTable = 
(EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h
index 12e9692..130eb90 100644
--- a/UefiCpuPkg/Include/AcpiCpuData.h
+++ b/UefiCpuPkg/Include/AcpiCpuData.h
@@ -1,7 +1,7 @@
 /** @file
 Definitions for CPU S3 data.
 
-Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -55,10 +55,10 @@ typedef struct {
   //
   UINT32InitialApicId;
   //
-  // Buffer of CPU_REGISTER_TABLE_ENTRY structures.  This buffer must be
+  // Physical address of CPU_REGISTER_TABLE_ENTRY structures.  This buffer 
must be
   // allocated below 4GB from memory of type EfiACPIMemoryNVS.
   //
-  CPU_REGISTER_TABLE_ENTRY  *RegisterTableEntry;
+  EFI_PHYSICAL_ADDRESS  RegisterTableEntry;
 } CPU_REGISTER_TABLE;
 
 //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 532b7c4..12efc1f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -1,7 +1,7 @@
 /** @file
 Code for Processor S3 restoration
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -826,13 +826,12 @@ CopyRegisterTable (
 
   CopyMem (DestinationRegisterTableList, SourceRegisterTableList, NumberOfCpus 
* sizeof (CPU_REGISTER_TABLE));
   for (Index = 0; Index < NumberOfCpus; Index++) {
-DestinationRegisterTableList[Index].RegisterTableEntry = AllocatePool 
(DestinationRegisterTableList[Index].AllocatedSize);
-ASSERT (DestinationRegisterTableList[Index].RegisterTableEntry != NULL);
-CopyMem (DestinationRegisterTableList[Index].RegisterTableEntry, 
SourceRegisterTableList[Index].RegisterTableEntry, 
DestinationRegisterTableList[Index].AllocatedSize);
+RegisterTableEntry = AllocatePool 
(DestinationRegisterTableList[Index].AllocatedSize);
+ASSERT (RegisterTableEntry != NULL);
+CopyMem (RegisterTableEntry, (VOID 
*)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry, 
DestinationRegisterTableList[Index].AllocatedSize);
 //
 // Go though all MSRs in register table to initialize MSR spin lock
 //
-RegisterTableEntry = 

[edk2] [PATCH 05/11] UefiCpuPkg/UefiCpuPkg.dec: Add a set of CPU features PCDs

2017-03-09 Thread Jeff Fan
PcdCpuFeaturesSupport supports PcdsFixedAtBuild/PcdsPatchableInModule types and
used to add/remove CPU feature from firmware during build time.

PcdCpuFeaturesUserConfiguration supports all PCD types and used to configurate
CPU features by platforms.

PcdCpuFeaturesCapability supports PcdsDynamic PCD and used to indicate the CPU
features capability on processors.

PcdCpuFeaturesSetting supports PcdsDynamic PCD and used to indicate the current
CPU features setting on processors.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/UefiCpuPkg.dec | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index c37b4d5..c4b43e2 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -1,7 +1,7 @@
 ## @file  UefiCpuPkg.dec
 # This Package provides UEFI compatible CPU modules and libraries.
 #
-# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License which accompanies this 
distribution.
@@ -181,6 +181,10 @@
   # @Prompt MSEG size.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize|0x20|UINT32|0x32132112
 
+  ## Specifies the supported CPU features bit in array
+  # @Prompt Supported CPU features
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSupport|{0xFF, 0xFF, 0xFF, 0xFF, 
0xFF, 0xFF, 0xFF, 0xFF}|VOID*|0x0016
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Specifies max supported number of Logical Processors.
   # @Prompt Configure max supported number of Logical Processors
@@ -226,6 +230,10 @@
   # @Prompt SMM CPU Synchronization Method.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x00|UINT8|0x6014
 
+  ## Specifies user's desired settings for enabling/disabling processor 
features.
+  # @Prompt User settings for enabling/disabling processor features.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesUserConfiguration|{0x00, 0x00, 0x00, 
0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x0017
+
 [PcdsDynamic, PcdsDynamicEx]
   ## Contains the pointer to a CPU S3 data buffer of structure ACPI_CPU_DATA.
   # @Prompt The pointer to a CPU S3 data buffer.
@@ -237,5 +245,15 @@
   # @ValidList   0x8001 | 0
   gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress|0x0|UINT64|0x6011
 
+  ## Indicates processor feature capabilities, each bit corresponding to a 
specific feature.
+  # @Prompt Processor feature capabilities.
+  # @ValidList   0x8001 | 0
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesCapability|{0x00, 0x00, 0x00, 0x00, 
0x00, 0x00, 0x00, 0x00}|VOID*|0x0018
+
+  ## Specifies actual settings for processor features, each bit corresponding 
to a specific feature.
+  # @Prompt Actual processor feature settings.
+  # @ValidList   0x8001 | 0
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting|{0x00, 0x00, 0x00, 0x00, 
0x00, 0x00, 0x00, 0x00}|VOID*|0x0019
+
 [UserExtensions.TianoCore."ExtraFiles"]
   UefiCpuPkgExtra.uni
-- 
2.9.3.windows.2

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


[edk2] [PATCH 00/11] Add CPU features driver

2017-03-09 Thread Jeff Fan
This serial of patches will add CPU featuers initialization on boot time.

1) One new Register CPU Features Library and instances are added to provide the
   capability to register CPU feature's detect/initialize services.
2) One new NULL class CPU Commong Features Library instance is added to provide
   detect/initialize servcies of CPU features defined in SDM.
3) New CPU features PEI/DXE drivers are added to initialize CPU features in PEI
   phase or DXE phase, by consuming Register CPU Features Library.

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

The whole updated UefiCpuPkg could be accessed at
https://github.com/JeffFan/edk2/tree/CpuFeatures/UefiCpuPkg for reveiw.

Cc: Feng Tian 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 

Jeff Fan (11):
  UefiCpuPkg/AcpiCpuData: Update RegisterTableEntry type
  UefiCpuPkg/CpuS3DataDxe: Consume the existing PcdCpuS3DataAddress
  UefiCpuPkg/PiSmmCpuDxeSmm: Skip if AllocatedSize is 0
  UefiCpuPkg/Msr: Add CPUID signature check MACROs
  UefiCpuPkg/UefiCpuPkg.dec: Add a set of CPU features PCDs
  UefiCpuPkg: Add GUID gEdkiiCpuFeaturesSetDoneGuid
  UefiCpuPkg: Add GUID gEdkiiCpuFeaturesInitDoneGuid
  UefiCpuPkg/Include/Library: Add Register CPU Features Library
  UefiCpuPkg: Add PEI/DXE Register CPU Features Library instances
  UefiCpuPkg: Add NULL CPU Common Features Library instance
  UefiCpuPkg: Add CPU Features PEI/DXE drivers

 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.c| 122 +++
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.inf  |  53 ++
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxe.uni  |  22 +
 UefiCpuPkg/CpuFeatures/CpuFeaturesDxeExtra.uni |  20 +
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.c|  75 ++
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.inf  |  49 ++
 UefiCpuPkg/CpuFeatures/CpuFeaturesPei.uni  |  22 +
 UefiCpuPkg/CpuFeatures/CpuFeaturesPeiExtra.uni |  20 +
 UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c|  56 +-
 UefiCpuPkg/Include/AcpiCpuData.h   |   6 +-
 UefiCpuPkg/Include/Guid/CpuFeaturesInitDone.h  |  26 +
 UefiCpuPkg/Include/Guid/CpuFeaturesSetDone.h   |  26 +
 .../Include/Library/RegisterCpuFeaturesLib.h   | 516 +
 UefiCpuPkg/Include/Register/Msr/AtomMsr.h  |  22 +-
 UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h |  21 +-
 UefiCpuPkg/Include/Register/Msr/Core2Msr.h |  19 +-
 UefiCpuPkg/Include/Register/Msr/CoreMsr.h  |  18 +-
 UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h  |  18 +-
 UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h  |  18 +-
 UefiCpuPkg/Include/Register/Msr/HaswellMsr.h   |  20 +-
 UefiCpuPkg/Include/Register/Msr/IvyBridgeMsr.h |  18 +-
 UefiCpuPkg/Include/Register/Msr/NehalemMsr.h   |  21 +-
 UefiCpuPkg/Include/Register/Msr/P6Msr.h|  23 +-
 UefiCpuPkg/Include/Register/Msr/Pentium4Msr.h  |  15 +-
 UefiCpuPkg/Include/Register/Msr/PentiumMMsr.h  |  18 +-
 UefiCpuPkg/Include/Register/Msr/PentiumMsr.h   |  20 +-
 UefiCpuPkg/Include/Register/Msr/SandyBridgeMsr.h   |  19 +-
 UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h|  22 +-
 UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h   |  19 +-
 UefiCpuPkg/Include/Register/Msr/Xeon5600Msr.h  |  19 +-
 UefiCpuPkg/Include/Register/Msr/XeonDMsr.h |  19 +-
 UefiCpuPkg/Include/Register/Msr/XeonE7Msr.h|  18 +-
 UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h   |  18 +-
 UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c|  94 +++
 UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c  |  79 ++
 .../Library/CpuCommonFeaturesLib/ClockModulation.c | 106 +++
 .../CpuCommonFeaturesLib/CpuCommonFeatures.h   | 852 +
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.c| 227 ++
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf  |  68 ++
 .../CpuCommonFeaturesLib/CpuCommonFeaturesLib.uni  |  25 +
 UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c |  81 ++
 .../Library/CpuCommonFeaturesLib/ExecuteDisable.c  |  91 +++
 .../Library/CpuCommonFeaturesLib/FastStrings.c |  52 ++
 .../Library/CpuCommonFeaturesLib/FeatureControl.c  | 315 
 .../CpuCommonFeaturesLib/LimitCpuIdMaxval.c|  82 ++
 .../Library/CpuCommonFeaturesLib/MachineCheck.c| 231 ++
 .../Library/CpuCommonFeaturesLib/MonitorMwait.c|  79 ++
 .../Library/CpuCommonFeaturesLib/PendingBreak.c|  90 +++
 UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c   |  81 ++
 .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 700 +
 .../DxeRegisterCpuFeaturesLib.c| 266 +++
 .../DxeRegisterCpuFeaturesLib.inf  |  62 ++
 .../PeiRegisterCpuFeaturesLib.c| 390 ++
 .../PeiRegisterCpuFeaturesLib.inf  |  64 ++
 .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   | 193 +
 .../RegisterCpuFeaturesDxe.uni

[edk2] [patch] NetworkPkg: Fix potential bug if the iSCSI use dns protocol.

2017-03-09 Thread Zhang Lubo
Since we use the Attempt and index as the attempt variable name instead of
the MAC address plus index, we need to update this to check the whether
the Controller handle is configured to use DNS protocol

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 NetworkPkg/IScsiDxe/IScsiMisc.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index 7b4044f..2c93590 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -1776,11 +1776,11 @@ IScsiDhcpIsConfigured (
   FreePool (AttemptConfigOrder);
   return FALSE;
 }
 
 /**
-  Check wheather the Controller handle is configured to use DNS protocol.
+  Check whether the Controller handle is configured to use DNS protocol.
 
   @param[in]  Controller   The handle of the controller.
   
   @retval TRUE The handle of the controller need the Dns 
protocol.
   @retval FALSEThe handle of the controller does not need 
the Dns protocol.
@@ -1797,10 +1797,11 @@ IScsiDnsIsConfigured (
   UINTN   Index;
   EFI_STATUS  Status;
   EFI_MAC_ADDRESS MacAddr;
   UINTN   HwAddressSize;
   UINT16  VlanId;
+  CHAR16  AttemptMacString[ISCSI_MAX_MAC_STRING_LEN];
   CHAR16  MacString[ISCSI_MAX_MAC_STRING_LEN];
   CHAR16  AttemptName[ISCSI_NAME_IFR_MAX_SIZE];
   
   AttemptConfigOrder = IScsiGetVariableAndSize (
  L"AttemptOrder",
@@ -1826,14 +1827,14 @@ IScsiDnsIsConfigured (
   
   for (Index = 0; Index < AttemptConfigOrderSize / sizeof (UINT8); Index++) {
 UnicodeSPrint (
   AttemptName,
   (UINTN) 128,
-  L"%s%d",
-  MacString,
+  L"Attempt %d",
   (UINTN) AttemptConfigOrder[Index]
   );
+
 Status = GetVariable2 (
AttemptName,
,
(VOID**),
NULL
@@ -1842,11 +1843,13 @@ IScsiDnsIsConfigured (
   continue;
 }
 
 ASSERT (AttemptConfigOrder[Index] == AttemptTmp->AttemptConfigIndex);
 
-if (AttemptTmp->SessionConfigData.Enabled == ISCSI_DISABLED) {
+AsciiStrToUnicodeStrS (AttemptTmp->MacString, AttemptMacString, sizeof 
(AttemptMacString) / sizeof (AttemptMacString[0]));
+
+if (AttemptTmp->SessionConfigData.Enabled == ISCSI_DISABLED || StrCmp 
(MacString, AttemptMacString)) {
   FreePool (AttemptTmp);
   continue;
 }
 
 if (AttemptTmp->SessionConfigData.DnsMode) {
-- 
1.9.5.msysgit.1

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


Re: [edk2] [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

2017-03-09 Thread Ard Biesheuvel
On 8 March 2017 at 20:05, Laszlo Ersek  wrote:
> This series replaces the PURE_ACPI_BOOT_ENABLE build option with dynamic
> behavior, matching QEMU's (inverse sense) "-no-acpi" switch. In
> particular, DT and ACPI are no longer exposed to the guest at the same
> time. (DT is exposed with "-no-acpi", or else ACPI is exposed without
> "-no-acpi".)
>
> Repo:   https://github.com/lersek/edk2.git
> Branch: dynamic_pure_acpi
> RHBZ:   https://bugzilla.redhat.com/show_bug.cgi?id=1430262
>
> Tested with RHEL-7.3 for ARM and Fedora 24 guests (DT vs. ACPI).
>
> Cc: Ard Biesheuvel 
>

Hi Laszlo,

This looks complicated to me. Given that it is arguably a policy to
only expose on h/w description or the other, couldn't we simply remove
the FDT config table in BDS if an ACPI/ACPI2.0 config table is
present?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel