Re: [edk2] [PATCH v2] MdeModulePkg/MemoryStatusCode: Expose the DXE memory status code table.

2016-06-27 Thread Gao, Liming
Cinnamon:
  I have two minor comments on usage. Install System table means PRODUCES. 
  Others are good to me.  Reviewed-by: Liming Gao 

1. StatusCodeHandlerRuntimeDxe.inf
   gMemoryStatusCodeRecordGuid   ## SOMETIMES_CONSUMES   ## HOB 
and ConfigurationTable
==> 
   ## SOMETIMES_CONSUMES   ## HOB
   ## PRODUCES ## SystemTable   
gMemoryStatusCodeRecordGuid

2. StatusCodeHandlerSmm.inf
gMemoryStatusCodeRecordGuid   ## SOMETIMES_CONSUMES   ## 
SmmConfigurationTable
==>
gMemoryStatusCodeRecordGuid   ## PRODUCES   ## UNDEFINED  # 
SmmConfigurationTable

Thanks
Liming
> -Original Message-
> From: Cinnamon Shia [mailto:cinnamon.s...@hpe.com]
> Sent: Monday, June 27, 2016 3:26 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Tian, Feng ;
> Zeng, Star ; Cinnamon Shia 
> Subject: [PATCH v2] MdeModulePkg/MemoryStatusCode: Expose the DXE
> memory status code table.
> 
> Let data of DXE memory status code can be used by other modules.
> 1. Save the address of DXE memory status code table to
> DxeConfigurationTable.
> 2. Save the address of SMM memory status code table to
> SmmConfigurationTable.
> 3. Move RUNTIME_MEMORY_STATUSCODE_HEADER to its public header file.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cinnamon Shia 
> ---
>  MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h | 30
> ++
>  .../RuntimeDxe/MemoryStatusCodeWorker.c| 27 +++---
> -
>  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h   | 17 
>  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf |  3 ++-
>  .../StatusCodeHandler/Smm/MemoryStatusCodeWorker.c | 21
> ++-
>  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.h   | 18 -
>  .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf |  4 ++-
>  7 files changed, 71 insertions(+), 49 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> index d6c3243..27d39e1 100644
> --- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> +++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> @@ -2,13 +2,14 @@
>GUID used to identify status code records HOB that originate from the PEI
> status code.
> 
>  Copyright (c) 2006 - 2010, 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.
> +(C) Copyright 2016 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 that accompanies this
> distribution.
>  The full text of the license may be found at
> -http://opensource.org/licenses/bsd-license.php.
> +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.
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
>  **/
> 
> @@ -56,6 +57,25 @@ typedef struct {
>  } MEMORY_STATUSCODE_PACKET_HEADER;
> 
>  ///
> +/// A header structure that is followed by an array of records that contain
> the
> +/// parameters passed into the ReportStatusCode() service in the DXE
> Services Table.
> +///
> +typedef struct {
> +  ///
> +  /// The index pointing to the last recored being stored.
> +  ///
> +  UINT32   RecordIndex;
> +  ///
> +  /// The number of records being stored.
> +  ///
> +  UINT32   NumberOfRecords;
> +  ///
> +  /// The maximum number of records that can be stored.
> +  ///
> +  UINT32   MaxRecordsNumber;
> +} RUNTIME_MEMORY_STATUSCODE_HEADER;
> +
> +///
>  /// A structure that contains the parameters passed into the
> ReportStatusCode()
>  /// service in the PEI Services Table.
>  ///
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStat
> usCodeWorker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStat
> usCodeWorker.c
> index 8680497..a306d75 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStat
> usCodeWorker.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStat
> usCodeWorker.c
> @@ -2,13 +2,14 @@
>Runtime memory status code worker.
> 
>Copyright (c) 2006 - 2009, 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 

Re: [edk2] SecurityPkg/Tcg: Fix bug that prevented SubmitCommand buffers from being Max size.

2016-06-27 Thread Yao, Jiewen
Good catch.

Reviewed-by: jiewen@intel.com



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Bret Barkelew
> Sent: Tuesday, June 28, 2016 10:29 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] SecurityPkg/Tcg: Fix bug that prevented SubmitCommand
> buffers from being Max size.
> 
> SecurityPkg/Tcg: Fix bug that prevented SubmitCommand buffers from being
> Max size.
> 
> SubmitCommand() was checking the buffer size for ">=" Max size. This would
> cause code to fail with "EFI_INVALID_PARAMETER" if a buffer was passed
> that was the "max" size as indicated by the GetCapability() command.
> Change to ">" to allow for maximum buffer size.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Bret Barkelew
> >
> 
> ---
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 4 ++--
> SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index bdff5bd..7720c27 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -1330,10 +1330,10 @@ Tcg2SubmitCommand (
>  return EFI_DEVICE_ERROR;
>}
> 
> -  if (InputParameterBlockSize >= mTcgDxeData.BsCap.MaxCommandSize) {
> +  if (InputParameterBlockSize > mTcgDxeData.BsCap.MaxCommandSize) {
>  return EFI_INVALID_PARAMETER;
>}
> -  if (OutputParameterBlockSize >= mTcgDxeData.BsCap.MaxResponseSize)
> {
> +  if (OutputParameterBlockSize > mTcgDxeData.BsCap.MaxResponseSize) {
>  return EFI_INVALID_PARAMETER;
>}
> 
> diff --git a/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
> b/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
> index dfdee04..a30cd51 100644
> --- a/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
> +++ b/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
> @@ -893,10 +893,10 @@ TreeSubmitCommand (
>  return EFI_UNSUPPORTED;
>}
> 
> -  if (InputParameterBlockSize >= mTcgDxeData.BsCap.MaxCommandSize) {
> +  if (InputParameterBlockSize > mTcgDxeData.BsCap.MaxCommandSize) {
>  return EFI_INVALID_PARAMETER;
>}
> -  if (OutputParameterBlockSize >= mTcgDxeData.BsCap.MaxResponseSize)
> {
> +  if (OutputParameterBlockSize > mTcgDxeData.BsCap.MaxResponseSize) {
>  return EFI_INVALID_PARAMETER;
>}
> 
> --
> 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] SecurityPkg/Tcg: Fix bug that prevented SubmitCommand buffers from being Max size.

2016-06-27 Thread Bret Barkelew
SecurityPkg/Tcg: Fix bug that prevented SubmitCommand buffers from being Max 
size.

SubmitCommand() was checking the buffer size for ">=" Max size. This would 
cause code to fail with "EFI_INVALID_PARAMETER" if a buffer was passed that was 
the "max" size as indicated by the GetCapability() command. Change to ">" to 
allow for maximum buffer size.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Bret Barkelew 
>

---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 4 ++--
SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c 
b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index bdff5bd..7720c27 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -1330,10 +1330,10 @@ Tcg2SubmitCommand (
 return EFI_DEVICE_ERROR;
   }

-  if (InputParameterBlockSize >= mTcgDxeData.BsCap.MaxCommandSize) {
+  if (InputParameterBlockSize > mTcgDxeData.BsCap.MaxCommandSize) {
 return EFI_INVALID_PARAMETER;
   }
-  if (OutputParameterBlockSize >= mTcgDxeData.BsCap.MaxResponseSize) {
+  if (OutputParameterBlockSize > mTcgDxeData.BsCap.MaxResponseSize) {
 return EFI_INVALID_PARAMETER;
   }

diff --git a/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c 
b/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
index dfdee04..a30cd51 100644
--- a/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
+++ b/SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c
@@ -893,10 +893,10 @@ TreeSubmitCommand (
 return EFI_UNSUPPORTED;
   }

-  if (InputParameterBlockSize >= mTcgDxeData.BsCap.MaxCommandSize) {
+  if (InputParameterBlockSize > mTcgDxeData.BsCap.MaxCommandSize) {
 return EFI_INVALID_PARAMETER;
   }
-  if (OutputParameterBlockSize >= mTcgDxeData.BsCap.MaxResponseSize) {
+  if (OutputParameterBlockSize > mTcgDxeData.BsCap.MaxResponseSize) {
 return EFI_INVALID_PARAMETER;
   }

--
2.9.0.windows.1

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


[edk2] [PATCH v2] MdeModulePkg: Skip registering BootManagerMenu if its FFS is not found

2016-06-27 Thread Sunny Wang
This is a enhancement to support the case when platform firmware doesn’t 
support Boot Manager Menu.
For now, if BootManagerMenu FFS can not be retrieved from FV, BDS core code 
will still register a boot option for it. Then, this non-functional boot option 
will still be booted by user's request (like HotKey or Exit from shell) to 
cause additional boot time and error status code reported. Therefore, it would 
be good to skip BootManagerMenu boot option registration and then return error 
status and Invalid BootOption data for this case so that the BootManagerBoot() 
or other consumers can directly return without doing anything.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sunny Wang 
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  7 ++--
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 39 ++
 MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c |  9 +++--
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c   | 29 +++-
 4 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 0fdb23d..649af9a 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -418,12 +418,13 @@ EfiBootManagerBoot (
   );
 
 /**
-  Return the Boot Manager Menu.
- 
+  Return the boot option corresponding to the Boot Manager Menu.
+  It may automatically create one if the boot option hasn't been created yet.
+
   @param BootOptionReturn the Boot Manager Menu.
 
   @retval EFI_SUCCESS   The Boot Manager Menu is successfully returned.
-  @retval EFI_NOT_FOUND The Boot Manager Menu is not found.
+  @retval StatusReturn status of either gRT->SetVariable () or 
GetSectionFromFv().
 **/
 EFI_STATUS
 EFIAPI
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index d016517..17e416a 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2,7 +2,7 @@
   Library functions which relates with booting.
 
 Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP
+(C) Copyright 2015-2016 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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -2159,16 +2159,19 @@ EfiBootManagerRefreshAllBootOption (
 }
 
 /**
-  This function is called to create the boot option for the Boot Manager Menu.
+  This function is called to get or create the boot option for the Boot 
Manager Menu.
 
   The Boot Manager Menu is shown after successfully booting a boot option.
   Assume the BootManagerMenuFile is in the same FV as the module links to this 
library.
 
-  @param  BootOptionReturn the boot option of the Boot Manager Menu
+  @param  BootOptionReturn the boot option of the Boot Manager Menu.
+If BootManagerMenu fails to be set as a Boot 
variables, BootOption
+still points to the Boot Manager Menu.
+If BootManagerMenu FFS section can not be retrieved, 
BootOption will 
+be contain invalid data to prevent other function from 
using it.
 
   @retval EFI_SUCCESS   Successfully register the Boot Manager Menu.
-  @retval StatusReturn status of gRT->SetVariable (). BootOption still 
points
-to the Boot Manager Menu even the Status is not 
EFI_SUCCESS.
+  @retval StatusReturn status of either gRT->SetVariable() or 
GetSectionFromFv().
 **/
 EFI_STATUS
 BmRegisterBootManagerMenu (
@@ -2181,9 +2184,32 @@ BmRegisterBootManagerMenu (
   EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
   EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage;
   MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;
+  VOID   *Data;
+  UINTN  DataSize;
 
   Status = GetSectionFromFv (
  PcdGetPtr (PcdBootManagerMenuFile),
+ EFI_SECTION_PE32,
+ 0,
+ (VOID **) ,
+ 
+ );
+  if (Data != NULL) {
+FreePool (Data);
+  }
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "[Bds]BootManagerMenu FFS section can not be found, 
skip its boot option registeration\n"));
+ZeroMem (BootOption, sizeof (EFI_BOOT_MANAGER_LOAD_OPTION));
+BootOption->OptionNumber = LoadOptionNumberUnassigned;
+BootOption->OptionType   = LoadOptionTypeMax;
+return Status;
+  }
+
+  //
+  // Get BootManagerMenu application's description from EFI User Interface 
Section.
+  //
+  Status = GetSectionFromFv (
+ PcdGetPtr (PcdBootManagerMenuFile),
  

Re: [edk2] [Patch 000/351] Convert EDK II core packages to NASM for IA32/X64

2016-06-27 Thread Gao, Liming
Hi, all
  I pushed all patches. If you meet with any issues, please raise it to me. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Gao, Liming
> Sent: Thursday, June 23, 2016 2:23 PM
> To: Laszlo Ersek ; Kinney, Michael D
> ; Justen, Jordan L 
> Cc: edk2-de...@ml01.01.org
> Subject: Re: [edk2] [Patch 000/351] Convert EDK II core packages to NASM
> for IA32/X64
> 
> Laszlo:
>Thanks!  If no more comments, I plan to push those patches early next
> week.
> 
> Thanks
> Liming
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Thursday, June 23, 2016 2:44 AM
> > To: Gao, Liming ; Kinney, Michael D
> > ; Justen, Jordan L
> 
> > Cc: edk2-de...@ml01.01.org
> > Subject: Re: [edk2] [Patch 000/351] Convert EDK II core packages to NASM
> > for IA32/X64
> >
> > On 06/22/16 05:15, Gao, Liming wrote:
> > > Laszlo:
> > > I don't find any other case. I have integrated your patches into
> > > https://github.com/lgao4/edk2.git branch nasm-v2. Please help check.
> >
> > I retested the SMM_REQUIRE + S3 cases (Ia32 Fedora, Ia32X64 Fedora,
> > Ia32X64 Windows 8.1), with your nasm-v2 branch (at commit
> > 7c10e13e7c36c593bb102ddc5ae27a747457097f). Everything seems to work
> > fine.
> >
> > I also checked the UefiCpuPkg/PiSmmCpuDxeSmm/*/MpFuncs.nasm files;
> > they
> > look good.
> >
> > The commit messages on
> >
> > 9bdcbb94016c3 UefiCpuPkg PiSmmCpuDxeSmm: Update
> > Ia32/MpFuncs.nasm
> > f6083bde349ce UefiCpuPkg PiSmmCpuDxeSmm: Update
> X64/MpFuncs.nasm
> >
> > seem fine as well.
> >
> > Thanks!
> > Laszlo
> >
> > >
> > > Thanks
> > > Liming
> > > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > > Sent: Tuesday, June 21, 2016 10:37 AM
> > > To: Gao, Liming ; Kinney, Michael D
> > ; Justen, Jordan L
> 
> > > Cc: edk2-de...@ml01.01.org
> > > Subject: Re: [edk2] [Patch 000/351] Convert EDK II core packages to
> NASM
> > for IA32/X64
> > >
> > > On 06/21/16 03:54, Gao, Liming wrote:
> > >> Laszlo:
> > >> Thanks for your test and finding. I notice this difference between
> > >> asm and nasm output file. I read IA32 manual to try understanding 66
> > >> and 67 prefix operand. I wrongly think 66 has the same functionality
> > >> to 66 67. So, I think they are same. I will go through all patches to
> > >> make sure there is no other 67 missing.
> > >>
> > >> ASM dump:
> > >> 0032: 66 67 EA 00 00 00 jmp _RendezvousFunnelProc
> > >> 00
> > >> NASM dump:
> > >> 0032: 66 EA 00 00 00 00 jmp _RendezvousFunnelProc
> > >
> > > Right, that's the pattern.
> > >
> > > Thanks!
> > > Laszlo
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > >
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Driver Health HII menu missing in OVMF master

2016-06-27 Thread Gao, Liming
MdeModulePkg\Universal\DriverHealthManagerDxe\DriverHealthManagerDxe.inf is 
missing after new MdeModulePkg/Universal/BdsDxe/BdsDxe.inf is applied. 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Bruce Cran
> Sent: Tuesday, June 28, 2016 5:19 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Driver Health HII menu missing in OVMF master
> 
> I just noticed that between UDK2015 and master branches in
> https://github.com/tianocore/edk2 the Driver Health display in "Device
> Manager" has disappeared.
> 
> Was its removal deliberate - has it been moved elsewhere?
> 
> --
> Bruce
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var cert name and guid

2016-06-27 Thread Wu, Jiaxin
That's fine. I will create one patch directly:).

Thanks.
Jiaxin 

> -Original Message-
> From: Palmer, Thomas [mailto:thomas.pal...@hpe.com]
> Sent: Tuesday, June 28, 2016 12:51 AM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Zimmer, Vincent ; Li, Ruth
> ; Fu, Siyuan ; Ye, Ting
> ; Hsiung, Harry L ; Shifflett,
> Joseph 
> Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var
> cert name and guid
> 
> I can create the patch if you tell me where to put everything.  Or if you are
> like me, may be easier for you to just code it up.  Either way is fine
> 
> Thomas
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Sunday, June 26, 2016 8:38 PM
> To: Palmer, Thomas ; edk2-devel@lists.01.org
> Cc: El-Haj-Mahmoud, Samer ; Zimmer,
> Vincent ; Li, Ruth ; Fu,
> Siyuan ; Ye, Ting ; Hsiung, Harry L
> 
> Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var
> cert name and guid
> 
> 
> 
> > -Original Message-
> > From: Palmer, Thomas [mailto:thomas.pal...@hpe.com]
> > Sent: Saturday, June 25, 2016 12:51 AM
> > To: Wu, Jiaxin ; edk2-devel@lists.01.org
> > Cc: El-Haj-Mahmoud, Samer ; Zimmer,
> > Vincent ; Li, Ruth ; Fu,
> > Siyuan ; Ye, Ting ; Hsiung,
> > Harry L 
> > Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS
> > var cert name and guid
> >
> > Jiaxin, et al ~
> >
> > I noticed while using the TLS feature that the GUID and Variable Name
> > define were being re-defined in multiple spots.  Currently, if someone
> > were to write a UEFI application, there is no single include file that
> > would provide the variable name in a define. As a matter of sheer
> > better programming practices, the Variable Name define and GUID should
> > be put into central locations and not copied all over the codebase.
> 
> Yes, I agree to put it into a single file. I can create another patch to 
> refine it. If
> you would like to provide it, I'm fine:).
> 
> >
> > With regards to GlobalVariable.h: I realize now this is not the place to 
> > put it.
> > Because our variable has a unique GUID, we would have to create a new
> > MdePkg/Include/Guid/ header file to hold the define, much like
> > ImageAuthentication.h which is also used by VarCheckUefiLibNullClass.c.
> >
> > I put the GUID definition into CryptoPkg.dec because the TlsLib and
> > OpenSSL library are there.  I can be persuaded to have it in
> > NetworkPkg.dec as it's modules are the ultimate consumers of the
> > variable. CryptoPkg is simply providing libraries.
> >
> > With regards to "[TlsCaCertificate is] only a private variable":   This 
> > variable
> is
> > super critical to secure TLS communication.  It is so critical that we
> > are even discussing how to protect it in runtime from malicious/careless
> modifications.
> > We understand that if this variable were compromised that there could
> > be severe security implications that follow.  This variable must be
> > respected properly.
> 
> Actually, I don't have the strong opinion about the security of this variable.
> TlsCaCertificate is used to store the public certificate that means everyone
> can get and use it. Take windows OS as an example, any login user
> can/should have the ability to modify this kind of certificate, we can think 
> it's
> not protected except for the system-level access control. This is the reason
> why I put it into plaintext currently. But I also agree that protecting this
> variable is also meaningful because we no such access control. As for how to
> protect it,  it is another question we discussed before.
> 
> >
> > For that reason, I'd argue that we should put the TlsCaCertificate
> > into the UEFI Spec.  When it gets put into the spec I do not know, but
> > we should be aiming for that. It is too important to security to not be in 
> > the
> spec.
> 
> In my opinion, TlsCaCertificate variable is just a temporary scenario to hold
> the certificate, not the finally or general UEFI solution for the certificate
> management. So,  it's pointless to standardize it, keeping it as a private
> variable is fine currently.
> 
> >
> > Not only that, but once this is in the spec it will enable 3rd party
> > applications to re-use this variable too. I've personally talked with
> > one such developer who is eagerly awaiting a variable that is a secure
> > UEFI standard for Certificate Authority storage.
> >
> > Thomas
> >
> > -Original Message-
> > From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> > Sent: 

Re: [edk2] [RFC V3.1] Proposal to organize packages into directories

2016-06-27 Thread Leif Lindholm
On Mon, Jun 27, 2016 at 03:50:02PM +0100, Ryan Harkin wrote:
> >>   Silicon
> >> Arm
> >>   ArmPkg
> >
> > Much of ArmPkg should probably move across into UefiCpuPkg and
> > MdeModulePkg over time, but this is probably the best thing to do for
> > the restructuring.
> >
> >>   ArmJunoPkg
> >>   ArmVExpressPkg
> >
> > For most of these ARM ltd. platforms, the Silicon is not usefully a
> > separate component (it's early access Silicon designed for a specific
> > development board). So I would lean towards moving all of these to
> > Platforms.
> >
> 
> I agree.  I was just looking at the list and thought, "Those are two
> directories I was supposed to have moved to the OpenPlatformPkg tree
> long before now".

:)

> Perhaps I should just migrate them?  First, patches to add them to
> OpenPlaformPkg, 2nd a patches to remove them from EDK2.  No other
> changes.

Well, but here's the twist: apart from this proposal, there is the
other one about upstream platform support in Tianocore:
http://thread.gmane.org/gmane.comp.bios.edk2.devel/11624/

Once that gets finalised, most of the original reasons behind creating
OpenPlatformPkg goes away. So while it would currently make a lot of
sense to move that code into OpenPlatformPkg, that would mean it would
need to move at least once more in the future.

Regards,

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


[edk2] Driver Health HII menu missing in OVMF master

2016-06-27 Thread Bruce Cran
I just noticed that between UDK2015 and master branches in 
https://github.com/tianocore/edk2 the Driver Health display in "Device 
Manager" has disappeared.


Was its removal deliberate - has it been moved elsewhere?

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


Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var cert name and guid

2016-06-27 Thread Palmer, Thomas
I can create the patch if you tell me where to put everything.  Or if you are 
like me, may be easier for you to just code it up.  Either way is fine

Thomas

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Sunday, June 26, 2016 8:38 PM
To: Palmer, Thomas ; edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer ; Zimmer, Vincent 
; Li, Ruth ; Fu, Siyuan 
; Ye, Ting ; Hsiung, Harry L 

Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS var cert 
name and guid



> -Original Message-
> From: Palmer, Thomas [mailto:thomas.pal...@hpe.com]
> Sent: Saturday, June 25, 2016 12:51 AM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: El-Haj-Mahmoud, Samer ; Zimmer, 
> Vincent ; Li, Ruth ; Fu, 
> Siyuan ; Ye, Ting ; Hsiung, 
> Harry L 
> Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS 
> var cert name and guid
> 
> Jiaxin, et al ~
> 
> I noticed while using the TLS feature that the GUID and Variable Name 
> define were being re-defined in multiple spots.  Currently, if someone 
> were to write a UEFI application, there is no single include file that 
> would provide the variable name in a define. As a matter of sheer 
> better programming practices, the Variable Name define and GUID should 
> be put into central locations and not copied all over the codebase.

Yes, I agree to put it into a single file. I can create another patch to refine 
it. If you would like to provide it, I'm fine:).

> 
> With regards to GlobalVariable.h: I realize now this is not the place to put 
> it.
> Because our variable has a unique GUID, we would have to create a new 
> MdePkg/Include/Guid/ header file to hold the define, much like 
> ImageAuthentication.h which is also used by VarCheckUefiLibNullClass.c.
> 
> I put the GUID definition into CryptoPkg.dec because the TlsLib and 
> OpenSSL library are there.  I can be persuaded to have it in 
> NetworkPkg.dec as it's modules are the ultimate consumers of the 
> variable. CryptoPkg is simply providing libraries.
> 
> With regards to "[TlsCaCertificate is] only a private variable":   This 
> variable is
> super critical to secure TLS communication.  It is so critical that we 
> are even discussing how to protect it in runtime from malicious/careless 
> modifications.
> We understand that if this variable were compromised that there could 
> be severe security implications that follow.  This variable must be 
> respected properly.

Actually, I don't have the strong opinion about the security of this variable.  
TlsCaCertificate is used to store the public certificate that means everyone 
can get and use it. Take windows OS as an example, any login user can/should 
have the ability to modify this kind of certificate, we can think it's not 
protected except for the system-level access control. This is the reason why I 
put it into plaintext currently. But I also agree that protecting this variable 
is also meaningful because we no such access control. As for how to protect it, 
 it is another question we discussed before. 

> 
> For that reason, I'd argue that we should put the TlsCaCertificate 
> into the UEFI Spec.  When it gets put into the spec I do not know, but 
> we should be aiming for that. It is too important to security to not be in 
> the spec.

In my opinion, TlsCaCertificate variable is just a temporary scenario to hold 
the certificate, not the finally or general UEFI solution for the certificate 
management. So,  it's pointless to standardize it, keeping it as a private 
variable is fine currently. 

> 
> Not only that, but once this is in the spec it will enable 3rd party 
> applications to re-use this variable too. I've personally talked with 
> one such developer who is eagerly awaiting a variable that is a secure 
> UEFI standard for Certificate Authority storage.
> 
> Thomas
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Thursday, June 23, 2016 9:56 PM
> To: Palmer, Thomas ; edk2-devel@lists.01.org
> Cc: El-Haj-Mahmoud, Samer ; Zimmer, 
> Vincent ; Li, Ruth ; Fu, 
> Siyuan ; Ye, Ting ; Hsiung, 
> Harry L 
> Subject: RE: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: Centralize TLS 
> var cert name and guid
> 
> Hi Thomas,
> One point we should know that TLS cert variable is not defined in UEFI 
> Spec, it's only private variable used to configure the CA certificate. 
> So, we can't add this variable check into VarCheckUefiLib.  
> VarCheckUefiLib only contain the variables defined in 

Re: [edk2] [RFC V3.1] Proposal to organize packages into directories

2016-06-27 Thread Ryan Harkin
On 26 June 2016 at 16:44, Leif Lindholm  wrote:
> So, I managed to miss this thread for the longest time ... sorry about
> the late comments.
>
> On Fri, Jun 17, 2016 at 10:51:47PM +, Kinney, Michael D wrote:
>> Hello,
>>
>> Please review this one.  I missed a few updates to the dir changes
>> in edk2/master near the bottom.
>>
>> I have looked through all the feedback from the mail threads below
>> and updated this RFC to V3.
>>
>>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/12364
>>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/12754
>>
>> Please let me know if I missed any feedback and if there is new
>> feedback on this revised proposal for the directory structure or the
>> directory names.
>>
>> I have setup a branch with this directory structure in this proposal to help
>> with the review.  I have verified that I can build some platforms in this
>> branch using the PACKAGES_PATH settings shown below.
>>
>> https://github.com/mdkinney/edk2/tree/NewDirStruct_V3
>>
>> ==
>>
>> Updates in V3:
>>
>> ==
>>
>> * IntelFspPkg   -> Silicon/Intel
>> * IntelFspWrapperPkg-> Silicon/Intel
>> * IntelFsp2Pkg  -> Silicon/Intel
>> * IntelFsp2WrapperPkg   -> Silicon/Intel
>> * CorebootModulePkg -> Driver
>> * EmulatorPkg   -> Platform/Common
>> * Nt32Pkg   -> Platform/Common
>>Merge into EmulatorPkg in future
>> * UnixPkg   -> Deprecated
>>Next step is a request to delete
>> * BeagleBoardPkg-> Platform/BeagleBoard
>> * Omap35xxPkg   -> Silicon/TexasInstruments
>> * IntelSiliconPkg   -> Silicon/Intel
>>
>> ==
>>
>>
>>
>> ==
>>
>> Updates in V2:
>>
>> ==
>>
>> * IntelFrameworkModulePkg   -> Deprecated
>> * IntelFrameworkPkg -> Deprecated
>> * IntelFspPkg   -> Deprecated
>> * IntelFspWrapperPkg-> Deprecated
>> * PerformancePkg-> Deprecated
>> * CorebootPayloadPkg-> Platform
>> * EmbeddedPkg   -> Driver
>> * ArmPlatformPkg/ArmJunoPkg -> Silicon/Arm/ArmJunoPkg
>> * ArmPlatformPkg/ArmVExpressPkg -> Silicon/Arm/ ArmVExpressPkg
>> * Change Drivers to Driver so no top level directories are plural.
>> * Remove Vendor directory from Silicon and Platform to reduce directory depth
>> * Add Platform/Common directory for non-vendor specific platform packages
>> * Add Silicon/Common directory for non-vendor specific packages of
>>   CPU/Chipset/SoC drivers
>> * Keep Vendor directory in Driver.
>>   Non-vendor specific packages of drivers are flat just below Driver.
>>   Provides area to migrate non-vendor specific drivers from Core over time
>>
>> ==
>>
>>
>>
>> 
>>
>> # EDK II - Proposal to organize packages into directories
>>
>> There have been some discussions about organizing packages into directories.
>> Below is a proposal for a top level directory structure and a mapping of the
>> packages from edk2/master.  Where applicable, vendor specific directories
>> can be added.
>>
>> The PACKAGES_PATH feature documented in the link below is used to support
>> this proposed directory structure with no source file changes.  An example
>> of setting PACKAGES_PATH in a windows environment is also shown below and
>> I have verified that platforms can be built using this proposal.
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/Multiple_Workspace
>>
>> # Top Level Directory Structure (Listed Alphabetically)
>>
>> ```
>>
>> edk2
>>   Application Applications and application support libraries
>>   BaseTools   EDK II build tools/scripts
>>   ConfEDK II build configuration files
>>   CorePlatform agnostic packages for core FW services
>>   Deprecated  Packages that may be removed from edk2/master
>>   Driver  EDK II Drivers (no platform assumptions)
>> Non-Vendor specific EDK II drivers
>> Non-Vendor specific EDK II drivers
>> . . .
>> VendorVendor specific EDK II drivers
>>   
>>   
>
> Following on to the previous discussions, I still like the idea of
> separate platform (PEI) drivers and UEFI drivers directories. The
> former could be the Vendor portion.
> However, I also agree with Jordan that for UEFI drivers, the
> Linux-style functional organisation makes more sense. (You are more
> likely to want maintainers/reviewers to pay attention to all devices
> of one type rather than all devices from one manufacturer.)
>
>>   PlatformPlatforms used to validate edk2/master features
>> Arm   ARM specific platform packages
>> BeagleBoard   Beagle Board specific platform packages
>
> If this refers to BeagleBoard as in BeagleBoard.org, as 

Re: [edk2] Adding a memory region to GCD on AARCH64?

2016-06-27 Thread Achin Gupta
Hi Laszlo,

On Thu, Jun 23, 2016 at 04:38:03PM +0200, Laszlo Ersek wrote:
> On 06/23/16 16:19, Achin Gupta wrote:
> > Hi Laszlo,
> >
> > On Wed, Jun 22, 2016 at 09:56:11PM +0200, Laszlo Ersek wrote:
> >> On 06/22/16 20:53, Achin Gupta wrote:
> >>> Hi All,
> >>>
> >>> I having some trouble trying an experiment on the AARCH64 Base FVP with 
> >>> UEFI and
> >>> ARM Trusted Firmware. There is a buffer that is allocated by the latter 
> >>> in DRAM
> >>> with TZC-400 attributes that allow non-secure access. Its extents are made
> >>> available to a UEFI DXE driver through an SMC. AFAIU, it should be added 
> >>> to the
> >>> UEFI memory map/EL2 translation tables and subsequently allocated before 
> >>> it can
> >>> be used.
> >>>
> >>> To add it to the memory map, I successfully call AddMemorySpace() GCD 
> >>> service as
> >>> follows:
> >>>
> >>>  Status = gDS->AddMemorySpace(EfiGcdMemoryTypeSystemMemory,
> >>>   (EFI_PHYSICAL_ADDRESS) mNsBufferAddress,
> >>>   mNsBufferMaxSize,
> >>>   EFI_MEMORY_WB | EFI_MEMORY_XP);
> >>>
> >>> To allocate this buffer, I call AllocateMemorySpace GCD service 
> >>> immediately
> >>> after AddMemorySpace() as follows:
> >>>
> >>> Status = gDS->AllocateMemorySpace(EfiGcdAllocateAddress,
> >>>   EfiGcdMemoryTypeSystemMemory,
> >>>   EFI_PAGE_SHIFT,
> >>>   mNsBufferMaxSize,
> >>>   (EFI_PHYSICAL_ADDRESS *) 
> >>> ,
> >>>   ImageHandle,
> >>>   NULL);
> >>>
> >>> The address of the buffer is 0xfbe0 and size is 0x20 i.e. it is 
> >>> aligned
> >>> to the page boundary. This call fails with EFI_NOT_FOUND. I am unable to 
> >>> figure
> >>> out what is going wrong. Am I using these interfaces in their intended 
> >>> way?
> >>
> >> I recall the following from the relevant volume of the PI spec (please
> >> look it up and double check it): when you add memory space of type
> >> EfiGcdMemoryTypeSystemMemory, it is immediately absorbed by the pool /
> >> page UEFI memory allocation services, for their internal management. So
> >> after you add this, you can't allocate memory *space* of
> >> EfiGcdMemoryTypeSystemMemory. If you want to allocate system memory,
> >> just use the AllocatePool() / AllocatePages() boot services.
> >
> > Makes sense. The spec. says that the new memory range may be automatically
> > allocated for use by UEFI memory services. The EDK2 implementation always 
> > does
> > this in CoreAddMemorySpace() in MdeModulePkg/Core/Dxe/Gcd/Gcd.c.
> >
> >>
> >> The pattern that you use above is valid, but you should use a different
> >> GCD memory type (probably EfiGcdMemoryTypeReserved).
> >
> > I have not tried this memory type. I think there is a more basic problem. 
> > Please
> > see below!
> >
> >>
> >> If you definitely need "system memory" (= plain memory) to reside at
> >> this address, and want to allocate it right after adding it to the
> >> global coherency domain as EfiGcdMemoryTypeSystemMemory, then allocate
> >> it with the AllocatePages() boot service, setting the first argument
> >> (--> EFI_ALLOCATE_TYPE) to AllocateAddress (--> attempt to allocate at
> >> the fixed address).
> >
> > I gave this a try since a similar approach has been followed in some ARM
> > platform code. However, it does not make a difference. I run into a Level 2
> > translation fault as soon as I try to access the buffer. I have disabled 
> > cache
> > state modelling so there are no TLBs or caches. So the translation fault is
> > likely to be 'cause of an invalid/missing page table descriptor.  .
> >
> > I think I am missing something obvious being a UEFI newbie. There are three
> > steps that need to be undertaken in order to access this buffer.
> >
> > 1. Add an identity mapped entry corresponding to the buffer in the 
> > translation
> >tables of the exception level UEFI is executing in (EL2 in my case).
> >
> > 2. Add this buffer in the UEFI memory map so that it can either be consumed 
> > by
> >the memory allocation services or reserved.
> >
> > 3. Allocate this buffer using its physical address so that it is not 
> > allocated
> >to another driver.
> >
> > I was under the impression that gDS->AddMemorySpace() will perform both 
> > steps
> > 1. and 2. However looking at the behaviour and the code, it seems that it 
> > does
> > only 2.
> >
> > Is it at all possible to do 1. after the translation tables have been 
> > created
> > and the MMU initialised in the PEI phase. This essentially means adding 
> > entries
> > to live translation tables. Is this expected to be done through architecture
> > specific mechanisms rather than the GCD services.
>
> Is the base address of the new memory area above any other address that
> you access otherwise?

Yes it is. It is still a region in DRAM. 

Re: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions complying Industry Standard specifications.

2016-06-27 Thread Gao, Liming
Got it. We will review this change. 

> -Original Message-
> From: Anandakrishnan Loganathan [mailto:anandakrishn...@ami.com]
> Sent: Monday, June 27, 2016 5:12 PM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions complying
> Industry Standard specifications.
> 
> Hi Gao,
> 
> Sorry.. I will check the problem in creating the patch my side. Meanwhile I
> have attached the Atapi.h for your reference
> 
> Thanks!
> 
> 
> -Original Message-
> From: Gao, Liming [mailto:liming@intel.com]
> Sent: 27 June 2016 14:33
> To: Anandakrishnan Loganathan; edk2-devel@lists.01.org
> Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions complying
> Industry Standard specifications.
> 
> Anandakrishnan:
>Your patch format is Unicode. I can't apply it. Could you create the normal
> GIT patch or directly send the updated Atapi.h?
> 
> > -Original Message-
> > From: Anandakrishnan Loganathan [mailto:anandakrishn...@ami.com]
> > Sent: Monday, June 27, 2016 2:02 PM
> > To: Gao, Liming ; edk2-devel@lists.01.org
> > Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions
> > complying Industry Standard specifications.
> >
> > Attached the Patch with the mail
> >
> > -Original Message-
> > From: Gao, Liming [mailto:liming@intel.com]
> > Sent: 27 June 2016 09:41
> > To: Anandakrishnan Loganathan; edk2-devel@lists.01.org
> > Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions
> > complying Industry Standard specifications.
> >
> > Hi, could you attach your patch?
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Anandakrishnan Loganathan
> > > Sent: Friday, June 24, 2016 4:56 PM
> > > To: edk2-devel@lists.01.org
> > > Subject: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions
> > > complying Industry Standard specifications.
> > >
> > > Dear MdePkg maintainer,
> > >
> > >
> > > Atapi.h  has only limited ATA/ATAPI related specification
> > > definitions and this attached patch includes various commonly usage
> > > Industry Standard definitions in Atapi.h. Please review and update to the
> MdePkg.
> > >
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Anandakrishnan Loganathan
> > > anandakrishn...@ami.com
> > >
> > >
> > > Thanks!
> > > Anandakrishnan
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions complying Industry Standard specifications.

2016-06-27 Thread Anandakrishnan Loganathan
Hi Gao,

Sorry.. I will check the problem in creating the patch my side. Meanwhile I 
have attached the Atapi.h for your reference

Thanks!


-Original Message-
From: Gao, Liming [mailto:liming@intel.com] 
Sent: 27 June 2016 14:33
To: Anandakrishnan Loganathan; edk2-devel@lists.01.org
Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions complying 
Industry Standard specifications.

Anandakrishnan:
   Your patch format is Unicode. I can't apply it. Could you create the normal 
GIT patch or directly send the updated Atapi.h?

> -Original Message-
> From: Anandakrishnan Loganathan [mailto:anandakrishn...@ami.com]
> Sent: Monday, June 27, 2016 2:02 PM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions 
> complying Industry Standard specifications.
> 
> Attached the Patch with the mail
> 
> -Original Message-
> From: Gao, Liming [mailto:liming@intel.com]
> Sent: 27 June 2016 09:41
> To: Anandakrishnan Loganathan; edk2-devel@lists.01.org
> Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions 
> complying Industry Standard specifications.
> 
> Hi, could you attach your patch?
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Anandakrishnan Loganathan
> > Sent: Friday, June 24, 2016 4:56 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions 
> > complying Industry Standard specifications.
> >
> > Dear MdePkg maintainer,
> >
> >
> > Atapi.h  has only limited ATA/ATAPI related specification 
> > definitions and this attached patch includes various commonly usage 
> > Industry Standard definitions in Atapi.h. Please review and update to the 
> > MdePkg.
> >
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Anandakrishnan Loganathan 
> > anandakrishn...@ami.com
> >
> >
> > Thanks!
> > Anandakrishnan
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
/** @file
  This file contains just some basic definitions that are needed by drivers
  that dealing with ATA/ATAPI interface.

Copyright (c) 2007 - 2013, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available 
under 
the terms and conditions of the BSD License that accompanies this distribution. 
 
The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php. 
 

THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,   
  
WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

**/

#ifndef _ATAPI_H_
#define _ATAPI_H_

#pragma pack(1)

///
/// ATA5_IDENTIFY_DATA is defined in ATA-5.
/// (This structure is provided mainly for backward-compatibility support.
/// Old drivers may reference fields that are marked "obsolete" in 
/// ATA_IDENTIFY_DATA, which currently conforms to ATA-8.) 
///
typedef struct {
  UINT16  config; ///< General Configuration.
  UINT16  cylinders;  ///< Number of Cylinders.
  UINT16  reserved_2; 
  UINT16  heads;  ///< Number of logical heads. 
  UINT16  vendor_data1; 
  UINT16  vendor_data2; 
  UINT16  sectors_per_track; 
  UINT16  vendor_specific_7_9[3]; 
  CHAR8   SerialNo[20];   ///< ASCII  
  UINT16  vendor_specific_20_21[2];  
  UINT16  ecc_bytes_available;
  CHAR8   FirmwareVer[8]; ///< ASCII  
  CHAR8   ModelName[40];  ///< ASCII
  UINT16  multi_sector_cmd_max_sct_cnt; 
  UINT16  reserved_48; 
  UINT16  capabilities; 
  UINT16  reserved_50; 
  UINT16  pio_cycle_timing;
  UINT16  reserved_52; 
  UINT16  field_validity; 
  UINT16  current_cylinders; 
  UINT16  current_heads; 
  UINT16  current_sectors;
  UINT16  CurrentCapacityLsb; 
  UINT16  CurrentCapacityMsb; 
  UINT16  reserved_59; 
  UINT16  user_addressable_sectors_lo; 
  UINT16  user_addressable_sectors_hi; 
  UINT16  reserved_62; 
  UINT16  multi_word_dma_mode;
  UINT16  advanced_pio_modes; 
  UINT16  min_multi_word_dma_cycle_time; 
  UINT16  rec_multi_word_dma_cycle_time; 
  UINT16  min_pio_cycle_time_without_flow_control; 
  UINT16  min_pio_cycle_time_with_flow_control; 
  UINT16  reserved_69_79[11]; 
  UINT16  major_version_no; 
  UINT16  minor_version_no; 
  UINT16  command_set_supported_82;///< word 82 
  UINT16  command_set_supported_83;///< word 83 
  UINT16  command_set_feature_extn;///< word 84 
  UINT16  command_set_feature_enb_85;  ///< word 85 
  UINT16  command_set_feature_enb_86;  ///< word 86 
  UINT16  command_set_feature_default; ///< word 87 
  UINT16  ultra_dma_mode;  ///< word 88 
  UINT16  reserved_89_127[39]; 
  UINT16  security_status; 
  

Re: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions complying Industry Standard specifications.

2016-06-27 Thread Gao, Liming
Anandakrishnan:
   Your patch format is Unicode. I can't apply it. Could you create the normal 
GIT patch or directly send the updated Atapi.h?

> -Original Message-
> From: Anandakrishnan Loganathan [mailto:anandakrishn...@ami.com]
> Sent: Monday, June 27, 2016 2:02 PM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions complying
> Industry Standard specifications.
> 
> Attached the Patch with the mail
> 
> -Original Message-
> From: Gao, Liming [mailto:liming@intel.com]
> Sent: 27 June 2016 09:41
> To: Anandakrishnan Loganathan; edk2-devel@lists.01.org
> Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions complying
> Industry Standard specifications.
> 
> Hi, could you attach your patch?
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Anandakrishnan Loganathan
> > Sent: Friday, June 24, 2016 4:56 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions
> > complying Industry Standard specifications.
> >
> > Dear MdePkg maintainer,
> >
> >
> > Atapi.h  has only limited ATA/ATAPI related specification definitions
> > and this attached patch includes various commonly usage Industry
> > Standard definitions in Atapi.h. Please review and update to the MdePkg.
> >
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Anandakrishnan Loganathan
> > anandakrishn...@ami.com
> >
> >
> > Thanks!
> > Anandakrishnan
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in AuthVariableLib

2016-06-27 Thread Zeng, Star

Reviewed-by: Star Zeng 

On 2016/6/27 15:12, Yao, Jiewen wrote:

Reviewed-by: jiewen@intel.com


-Original Message-
From: Zhang, Chao B
Sent: Monday, June 27, 2016 3:06 PM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen ; Zeng, Star ;
Zhang, Chao B 
Subject: [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in
AuthVariableLib

AuthVariableLib is updated to cache the UserPhysicalPresent state to global
variable. This avoids calling PlatformSecureLib during runtime and makes
PhysicalPresent state consistent during one boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 
 SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h | 1 +
 SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 7 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 6e1e284..1d49b6a 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -931,7 +931,7 @@ ProcessVarWithPk (
   // Init state of Del. State may change due to secure check
   //
   Del = FALSE;
-  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode ==
SETUP_MODE && !IsPk)) {
+  if ((InCustomMode() && mUserPhysicalPresent) || (mPlatformMode ==
SETUP_MODE && !IsPk)) {
 Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
 PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
 if (PayloadSize == 0) {
@@ -1049,7 +1049,7 @@ ProcessVarWithKek (
   }

   Status = EFI_SUCCESS;
-  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
UserPhysicalPresent())) {
+  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
mUserPhysicalPresent)) {
 //
 // Time-based, verify against X509 Cert KEK.
 //
@@ -1204,7 +1204,7 @@ ProcessVariable (
  
  );

-  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
UserPhysicalPresent()) {
+  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
mUserPhysicalPresent) {
 //
 // Allow the delete operation of common authenticated variable at
user physical presence.
 //
@@ -1222,7 +1222,7 @@ ProcessVariable (
 return Status;
   }

-  if (NeedPhysicallyPresent (VariableName, VendorGuid)
&& !UserPhysicalPresent()) {
+  if (NeedPhysicallyPresent (VariableName, VendorGuid)
&& !mUserPhysicalPresent) {
 //
 // This variable is protected, only physical present user could modify its
value.
 //
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..ac7ea89 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -128,6 +128,7 @@ extern UINT8*mCertDbStore;
 extern UINT32   mMaxCertDbSize;
 extern UINT32   mPlatformMode;
 extern UINT8mVendorKeyState;
+extern BOOLEAN  mUserPhysicalPresent;

 extern VOID *mHashCtx;

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index c4fbb64..dd35a44 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
@@ -35,6 +35,7 @@ UINT8*mCertDbStore;
 UINT32   mMaxCertDbSize;
 UINT32   mPlatformMode;
 UINT8mVendorKeyState;
+BOOLEAN  mUserPhysicalPresent;

 EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID,
EFI_CERT_SHA256_GUID, EFI_CERT_RSA2048_GUID,
EFI_CERT_X509_GUID};

@@ -435,6 +436,12 @@ AuthVariableLibInitialize (
   AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
   AuthVarLibContextOut->AddressPointerCount = sizeof
(mAuthVarAddressPointer) / sizeof (mAuthVarAddressPointer[0]);

+  //
+  // Cache UserPhysicalPresent State.
+  // Platform should report PhysicalPresent before this point
+  //
+  mUserPhysicalPresent = UserPhysicalPresent();
+
   return Status;
 }

--
1.9.5.msysgit.1

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


[edk2] [PATCH v2] MdeModulePkg/MemoryStatusCode: Expose the DXE memory status code table.

2016-06-27 Thread Cinnamon Shia
Let data of DXE memory status code can be used by other modules.
1. Save the address of DXE memory status code table to DxeConfigurationTable.
2. Save the address of SMM memory status code table to SmmConfigurationTable.
3. Move RUNTIME_MEMORY_STATUSCODE_HEADER to its public header file.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
 MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h | 30 ++
 .../RuntimeDxe/MemoryStatusCodeWorker.c| 27 +++
 .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h   | 17 
 .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf |  3 ++-
 .../StatusCodeHandler/Smm/MemoryStatusCodeWorker.c | 21 ++-
 .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.h   | 18 -
 .../StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf |  4 ++-
 7 files changed, 71 insertions(+), 49 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h 
b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
index d6c3243..27d39e1 100644
--- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
+++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
@@ -2,13 +2,14 @@
   GUID used to identify status code records HOB that originate from the PEI 
status code.
   
 Copyright (c) 2006 - 2010, 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.  
+(C) Copyright 2016 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 that accompanies this distribution.
 The full text of the license may be found at
-http://opensource.org/licenses/bsd-license.php.

+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.  
   
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
@@ -56,6 +57,25 @@ typedef struct {
 } MEMORY_STATUSCODE_PACKET_HEADER;
 
 ///
+/// A header structure that is followed by an array of records that contain 
the 
+/// parameters passed into the ReportStatusCode() service in the DXE Services 
Table.
+///
+typedef struct {
+  ///
+  /// The index pointing to the last recored being stored.
+  ///
+  UINT32   RecordIndex;
+  ///
+  /// The number of records being stored.
+  ///
+  UINT32   NumberOfRecords;
+  ///
+  /// The maximum number of records that can be stored.
+  ///
+  UINT32   MaxRecordsNumber;
+} RUNTIME_MEMORY_STATUSCODE_HEADER;
+
+///
 /// A structure that contains the parameters passed into the 
ReportStatusCode() 
 /// service in the PEI Services Table.
 ///
diff --git 
a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStatusCodeWorker.c 
b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStatusCodeWorker.c
index 8680497..a306d75 100644
--- 
a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStatusCodeWorker.c
+++ 
b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/MemoryStatusCodeWorker.c
@@ -2,13 +2,14 @@
   Runtime memory status code worker.
 
   Copyright (c) 2006 - 2009, 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. 
+  (C) Copyright 2016 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
+  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.
 
 **/
 
@@ -18,8 +19,9 @@ RUNTIME_MEMORY_STATUSCODE_HEADER  *mRtMemoryStatusCodeTable;
 
 /**
   Initialize runtime memory status code table as initialization for runtime 
memory status code worker
- 
+
   @retval EFI_SUCCESS  Runtime memory status code table successfully 
initialized.
+  @retval others  

Re: [edk2] [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in AuthVariableLib

2016-06-27 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zhang, Chao B
> Sent: Monday, June 27, 2016 3:06 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star ;
> Zhang, Chao B 
> Subject: [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in
> AuthVariableLib
> 
> AuthVariableLib is updated to cache the UserPhysicalPresent state to global
> variable. This avoids calling PlatformSecureLib during runtime and makes
> PhysicalPresent state consistent during one boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 
>  SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h | 1 +
>  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 7 +++
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> index 6e1e284..1d49b6a 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> @@ -931,7 +931,7 @@ ProcessVarWithPk (
>// Init state of Del. State may change due to secure check
>//
>Del = FALSE;
> -  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode ==
> SETUP_MODE && !IsPk)) {
> +  if ((InCustomMode() && mUserPhysicalPresent) || (mPlatformMode ==
> SETUP_MODE && !IsPk)) {
>  Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
>  PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
>  if (PayloadSize == 0) {
> @@ -1049,7 +1049,7 @@ ProcessVarWithKek (
>}
> 
>Status = EFI_SUCCESS;
> -  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
> UserPhysicalPresent())) {
> +  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
> mUserPhysicalPresent)) {
>  //
>  // Time-based, verify against X509 Cert KEK.
>  //
> @@ -1204,7 +1204,7 @@ ProcessVariable (
>   
>   );
> 
> -  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
> (OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
> UserPhysicalPresent()) {
> +  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
> (OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
> mUserPhysicalPresent) {
>  //
>  // Allow the delete operation of common authenticated variable at
> user physical presence.
>  //
> @@ -1222,7 +1222,7 @@ ProcessVariable (
>  return Status;
>}
> 
> -  if (NeedPhysicallyPresent (VariableName, VendorGuid)
> && !UserPhysicalPresent()) {
> +  if (NeedPhysicallyPresent (VariableName, VendorGuid)
> && !mUserPhysicalPresent) {
>  //
>  // This variable is protected, only physical present user could modify 
> its
> value.
>  //
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> index e7c4bf0..ac7ea89 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> @@ -128,6 +128,7 @@ extern UINT8*mCertDbStore;
>  extern UINT32   mMaxCertDbSize;
>  extern UINT32   mPlatformMode;
>  extern UINT8mVendorKeyState;
> +extern BOOLEAN  mUserPhysicalPresent;
> 
>  extern VOID *mHashCtx;
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> index c4fbb64..dd35a44 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> @@ -35,6 +35,7 @@ UINT8*mCertDbStore;
>  UINT32   mMaxCertDbSize;
>  UINT32   mPlatformMode;
>  UINT8mVendorKeyState;
> +BOOLEAN  mUserPhysicalPresent;
> 
>  EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID,
> EFI_CERT_SHA256_GUID, EFI_CERT_RSA2048_GUID,
> EFI_CERT_X509_GUID};
> 
> @@ -435,6 +436,12 @@ AuthVariableLibInitialize (
>AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
>AuthVarLibContextOut->AddressPointerCount = sizeof
> (mAuthVarAddressPointer) / sizeof (mAuthVarAddressPointer[0]);
> 
> +  //
> +  // Cache UserPhysicalPresent State.
> +  // Platform should report PhysicalPresent before this point
> +  //
> +  mUserPhysicalPresent = UserPhysicalPresent();
> +
>return Status;
>  }
> 
> --
> 1.9.5.msysgit.1

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


[edk2] [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in AuthVariableLib

2016-06-27 Thread Zhang, Chao B
AuthVariableLib is updated to cache the UserPhysicalPresent state to global 
variable. This avoids calling PlatformSecureLib during runtime and makes 
PhysicalPresent state consistent during one boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 
 SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h | 1 +
 SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 7 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 6e1e284..1d49b6a 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -931,7 +931,7 @@ ProcessVarWithPk (
   // Init state of Del. State may change due to secure check
   //
   Del = FALSE;
-  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode == 
SETUP_MODE && !IsPk)) {
+  if ((InCustomMode() && mUserPhysicalPresent) || (mPlatformMode == SETUP_MODE 
&& !IsPk)) {
 Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
 PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
 if (PayloadSize == 0) {
@@ -1049,7 +1049,7 @@ ProcessVarWithKek (
   }
 
   Status = EFI_SUCCESS;
-  if (mPlatformMode == USER_MODE && !(InCustomMode() && 
UserPhysicalPresent())) {
+  if (mPlatformMode == USER_MODE && !(InCustomMode() && mUserPhysicalPresent)) 
{
 //
 // Time-based, verify against X509 Cert KEK.
 //
@@ -1204,7 +1204,7 @@ ProcessVariable (
  
  );
 
-  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable 
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) && 
UserPhysicalPresent()) {
+  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable 
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) && 
mUserPhysicalPresent) {
 //
 // Allow the delete operation of common authenticated variable at user 
physical presence.
 //
@@ -1222,7 +1222,7 @@ ProcessVariable (
 return Status;
   }
 
-  if (NeedPhysicallyPresent (VariableName, VendorGuid) && 
!UserPhysicalPresent()) {
+  if (NeedPhysicallyPresent (VariableName, VendorGuid) && 
!mUserPhysicalPresent) {
 //
 // This variable is protected, only physical present user could modify its 
value.
 //
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..ac7ea89 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -128,6 +128,7 @@ extern UINT8*mCertDbStore;
 extern UINT32   mMaxCertDbSize;
 extern UINT32   mPlatformMode;
 extern UINT8mVendorKeyState;
+extern BOOLEAN  mUserPhysicalPresent;
 
 extern VOID *mHashCtx;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c 
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index c4fbb64..dd35a44 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
@@ -35,6 +35,7 @@ UINT8*mCertDbStore;
 UINT32   mMaxCertDbSize;
 UINT32   mPlatformMode;
 UINT8mVendorKeyState;
+BOOLEAN  mUserPhysicalPresent;
 
 EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID, EFI_CERT_SHA256_GUID, 
EFI_CERT_RSA2048_GUID, EFI_CERT_X509_GUID};
 
@@ -435,6 +436,12 @@ AuthVariableLibInitialize (
   AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
   AuthVarLibContextOut->AddressPointerCount = sizeof (mAuthVarAddressPointer) 
/ sizeof (mAuthVarAddressPointer[0]);
 
+  //
+  // Cache UserPhysicalPresent State. 
+  // Platform should report PhysicalPresent before this point
+  //
+  mUserPhysicalPresent = UserPhysicalPresent();
+
   return Status;
 }
 
-- 
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] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-27 Thread Evgeny Yakovlev
Great, thanks!

> 27 июня 2016 г., в 7:51, Tian, Feng  написал(а):
> 
> Thanks, Yakovlev.
>  
> If there is no objection, I will commit and push these two patches to git 
> repo with your RB and mine.
>  
> Thanks
> Feng
>  
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
> Sent: Friday, June 24, 2016 9:35 PM
> To: Tian, Feng 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
>  
> Hi! We verified that xHCI with your patch correctly handles handshaking with 
> the problem device. 
>  
> 2016-06-14 3:42 GMT+03:00 Tian, Feng :
> Thanks for your info, Yakovlev.
> 
> Did you see problem with XHCI? If yes, I can provide a patch and could you 
> help verify it?
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com]
> Sent: Monday, June 13, 2016 5:29 PM
> To: Tian, Feng 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi Feng.
> 
> Sorry, i was AFK for the past week.
> 
> We have encountered this with Platronix USB headset. This device was 
> reporting *USB Interface* descriptor sizes 1 byte larger than what the spec 
> says. Headset was simply plugged in when we booted our firmware and 
> UsbCreateDesc failed to parse that and returned NULL which eventually crashed 
> the firmware later (i will try and send a separate patch to fix that). We 
> fixed UsbCreateDesc and device enumeration went fine, all descriptors it 
> sends are correct except for this odd size field.
> 
> Evgeny
> 
> > On 13 Jun 2016, at 05:32, Tian, Feng  wrote:
> >
> > Hi, Yakovlev
> >
> > Any response on this?
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: Tian, Feng
> > Sent: Monday, June 6, 2016 9:27 AM
> > To: Evgeny Yakovlev ; edk2-devel@lists.01.org
> > Cc: Tian, Feng 
> > Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> > length check
> >
> > Hi, Evgenv
> >
> > Thanks for your contribution, Evgenv
> >
> > Could you let me know what error case you met? Is the Device Desc or Config 
> > Desc larger than expected or other descs?
> >
> > Why I ask this question is because it would impact Xhci driver's 
> > implementation in which we store some desc contents at internal. So looks 
> > like XHCI driver also needs to be updated.
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> > Evgeny Yakovlev
> > Sent: Sunday, June 5, 2016 10:29 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> > check
> >
> > According to spec if the length of a descriptor is smaller than what the 
> > specification defines, then the host shall ignore it.
> > However if the size is greater than expected the host will ignore the extra 
> > bytes and start looking for the next descriptor at the end of actual length 
> > returned. Original check did not handle the latter case correctly and only 
> > allowed descriptors with lengths exactly as defined in specification.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Evgeny Yakovlev 
> > ---
> > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > index 5b8b1aa..fba60da 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > @@ -199,8 +199,8 @@ UsbCreateDesc (
> > }
> >   }
> >
> > -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> > -  (Head->Type != Type) || (Head->Len != DescLen)) {
> > +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> > +  (Head->Type != Type) || (Head->Len < DescLen)) {
> > DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> > return NULL;
> >   }
> > --
> > 2.7.4 (Apple Git-66)
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
>  
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions complying Industry Standard specifications.

2016-06-27 Thread Anandakrishnan Loganathan
Attached the Patch with the mail

-Original Message-
From: Gao, Liming [mailto:liming@intel.com] 
Sent: 27 June 2016 09:41
To: Anandakrishnan Loganathan; edk2-devel@lists.01.org
Subject: RE: [PATCH] [MdePkg ] Additional Atapi.h definitions complying 
Industry Standard specifications.

Hi, could you attach your patch? 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Anandakrishnan Loganathan
> Sent: Friday, June 24, 2016 4:56 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] [MdePkg ] Additional Atapi.h definitions 
> complying Industry Standard specifications.
> 
> Dear MdePkg maintainer,
> 
> 
> Atapi.h  has only limited ATA/ATAPI related specification definitions 
> and this attached patch includes various commonly usage Industry 
> Standard definitions in Atapi.h. Please review and update to the MdePkg.
> 
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Anandakrishnan Loganathan 
> anandakrishn...@ami.com
> 
> 
> Thanks!
> Anandakrishnan
> ___
> 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