Re: [edk2] [PATCH] SecurityPkg/OpalPassword: Add NULL pointer check before using it

[Dong, Eric]

Hi Maggie,

Thanks for your contribution.

Reviewed-by: Eric Dong 

Pushed: d72d8561fbe03a64e01c2ad57a93777de4b9ae2f

Thanks,
Eric

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Maggie Chu
> Sent: Friday, February 1, 2019 6:44 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Dong, Eric ;
> Zhang, Chao B 
> Subject: [edk2] [PATCH] SecurityPkg/OpalPassword: Add NULL pointer check
> before using it
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1503
> A pointer variable should be checked if it is NULL or Valid before using it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Maggie Chu 
> Cc: Chao Zhang 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 734c5f06ff..f79f9f7282 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -436,6 +436,9 @@ BuildOpalDeviceInfoAta (
> 
>DevInfoAta = AllocateZeroPool (DevInfoLengthAta);
>ASSERT (DevInfoAta != NULL);
> +  if (DevInfoAta == NULL) {
> +return;
> +  }
> 
>TempDevInfoAta = DevInfoAta;
>TmpDev = mOpalDriver.DeviceList;
> @@ -527,6 +530,9 @@ BuildOpalDeviceInfoNvme (
> 
>DevInfoNvme = AllocateZeroPool (DevInfoLengthNvme);
>ASSERT (DevInfoNvme != NULL);
> +  if (DevInfoNvme == NULL) {
> +return;
> +  }
> 
>TempDevInfoNvme = DevInfoNvme;
>TmpDev = mOpalDriver.DeviceList;
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] SecurityPkg/OpalPassword: Update strings on Opal Setup page

[Dong, Eric]

Hi Maggie,

Thanks for your contribution. 

Reviewed-by: Eric Dong 

Pushed: SHA-1: 315873959ec3e0a44168efdf4c03fe6160121cdb

Thanks,
Eric

> -Original Message-
> From: Chu, Maggie
> Sent: Wednesday, February 13, 2019 2:56 PM
> To: edk2-devel@lists.01.org
> Cc: Zhang, Chao B ; Yao, Jiewen
> ; Dong, Eric 
> Subject: [PATCH] SecurityPkg/OpalPassword: Update strings on Opal Setup
> page
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1506
> Updated some descriptions on SETUP page to avoid user confusion.
> Currently it shows "1.0 UEFI Opal Driver", however it may be mislead user to
> think it is only for Opal drive but not for Pyrite drive.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Maggie Chu 
> Cc: Chao Zhang 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c  | 10 --
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h  | 11 ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiCallbacks.c | 16 
> +---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormStrings.uni | 11 ++
> -
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr   |  4 ++--
>  5 files changed, 9 insertions(+), 43 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> index 4336604d0d..51aa93c2fd 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> @@ -393,7 +393,6 @@ OpalHiiAddPackages(
>)
>  {
>EFI_HANDLE   DriverHandle;
> -  CHAR16   *NewString;
> 
>DriverHandle = HiiGetDriverImageHandleCB();
> 
> @@ -416,15 +415,6 @@ OpalHiiAddPackages(
>  return EFI_OUT_OF_RESOURCES;
>}
> 
> -  //
> -  // Update Version String in main window
> -  //
> -  NewString = HiiGetDriverNameCB ();
> -  if (HiiSetString(gHiiPackageListHandle,
> STRING_TOKEN(STR_MAIN_OPAL_VERSION), NewString, NULL) == 0) {
> -DEBUG ((DEBUG_INFO,  "OpalHiiAddPackages: HiiSetString( ) failed\n"));
> -return EFI_OUT_OF_RESOURCES;
> -  }
> -
>return EFI_SUCCESS;
>  }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> index 8b368fe995..e137a55810 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> @@ -285,17 +285,6 @@ OpalHiiAddPackages(
>VOID
>);
> 
> -/**
> -  Returns the driver name.
> -
> -  @retval Returns the driver name.
> -
> -**/
> -CHAR16*
> -HiiGetDriverNameCB(
> -  VOID
> -  );
> -
>  /**
>Returns the opaque pointer to a physical disk context.
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiCallbacks.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiCallbacks.c
> index 31e1aa2240..1b6f067076 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiCallbacks.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiCallbacks.c
> @@ -115,18 +115,4 @@ HiiDiskGetNameCB(
>  return Ctx->NameZ;
>}
>return NULL;
> -}
> -
> -/**
> -  Returns the driver name.
> -
> -  @retval Returns the driver name.
> -
> -**/
> -CHAR16*
> -HiiGetDriverNameCB(
> -  VOID
> -  )
> -{
> -  return (CHAR16*)EFI_DRIVER_NAME_UNICODE; -}
> +}
> \ No newline at end of file
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormStrings.uni
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormStrings.uni
> index 5f753dfa8c..44cfc80626 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormStrings.uni
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormStrings.uni
> @@ -20,14 +20,14 @@
>  #string STR_NULL#language en-US " "
> 
>  /   FORM SET
> /
> -#string STR_FORM_SET_HELP#language en-US "Manage Opal
> disks"
> +#string STR_FORM_SET_HELP#language en-US "Select to
> manage"
> 
>  /   MULTIPLE FORMS
> /
> -#string STR_OPAL #language en-US "Opal"
> -#string STR_MAIN_OPAL_VERSION#language en-US "Version
> 00.0.0."
> +#string STR_OPAL #language en-US "TCG Drive
> Management"
> 
>  /   MAIN MENU FORM
> /
>  #string STR_MAIN_PHY_DISKS_LBL   #language en-US "Physical
> Disks:"
> +#string STR_MAIN_OPAL_TITLE_LBL  #language en-US "Allows user
> to choose drive to configure drive security. User can also set storage action
> policy for use of the TCG Block SID authentication feature."
> 
>  #string STR_MAIN_GOTO_DISK_INFO_0#language en-US " "
>  #string STR_MAIN_GOTO_DISK_INFO_1#language en-US " "
> @@ -36,13 +36,14 @@
>  #string STR_MAIN_GOTO_DISK_INFO_4#language en-US " "
>  #string 

Re: [edk2] [edk2-announce] February Community Meeting

[stephano]

On 1/31/2019 8:49 AM, stephano wrote:
We will be holding our February Community Meeting on the *2nd* Thursday 
of February to accommodate holiday schedules. Details are posted here:


Sorry for the last minute notice here, but I'm going to move the APAC 
meeting for February from Thursday night PST (aka Valentine's Day) to 
Tuesday night PST (aka the day *after* Presidents Day).


Folks in EMEA, please let me know if you want me to move your meeting as 
well. It's currently scheduled for 17:00 GMT, so I will leave it to the 
community if that is too late in the day for work-chat.


My apologies for the confusion as I navigate the holiday-land-mine-field 
that is February.


Kind Regards,
Stephano


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

Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function

[Gao, Liming]

Bob:
  So, this is OS issue. I prefer to update wiki page to describe how to resolve 
it. 

Thanks
Liming
>-Original Message-
>From: Feng, Bob C
>Sent: Thursday, February 14, 2019 10:51 AM
>To: Laszlo Ersek ; Gao, Liming ;
>Bi, Dandan ; Carsey, Jaben 
>Cc: edk2-devel@lists.01.org
>Subject: RE: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function
>
>Hi Laszlo, Liming, Jaben and Dandan,
>
>I found this is a Ubuntu18 bug. Refer to
>https://bugs.launchpad.net/ubuntu/+source/fdroidserver/+bug/1762183
>
>And Ubuntu fixed this bug via a Ubuntu18.04.1 update package which was
>published on 2018-08-09. Refer to
>https://launchpad.net/ubuntu/+source/fdroidserver/1.0.9-1~18.04.1
>
>While the latest Ubuntu 18.04 release (ubuntu-18.04.1-desktop-amd64.iso)
>on http://releases.ubuntu.com/18.04/ was published on 2018-07-25.  So there
>is no distutils.util library on Ubuntu18.04 default installation. But I think 
>it's
>clear that distutils.util is not *3rd party* python library.
>
>I have tried that the command "sudo apt upgrade"  can't fix this bug while the
>command "sudo apt-get install python3-distutils" works.
>
>Thanks,
>Bob
>
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Tuesday, February 12, 2019 10:02 PM
>To: Gao, Liming ; Feng, Bob C
>; Bi, Dandan 
>Cc: edk2-devel@lists.01.org
>Subject: Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function
>
>On 02/12/19 14:33, Gao, Liming wrote:
>> Laszlo:
>>  To install python3-distutils should resolve this issue. I expect BaseTools
>build functionality doesn't depend on the third party python lib.
>
>I completely agree with your expectation, regarding *3rd party* python
>packages. We shouldn't expect developers to install packages from
>repositories that fall outside of their normal distro repos.
>
>However, my understanding was that python3-distutils should be available as
>a normal (not 3rd party) component on Ubuntu 18. I think we can expect
>developers to install additional packages if those packages are readily 
>available
>in their normal (distro-provided) repos.
>
>> So, I suggest to check whether python3-distutils is the native python 
>> library.
>If it is native python library, why Ubuntu18 doesn't include it. I will work 
>with
>Dandan to collect more information.
>
>Right, that's exactly what I'm asking for. Thank you very much!
>Laszlo
>
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Tuesday, February 12, 2019 8:24 PM
>>> To: Feng, Bob C ; Bi, Dandan
>>> 
>>> Cc: edk2-devel@lists.01.org; Gao, Liming 
>>> Subject: Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted
>>> function
>>>
>>> On 02/04/19 20:12, Laszlo Ersek wrote:
 On 02/03/19 06:55, Feng, Bob C wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> On some Linux environment, there may be no distutils.util library
> for python3 that will cause build tool crash.
> This patch implement distutils.util.split_quoted in BaseTools so
> that the Basetools will be independent with distutils.util library.
>
> Feng, Bob C (3):
>   BaseTools: Implement splitquoted function in Build tool
>   BaseTools: Implement splitquoted function in UPT
>   BaseTools: unit test for splitquoted function
>
>  BaseTools/Source/Python/AutoGen/UniClassObject.py | 50
>++
>  BaseTools/Source/Python/UPT/Library/UniClassObject.py | 47
>---
>  BaseTools/Tests/TestStringSplit.py| 38
>++
>  3 files changed, 128 insertions(+), 7 deletions(-)  create mode
> 100644 BaseTools/Tests/TestStringSplit.py
>

 Is this really necessary? BZ#1509 references Ubuntu18; however it
 looks like the issue can be resolved by a simple package
 installation on Ubuntu 18:

 https://superuser.com/questions/1319047/cant-install-virtual-interpr
 eter-in-pycharm-in-linux

 """
 sudo apt-get install python3-distutils """

 I'm not a Ubuntu user myself; so all I can do here (without
 installing a
 Ubuntu18 VM) is check the Ubuntu package directory:

 https://packages.ubuntu.com/search?keywords=python3-
>distutils
 on=names=all=all

 python3-distutils appears available for both "bionic (18.04LTS)" and
 "cosmic (18.10)".

 Dandan, if you install python3-distutils, does that solve the issue for 
 you?
>>>
>>> I'd still like to get an answer to my question, before the series is pushed.
>>>
>>> Thanks,
>>> Laszlo

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

[edk2] [PATCH] ShellPkg/Application/Shell: add array index check for shell delay option

[Zhichao Gao]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1528
Shell delay option without parameters do not check the
index of shell parameter argv. Add index check to avoid
invalid pointer references.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao 

Cc: Liming Gao 
Cc: Ray Ni 
---
 ShellPkg/Application/Shell/Shell.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index 104f4c8961..f4d9668d81 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1002,7 +1002,11 @@ ProcessCommandLine(
  ) == 0) {
   ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay= TRUE;
   // Check for optional delay value following "-delay"
-  DelayValueStr = gEfiShellParametersProtocol->Argv[LoopVar + 1];
+  if ((LoopVar + 1) >= gEfiShellParametersProtocol->Argc) {
+DelayValueStr = NULL;
+  } else {
+DelayValueStr = gEfiShellParametersProtocol->Argv[LoopVar + 1];
+  }
   if (DelayValueStr != NULL){
 if (*DelayValueStr == L':') {
   DelayValueStr++;
-- 
2.16.2.windows.1

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

Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function

[Feng, Bob C]

Hi Laszlo, Liming, Jaben and Dandan,

I found this is a Ubuntu18 bug. Refer to 
https://bugs.launchpad.net/ubuntu/+source/fdroidserver/+bug/1762183

And Ubuntu fixed this bug via a Ubuntu18.04.1 update package which was 
published on 2018-08-09. Refer to 
https://launchpad.net/ubuntu/+source/fdroidserver/1.0.9-1~18.04.1

While the latest Ubuntu 18.04 release (ubuntu-18.04.1-desktop-amd64.iso)  on 
http://releases.ubuntu.com/18.04/ was published on 2018-07-25.  So there is no 
distutils.util library on Ubuntu18.04 default installation. But I think it's 
clear that distutils.util is not *3rd party* python library. 

I have tried that the command "sudo apt upgrade"  can't fix this bug while the 
command "sudo apt-get install python3-distutils" works.

Thanks,
Bob

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, February 12, 2019 10:02 PM
To: Gao, Liming ; Feng, Bob C ; Bi, 
Dandan 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function

On 02/12/19 14:33, Gao, Liming wrote:
> Laszlo:
>  To install python3-distutils should resolve this issue. I expect BaseTools 
> build functionality doesn't depend on the third party python lib.

I completely agree with your expectation, regarding *3rd party* python 
packages. We shouldn't expect developers to install packages from repositories 
that fall outside of their normal distro repos.

However, my understanding was that python3-distutils should be available as a 
normal (not 3rd party) component on Ubuntu 18. I think we can expect developers 
to install additional packages if those packages are readily available in their 
normal (distro-provided) repos.

> So, I suggest to check whether python3-distutils is the native python 
> library. If it is native python library, why Ubuntu18 doesn't include it. I 
> will work with Dandan to collect more information. 

Right, that's exactly what I'm asking for. Thank you very much!
Laszlo

>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, February 12, 2019 8:24 PM
>> To: Feng, Bob C ; Bi, Dandan 
>> 
>> Cc: edk2-devel@lists.01.org; Gao, Liming 
>> Subject: Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted 
>> function
>>
>> On 02/04/19 20:12, Laszlo Ersek wrote:
>>> On 02/03/19 06:55, Feng, Bob C wrote:
 BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1509
 On some Linux environment, there may be no distutils.util library 
 for python3 that will cause build tool crash.
 This patch implement distutils.util.split_quoted in BaseTools so 
 that the Basetools will be independent with distutils.util library.

 Feng, Bob C (3):
   BaseTools: Implement splitquoted function in Build tool
   BaseTools: Implement splitquoted function in UPT
   BaseTools: unit test for splitquoted function

  BaseTools/Source/Python/AutoGen/UniClassObject.py | 50 
 ++
  BaseTools/Source/Python/UPT/Library/UniClassObject.py | 47 
 ---
  BaseTools/Tests/TestStringSplit.py| 38 
 ++
  3 files changed, 128 insertions(+), 7 deletions(-)  create mode 
 100644 BaseTools/Tests/TestStringSplit.py

>>>
>>> Is this really necessary? BZ#1509 references Ubuntu18; however it 
>>> looks like the issue can be resolved by a simple package 
>>> installation on Ubuntu 18:
>>>
>>> https://superuser.com/questions/1319047/cant-install-virtual-interpr
>>> eter-in-pycharm-in-linux
>>>
>>> """
>>> sudo apt-get install python3-distutils """
>>>
>>> I'm not a Ubuntu user myself; so all I can do here (without 
>>> installing a
>>> Ubuntu18 VM) is check the Ubuntu package directory:
>>>
>>> https://packages.ubuntu.com/search?keywords=python3-distutils
>>> on=names=all=all
>>>
>>> python3-distutils appears available for both "bionic (18.04LTS)" and 
>>> "cosmic (18.10)".
>>>
>>> Dandan, if you install python3-distutils, does that solve the issue for you?
>>
>> I'd still like to get an answer to my question, before the series is pushed.
>>
>> Thanks,
>> Laszlo

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

[edk2] [PATCH] MdePkg/BaseLib: Change a variable type in a bitwise operation

[Shenglei Zhang]

Change the type of variable Chr from CHAR8 to UINT32 in a
bitwise operation, to make the two variables in the operation
have the same size.
https://bugzilla.tianocore.org/show_bug.cgi?id=1527

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang 
---
 MdePkg/Library/BaseLib/String.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
index 53ff730e9e..a389115d71 100644
--- a/MdePkg/Library/BaseLib/String.c
+++ b/MdePkg/Library/BaseLib/String.c
@@ -2070,7 +2070,7 @@ Base64Decode (
   Chr = DecodingTable[(UINT8) Source[SourceIndex++]];
   } while (Chr == BAD_V);
   Value <<= 6;
-  Value |= Chr;
+  Value |= (UINT32)Chr;
 }
 
 //
-- 
2.18.0.windows.1

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

Re: [edk2] [patch V2] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible

[Bi, Dandan]

This patch is also available at
 
https://github.com/dandanbi/edk2/commit/e5c432b2d5083c0bbadbf773d460a9c22d36b267


Thanks,
Dandan
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Thursday, February 14, 2019 10:39 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Michael Turner
> ; Max Knutsen
> ; Gao, Liming 
> Subject: [edk2] [patch V2] MdeModulePkg/ReportStatusCodeLib: Avoid
> using AllocatePool if possible
> 
> From: Max Knutsen 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
> 
> V2:  simplify the code logic.
> update
> if (!mHaveExitedBootServices &&
>   (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
>   gBS->FreePool (StatusCodeData);
> }
> to
> if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
>   gBS->FreePool (StatusCodeData);
> }
> 
> When report status code with ExtendedData data, and the extended data
> can fit in the local static buffer, there is no need to use AllocatePool to 
> hold
> the ExtendedData data.
> 
> This patch is just to do the enhancement to avoid using AllocatePool.
> 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Cc: Michael Turner 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  .../Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c | 9 +++--
>   .../RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c  | 9 +++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> index b28dc5c3bb..c88be5a1a3 100644
> ---
> a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> +++
> b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> @@ -505,13 +505,18 @@ ReportStatusCodeEx (
>//
>Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>gBS->RestoreTPL (Tpl);
> 
>StatusCodeData = NULL;
> -  if (Tpl <= TPL_NOTIFY) {
> +  if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof
> + (EFI_STATUS_CODE_DATA))) {
>  //
> -// Allocate space for the Status Code Header and its buffer
> +// Use Buffer instead of allocating if possible.
> +//
> +StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;  } else if (Tpl <=
> + TPL_NOTIFY){
> +//
> +// If Buffer is not big enough, allocate space for the Status Code
> + Header and its buffer
>  //
>  gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA)
> + ExtendedDataSize, (VOID **));
>}
> 
>if (StatusCodeData == NULL) {
> diff --git
> a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> odeLib.c
> b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> odeLib.c
> index b73103517a..75e2f88ad2 100644
> ---
> a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> odeLib.c
> +++
> b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> od
> +++ eLib.c
> @@ -635,11 +635,14 @@ ReportStatusCodeEx (
>if (mHaveExitedBootServices) {
>  if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize >
> MAX_EXTENDED_DATA_SIZE) {
>return EFI_OUT_OF_RESOURCES;
>  }
>  StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
> -  } else  {
> +  } else if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize >
> MAX_EXTENDED_DATA_SIZE) {
> +//
> +// Only use AllocatePool for larger ExtendedData.
> +//
>  if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
>return EFI_UNSUPPORTED;
>  }
> 
>  //
> @@ -648,10 +651,12 @@ ReportStatusCodeEx (
>  StatusCodeData = NULL;
>  gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA)
> + ExtendedDataSize, (VOID **));
>  if (StatusCodeData == NULL) {
>return EFI_OUT_OF_RESOURCES;
>  }
> +  } else {
> +StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
>}
> 
>//
>// Fill in the extended data header
>//
> @@ -678,11 +683,11 @@ ReportStatusCodeEx (
>Status = InternalReportStatusCode (Type, Value, Instance, CallerId,
> StatusCodeData);
> 
>//
>// Free the allocated buffer
>//
> -  if (!mHaveExitedBootServices) {
> +  if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
>  gBS->FreePool (StatusCodeData);
>}
> 
>return Status;
>  }
> --
> 2.18.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 V2] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible

[Dandan Bi]

From: Max Knutsen 

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

V2:  simplify the code logic.
update
if (!mHaveExitedBootServices &&
  (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
  gBS->FreePool (StatusCodeData);
}
to
if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
  gBS->FreePool (StatusCodeData);
}

When report status code with ExtendedData data,
and the extended data can fit in the local static buffer,
there is no need to use AllocatePool to hold the ExtendedData data.

This patch is just to do the enhancement to avoid using AllocatePool.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Michael Turner 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c | 9 +++--
 .../RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c  | 9 +++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c 
b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
index b28dc5c3bb..c88be5a1a3 100644
--- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -505,13 +505,18 @@ ReportStatusCodeEx (
   //
   Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
   gBS->RestoreTPL (Tpl);
 
   StatusCodeData = NULL;
-  if (Tpl <= TPL_NOTIFY) {
+  if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof 
(EFI_STATUS_CODE_DATA))) {
 //
-// Allocate space for the Status Code Header and its buffer
+// Use Buffer instead of allocating if possible.
+//
+StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
+  } else if (Tpl <= TPL_NOTIFY){
+//
+// If Buffer is not big enough, allocate space for the Status Code Header 
and its buffer
 //
 gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + 
ExtendedDataSize, (VOID **));
   }
 
   if (StatusCodeData == NULL) {
diff --git 
a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c 
b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
index b73103517a..75e2f88ad2 100644
--- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -635,11 +635,14 @@ ReportStatusCodeEx (
   if (mHaveExitedBootServices) {
 if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > 
MAX_EXTENDED_DATA_SIZE) {
   return EFI_OUT_OF_RESOURCES;
 }
 StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
-  } else  {
+  } else if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > 
MAX_EXTENDED_DATA_SIZE) {
+//
+// Only use AllocatePool for larger ExtendedData.
+//
 if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
   return EFI_UNSUPPORTED;
 }
 
 //
@@ -648,10 +651,12 @@ ReportStatusCodeEx (
 StatusCodeData = NULL;
 gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + 
ExtendedDataSize, (VOID **));
 if (StatusCodeData == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
+  } else {
+StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
   }
 
   //
   // Fill in the extended data header
   //
@@ -678,11 +683,11 @@ ReportStatusCodeEx (
   Status = InternalReportStatusCode (Type, Value, Instance, CallerId, 
StatusCodeData);
 
   //
   // Free the allocated buffer
   //
-  if (!mHaveExitedBootServices) {
+  if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
 gBS->FreePool (StatusCodeData);
   }
 
   return Status;
 }
-- 
2.18.0.windows.1

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

Re: [edk2] [Patch 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD PcdCpuFeaturesUserConfiguration.

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, February 13, 2019 10:44 AM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ray 
> Subject: Re: [Patch 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD
> PcdCpuFeaturesUserConfiguration.
> 
> On 02/13/19 03:04, Eric Dong wrote:
> > In current implementation, PCD PcdCpuFeaturesUserConfiguration used as
> > user enabled CPU features list. It is initialzied in platform driver
> > and as an input for CpuFeatures driver. PCD PcdCpuFeaturesSetting used
> > as an output for the final enabled CPU features list. For now,
> > PcdCpuFeaturesUserConfiguration is only used as an input and
> > PcdCpuFeaturesSetting only used as an output.
> >
> > This change merge PcdCpuFeaturesUserConfiguration into
> > PcdCpuFeaturesSetting.
> > Use PcdCpuFeaturesSetting as input for the user input feature setting
> > Use PcdCpuFeaturesSetting as output for the final CPU feature setting
> >
> > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 37 
> --
> >  .../DxeRegisterCpuFeaturesLib.inf  |  1 -
> >  .../PeiRegisterCpuFeaturesLib.inf  |  1 -
> >  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
> >  UefiCpuPkg/UefiCpuPkg.dec  |  7 ++--
> >  5 files changed, 22 insertions(+), 25 deletions(-)
> >
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > index bae92b89a6..4ebd0025b4 100644
> > ---
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
> > +++ c
> > @@ -54,41 +54,41 @@ SetSettingPcd (
> >  }
> >
> >  /**
> > -  Worker function to get PcdCpuFeaturesSupport.
> > +  Worker function to get PcdCpuFeaturesSetting.
> >
> >@return  The pointer to CPU feature bits mask buffer.
> >  **/
> >  UINT8 *
> > -GetSupportPcd (
> > +GetSettingPcd (
> >VOID
> >)
> >  {
> > -  UINT8  *SupportBitMask;
> > +  UINT8  *SettingBitMask;
> >
> > -  SupportBitMask = AllocateCopyPool (
> > -  PcdGetSize (PcdCpuFeaturesSupport),
> > -  PcdGetPtr (PcdCpuFeaturesSupport)
> > +  SettingBitMask = AllocateCopyPool (
> > +  PcdGetSize (PcdCpuFeaturesSetting),
> > +  PcdGetPtr (PcdCpuFeaturesSetting)
> >);
> 
> (1) The indentation of the AllocateCopyPool() arguments is not idiomatic. I
> suggest fixing that, even if it means diverging from the old code.

Agree, will include this change in next version patches.

> 
> > -  ASSERT (SupportBitMask != NULL);
> > +  ASSERT (SettingBitMask != NULL);
> >
> > -  return SupportBitMask;
> > +  return SettingBitMask;
> >  }
> >
> >  /**
> > -  Worker function to get PcdCpuFeaturesUserConfiguration.
> > +  Worker function to get PcdCpuFeaturesSupport.
> >
> >@return  The pointer to CPU feature bits mask buffer.
> >  **/
> >  UINT8 *
> > -GetConfigurationPcd (
> > +GetSupportPcd (
> >VOID
> >)
> >  {
> >UINT8  *SupportBitMask;
> >
> >SupportBitMask = AllocateCopyPool (
> > -  PcdGetSize (PcdCpuFeaturesUserConfiguration),
> > -  PcdGetPtr (PcdCpuFeaturesUserConfiguration)
> > +  PcdGetSize (PcdCpuFeaturesSupport),
> > +  PcdGetPtr (PcdCpuFeaturesSupport)
> >);
> >ASSERT (SupportBitMask != NULL);
> >
> > @@ -287,7 +287,6 @@ CpuInitDataInitialize (
> >// Get support and configuration PCDs
> >//
> >CpuFeaturesData->SupportPcd   = GetSupportPcd ();
> > -  CpuFeaturesData->ConfigurationPcd = GetConfigurationPcd ();  }
> >
> >  /**
> > @@ -595,6 +594,9 @@ AnalysisProcessorFeatures (
> >CPU_FEATURE_DEPENDENCE_TYPE  AfterDep;
> >CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibBeforeDep;
> >CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibAfterDep;
> > +  UINT8*ConfigurationPcd;
> > +
> > +  ConfigurationPcd = NULL;
> >
> >CpuFeaturesData = GetCpuFeaturesData ();
> >CpuFeaturesData->CapabilityPcd = AllocatePool
> > (CpuFeaturesData->BitMaskSize); @@ -610,10 +612,13 @@
> AnalysisProcessorFeatures (
> >//
> >// Calculate the last setting
> >//
> > -
> >CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData-
> >BitMaskSize, CpuFeaturesData->CapabilityPcd);
> >ASSERT (CpuFeaturesData->SettingPcd != NULL);
> > -  SupportedMaskAnd (CpuFeaturesData->SettingPcd,
> > CpuFeaturesData->ConfigurationPcd);
> > +  ConfigurationPcd = GetSettingPcd ();  SupportedMaskAnd
> > + (CpuFeaturesData->SettingPcd, ConfigurationPcd);  if
> > + (ConfigurationPcd != NULL) {
> > +FreePool 

Re: [edk2] [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, February 13, 2019 11:01 AM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ray 
> Subject: Re: [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify
> PcdCpuFeaturesSupport.
> 
> On 02/13/19 03:04, Eric Dong wrote:
> > PcdCpuFeaturesSupport used to specify the platform policy about what
> > CPU features this platform supports. This value is decide by platform
> > owner, not by hardware. After this change, this PCD will be used in
> > IsCpuFeatureSupported function only.
> >
> > Now RegisterCpuFeaturesLib use this PCD as an template to Get the pcd
> > size. Update the code logic to replace it with PcdCpuFeaturesSetting.
> >
> > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 66 +++
> ---
> >  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
> >  .../RegisterCpuFeaturesLib.c   | 10 ++--
> >  3 files changed, 24 insertions(+), 53 deletions(-)
> >
> > diff --git
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > index 4ebd0025b4..762eaec277 100644
> > ---
> > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.
> > +++ c
> > @@ -74,27 +74,6 @@ GetSettingPcd (
> >return SettingBitMask;
> >  }
> >
> > -/**
> > -  Worker function to get PcdCpuFeaturesSupport.
> > -
> > -  @return  The pointer to CPU feature bits mask buffer.
> > -**/
> > -UINT8 *
> > -GetSupportPcd (
> > -  VOID
> > -  )
> > -{
> > -  UINT8  *SupportBitMask;
> > -
> > -  SupportBitMask = AllocateCopyPool (
> > -  PcdGetSize (PcdCpuFeaturesSupport),
> > -  PcdGetPtr (PcdCpuFeaturesSupport)
> > -  );
> > -  ASSERT (SupportBitMask != NULL);
> > -
> > -  return SupportBitMask;
> > -}
> > -
> >  /**
> >Collects CPU type and feature information.
> >
> > @@ -282,11 +261,6 @@ CpuInitDataInitialize (
> >ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
> >CpuFeaturesData->CpuFlags.PackageSemaphoreCount =
> AllocateZeroPool (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus-
> >MaxCoreCount * CpuStatus->MaxThreadCount);
> >ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
> > -
> > -  //
> > -  // Get support and configuration PCDs
> > -  //
> > -  CpuFeaturesData->SupportPcd   = GetSupportPcd ();
> >  }
> >
> >  /**
> > @@ -306,7 +280,7 @@ SupportedMaskOr (
> >UINT8  *Data1;
> >UINT8  *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> > +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >Data1 = SupportedFeatureMask;
> >Data2 = OrFeatureBitMask;
> >for (Index = 0; Index < BitMaskSize; Index++) { @@ -331,7 +305,7 @@
> > SupportedMaskAnd (
> >UINT8  *Data1;
> >UINT8  *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> > +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >Data1 = SupportedFeatureMask;
> >Data2 = AndFeatureBitMask;
> >for (Index = 0; Index < BitMaskSize; Index++) { @@ -356,7 +330,7 @@
> > SupportedMaskCleanBit (
> >UINT8  *Data1;
> >UINT8  *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> > +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >Data1 = SupportedFeatureMask;
> >Data2 = AndFeatureBitMask;
> >for (Index = 0; Index < BitMaskSize; Index++) { @@ -387,7 +361,7 @@
> > IsBitMaskMatch (
> >UINT8  *Data1;
> >UINT8  *Data2;
> >
> > -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> > +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> >
> >Data1 = SupportedFeatureMask;
> >Data2 = ComparedFeatureBitMask;
> > @@ -426,22 +400,22 @@ CollectProcessorData (
> >Entry = GetFirstNode (>FeatureList);
> >while (!IsNull (>FeatureList, Entry)) {
> >  CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> > -if (IsBitMaskMatch (CpuFeaturesData->SupportPcd, CpuFeature-
> >FeatureMask)) {
> > -  if (CpuFeature->SupportFunc == NULL) {
> > -//
> > -// If SupportFunc is NULL, then the feature is supported.
> > -//
> > -SupportedMaskOr (
> > -  CpuFeaturesData-
> >InitOrder[ProcessorNumber].FeaturesSupportedMask,
> > -  CpuFeature->FeatureMask
> > -  );
> > -  } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo,
> CpuFeature->ConfigData)) {
> > -SupportedMaskOr (
> > -  CpuFeaturesData-
> >InitOrder[ProcessorNumber].FeaturesSupportedMask,
> > -  

Re: [edk2] [PATCH v2] NetworkPkg/Ip6Dxe: Clean the invalid IPv6 configuration during driver start.

[Wu, Jiaxin]

Hi Siyuan,

Both of them should be fine to clear the invalid configuration data. In my 
opinion, since the error returned from SetData(), we will treat it as invalid 
except the asynchronous process (EFI_NOT_READY). That's already have been 
checked in the if condition.

Thanks,
Jiaxin


> -Original Message-
> From: Fu, Siyuan
> Sent: Tuesday, February 12, 2019 9:05 AM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Michael Turner ; Ye, Ting
> 
> Subject: RE: [PATCH v2] NetworkPkg/Ip6Dxe: Clean the invalid IPv6
> configuration during driver start.
> 
> Hi, Jiaxin
> 
> I think the Ip6Cfg->SetData() may return other error, which not only because
> invalid config data is used. Why not just check the config data in
> Ip6ConfigReadConfigData()?
> 
> BestRegards
> Fu Siyuan
> 
> 
> > -Original Message-
> > From: Wu, Jiaxin
> > Sent: Tuesday, February 12, 2019 8:47 AM
> > To: edk2-devel@lists.01.org
> > Cc: Michael Turner ; Ye, Ting
> > ; Fu, Siyuan ; Wu, Jiaxin
> > 
> > Subject: [PATCH v2] NetworkPkg/Ip6Dxe: Clean the invalid IPv6
> configuration
> > during driver start.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1448
> >
> > *v2: Add the warning debug message.
> >
> > This patch is to clean the invalid data and continue to start IP6 driver.
> >
> > Cc: Michael Turner 
> > Cc: Ye Ting 
> > Cc: Fu Siyuan 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wu Jiaxin 
> > ---
> >  NetworkPkg/Ip6Dxe/Ip6Driver.c | 22 --
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/NetworkPkg/Ip6Dxe/Ip6Driver.c
> b/NetworkPkg/Ip6Dxe/Ip6Driver.c
> > index 4c607125a6..7ec74f6ebc 100644
> > --- a/NetworkPkg/Ip6Dxe/Ip6Driver.c
> > +++ b/NetworkPkg/Ip6Dxe/Ip6Driver.c
> > @@ -586,11 +586,20 @@ Ip6DriverBindingStart (
> > Ip6ConfigDataTypeManualAddress,
> > DataItem->DataSize,
> > DataItem->Data.Ptr
> > );
> >  if (EFI_ERROR(Status) && Status != EFI_NOT_READY) {
> > -  goto UNINSTALL_PROTOCOL;
> > +  //
> > +  // Clean the invalid ManualAddress configuration.
> > +  //
> > +  Status = Ip6Cfg->SetData (
> > + Ip6Cfg,
> > + Ip6ConfigDataTypeManualAddress,
> > + 0,
> > + NULL
> > + );
> > +  DEBUG ((EFI_D_WARN, "Ip6DriverBindingStart: Clean the invalid
> > ManualAddress configuration.\n"));
> >  }
> >}
> >
> >//
> >// If there is any default gateway address, set it.
> > @@ -602,11 +611,20 @@ Ip6DriverBindingStart (
> > Ip6ConfigDataTypeGateway,
> > DataItem->DataSize,
> > DataItem->Data.Ptr
> > );
> >  if (EFI_ERROR(Status)) {
> > -  goto UNINSTALL_PROTOCOL;
> > +  //
> > +  // Clean the invalid Gateway configuration.
> > +  //
> > +  Status = Ip6Cfg->SetData (
> > + Ip6Cfg,
> > + Ip6ConfigDataTypeGateway,
> > + 0,
> > + NULL
> > + );
> > +  DEBUG ((EFI_D_WARN, "Ip6DriverBindingStart: Clean the invalid
> Gateway
> > configuration.\n"));
> >  }
> >}
> >
> >//
> >// ready to go: start the receiving and timer
> > --
> > 2.17.1.windows.2

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

Re: [edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

[Laszlo Ersek]

On 02/13/19 18:00, Philippe Mathieu-Daudé wrote:
> On 2/13/19 9:37 AM, Laszlo Ersek wrote:
>>
>> using QEMU, when I specify a nonzero LUN for the hard disk that sits on
>> the "SCSI bus" that embodies the USB Bulk Only Transfer device, then
>> UsbMassStorageDxe fails to recognize the hard disk.
>>
>> (1) Consider the following QEMU command line snippet:
>>
>>   -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
>>   -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
>>   -device usb-bot,bus=xhci1.0,port=3,id=bot1 \
> 
> Do you have a specific need to use the 'usb-bot' device?

Nothing beyond .

>>   -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \

[...]

>> In this case, edk2 recognizes the disk and things work fine.
>>
>> (In fact, for lun=0, the QemuBootOrderLib pattern matching / translation
>> works fine as well -- verifying which was my original goal, before I ran
>> into the issues below, for nonzero LUNs. But, I digress.)
>>
>>
>> (2) If I change the cmdline to "lun=5", then the exchange is:
> 
> From qemu/docs/usb-storage.txt:
> 
>   The LUN numbers must be continuous, i.e. for three devices you must
>   use 0+1+2. The 0+1+5 numbering from the "usb-uas" example isn't going
>   to work with "usb-bot".
> 
> A failure is expected :/

OK, that explains the issue in (2). Wrong config. Thanks!

[...]

>> (3) Starting again from the original command line, if I change "lun=0"
>> to "lun=1" (rather than to "lun=5"), then OVMF even hangs, with the
>> following log:

[...]

>>> ASSERT MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c(1915): TrsRing != ((void *) 
>>> 0)
>>
>> In this case, edk2 seems to recognize that a nonzero LUN is available on
>> the target, but the initialization never completes, and then an
>> assertion fails, apparently in the lower-level XHCI transport code.
> 
> Can you try using the 'usb-uas' device instead of the 'usb-bot'?

Thanks, but no, thanks. :)

For USB storage options, I prefer the absolute minimum. I thought that
usb-storage was the end of the story -- it works perfectly fine; please
see the scope in:
- https://bugzilla.redhat.com/show_bug.cgi?id=1458192
- https://github.com/tianocore/edk2/commit/f9c59fa44ae2

Due to , usb-bot now
looks relevant as well. I'm trying to see how that maps to the existent
usb-storage support code, and what extensions if any are needed.

"usb-uas" remains totally out of scope though.

--*--

Anyway, now I realize that my test (3) was invalid too, because, by
*changing* lun0 to lun1 (rather than adding lun1 after lun0), I again
created a discontiguous LUN space.

(4) Unfortunately, the same assertion failure hits in edk2, even if I
add *both* lun0 and lun1:

  -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
  -drive id=disk2,if=none,format=raw,readonly,file=$APPDISK \
  -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
  -device usb-bot,bus=xhci1.0,port=4,id=bot1 \
  -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \
  -device scsi-hd,drive=disk2,bus=bot1.0,lun=1,bootindex=2 \

Based on the last paragraphs in "docs/usb-storage.txt" (specifically
step (2b)), I'd expect this to work -- do you agree?

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

Re: [edk2] [PATCH v3 00/10] Remove unused tool chain configuration

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhang, Shenglei
>Sent: Wednesday, February 13, 2019 9:43 AM
>To: edk2-devel@lists.01.org
>Cc: Feng, Bob C ; Gao, Liming
>; Zhu, Yonghong ; Ni, Ray
>; Justen, Jordan L ; Laszlo
>Ersek ; Ard Biesheuvel ;
>Anthony Perard ; Julien Grall
>
>Subject: [PATCH v3 00/10] Remove unused tool chain configuration
>
>VS2003, VS2005, DDK3790, UNIXGCC, ELFGCC, CYGCC and MYTOOLS are
>too old. There is no verification for them. So remove them from
>edk2/master.
>https://bugzilla.tianocore.org/show_bug.cgi?id=1377
>
>v2:1.Combine previous 05/10 and 06/10 to 05/10.
>   2.Add 10/10(Remove GCCLD).
>
>v3:1.Change order of patch series because of bisectability.
>   2.Make changes in 04/10 and 09/10.
>
>Cc: Bob Feng 
>Cc: Liming Gao 
>Cc: Yonghong Zhu 
>Cc: Ray Ni 
>Cc: Jordan Justen 
>Cc: Laszlo Ersek 
>Cc: Ard Biesheuvel 
>Cc: Anthony Perard 
>Cc: Julien Grall 
>Shenglei Zhang (10):
>  BaseTools/tools_def.template: Remove CYGGCC
>  OptionRomPkg/ReadMe.txt: Remove CYGGCC
>  BaseTools: Update MYTOOLS
>  BaseTools/tools_def.template: Remove VS2003 and VS2005
>  OptionRomPkg/ReadMe.txt: Remove VS2005
>  BaseTools/tools_def.template: Remove UNIXGCC
>  OvmfPkg/README: Remove UNIXGCC
>  BaseTools/tools_def.template: Remove ELFGCC
>  BaseTools/tools_def.template: Remove DDK3790
>  BaseTools/build_rule.template: Remove GCCLD
>
> BaseTools/Conf/build_rule.template |   33 +-
> BaseTools/Conf/target.template |2 +-
> BaseTools/Conf/tools_def.template  | 1421 +---
> OptionRomPkg/ReadMe.txt|2 -
> OvmfPkg/OvmfPkgIa32.dsc|1 -
> OvmfPkg/OvmfPkgIa32X64.dsc |1 -
> OvmfPkg/OvmfPkgX64.dsc |1 -
> OvmfPkg/README |   19 -
> 8 files changed, 18 insertions(+), 1462 deletions(-)
>
>--
>2.18.0.windows.1

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

Re: [edk2] [PATCH 3/3] MdeModulePkg/SmmS3SaveStateDxe: Change function parameter types

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Shenglei Zhang
>Sent: Wednesday, February 13, 2019 2:52 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A ; Zeng, Star 
>Subject: [edk2] [PATCH 3/3] MdeModulePkg/SmmS3SaveStateDxe: Change
>function parameter types
>
>Change parameter Opcode from UINT16 to UINTN in
>BootScriptWrite and BootScriptInsert.
>https://bugzilla.tianocore.org/show_bug.cgi?id=1517
>
>Cc: Jian J Wang 
>Cc: Hao Wu 
>Cc: Ray Ni 
>Cc: Star Zeng 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Shenglei Zhang 
>---
> .../Universal/Acpi/SmmS3SaveState/InternalSmmSaveState.h  | 4 ++--
> MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c   | 4
>++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git
>a/MdeModulePkg/Universal/Acpi/SmmS3SaveState/InternalSmmSaveState.
>h
>b/MdeModulePkg/Universal/Acpi/SmmS3SaveState/InternalSmmSaveState.
>h
>index 51cf9db4aa..440f43defe 100644
>---
>a/MdeModulePkg/Universal/Acpi/SmmS3SaveState/InternalSmmSaveState.
>h
>+++
>b/MdeModulePkg/Universal/Acpi/SmmS3SaveState/InternalSmmSaveState.
>h
>@@ -58,7 +58,7 @@ EFI_STATUS
> EFIAPI
> BootScriptWrite (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
>-  IN   UINT16   OpCode,
>+  IN   UINTNOpCode,
>   ...
>   );
> /**
>@@ -95,7 +95,7 @@ BootScriptInsert (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
>   IN   BOOLEAN  BeforeOrAfter,
>   IN OUT   EFI_S3_BOOT_SCRIPT_POSITION *Position OPTIONAL,
>-  IN   UINT16   OpCode,
>+  IN   UINTNOpCode,
>   ...
>   );
> /**
>diff --git
>a/MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c
>b/MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c
>index c1d29b5d3a..bd43011b79 100644
>--- a/MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c
>+++ b/MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c
>@@ -540,7 +540,7 @@ EFI_STATUS
> EFIAPI
> BootScriptWrite (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL *This,
>-  IN UINT16OpCode,
>+  IN UINTN OpCode,
>   ...
>   )
> {
>@@ -695,7 +695,7 @@ BootScriptInsert (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL*This,
>   IN   BOOLEAN  BeforeOrAfter,
>   IN OUT   EFI_S3_BOOT_SCRIPT_POSITION *Position OPTIONAL,
>-  IN   UINT16   OpCode,
>+  IN   UINTNOpCode,
>   ...
>   )
> {
>--
>2.18.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

Re: [edk2] [PATCH 2/3] MdeModulePkg/S3SaveStateDxe: Change function parameter types

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Shenglei Zhang
>Sent: Wednesday, February 13, 2019 2:52 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A ; Zeng, Star 
>Subject: [edk2] [PATCH 2/3] MdeModulePkg/S3SaveStateDxe: Change
>function parameter types
>
>Change parameter Opcode from UINT16 to UINTN in
>BootScriptWrite and BootScriptInsert.
>https://bugzilla.tianocore.org/show_bug.cgi?id=1517
>
>Cc: Jian J Wang 
>Cc: Hao Wu 
>Cc: Ray Ni 
>Cc: Star Zeng 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Shenglei Zhang 
>---
> .../Universal/Acpi/S3SaveStateDxe/InternalS3SaveState.h   | 4 ++--
> MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c  | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git
>a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/InternalS3SaveState.h
>b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/InternalS3SaveState.h
>index 19600085f1..b710919881 100644
>--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/InternalS3SaveState.h
>+++
>b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/InternalS3SaveState.h
>@@ -75,7 +75,7 @@ EFI_STATUS
> EFIAPI
> BootScriptWrite (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
>-  IN   UINT16   OpCode,
>+  IN   UINTNOpCode,
>   ...
>   );
> /**
>@@ -112,7 +112,7 @@ BootScriptInsert (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL*This,
>   IN   BOOLEAN  BeforeOrAfter,
>   IN OUT   EFI_S3_BOOT_SCRIPT_POSITION *Position OPTIONAL,
>-  IN   UINT16   OpCode,
>+  IN   UINTNOpCode,
>   ...
>   );
> /**
>diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c
>b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c
>index 274f3be12c..5913078d69 100644
>--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c
>+++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c
>@@ -542,7 +542,7 @@ EFI_STATUS
> EFIAPI
> BootScriptWrite (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
>-  IN   UINT16   OpCode,
>+  IN   UINTNOpCode,
>   ...
>   )
> {
>@@ -697,7 +697,7 @@ BootScriptInsert (
>   IN CONST EFI_S3_SAVE_STATE_PROTOCOL *This,
>   IN   BOOLEANBeforeOrAfter,
>   IN OUT   EFI_S3_BOOT_SCRIPT_POSITION*Position OPTIONAL,
>-  IN   UINT16 OpCode,
>+  IN   UINTN  OpCode,
>   ...
>   )
> {
>--
>2.18.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

Re: [edk2] [PATCH 1/3] MdePkg: Change structure parameter types

[Gao, Liming]

The change is good. Please update header comments to describe the change in PI 
1.7. 

With this change, Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhang, Shenglei
>Sent: Wednesday, February 13, 2019 2:52 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Gao, Liming
>
>Subject: [PATCH 1/3] MdePkg: Change structure parameter types
>
>Change parameter Opcode from UINT16 to UINTN
>in EFI_S3_SAVE_STATE_WRITE and EFI_S3_SAVE_STATE_INSERT.
>https://bugzilla.tianocore.org/show_bug.cgi?id=1517
>
>Cc: Michael D Kinney 
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Shenglei Zhang 
>---
> MdePkg/Include/Protocol/S3SaveState.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Include/Protocol/S3SaveState.h
>b/MdePkg/Include/Protocol/S3SaveState.h
>index 9e7b4050f6..1563527148 100644
>--- a/MdePkg/Include/Protocol/S3SaveState.h
>+++ b/MdePkg/Include/Protocol/S3SaveState.h
>@@ -52,7 +52,7 @@ typedef
> EFI_STATUS
> (EFIAPI *EFI_S3_SAVE_STATE_WRITE)(
>IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
>-   IN   UINT16  OpCode,
>+   IN   UINTN   OpCode,
>...
> );
>
>@@ -98,7 +98,7 @@ EFI_STATUS
>IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
>IN   BOOLEAN BeforeOrAfter,
>IN OUT   EFI_S3_BOOT_SCRIPT_POSITION *Position   OPTIONAL,
>-   IN   UINT16  OpCode,
>+   IN   UINTN   OpCode,
>...
> );
>
>--
>2.18.0.windows.1

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

Re: [edk2] [PATCH v2 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Zeng, Star
>Sent: Wednesday, February 13, 2019 5:50 PM
>To: Chiu, Chasel ; edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Gao, Liming
>; Zeng, Star 
>Subject: RE: [edk2] [PATCH v2 1/3] MdePkg: Support
>EFI_PEI_CORE_FV_LOCATION_PPI
>
>Reviewed-by: Star Zeng 
>
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Chasel, Chiu
>Sent: Wednesday, February 13, 2019 5:47 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Gao, Liming
>
>Subject: [edk2] [PATCH v2 1/3] MdePkg: Support
>EFI_PEI_CORE_FV_LOCATION_PPI
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
>
>Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on PI spec 1.7,
>Section 6.3.9.
>This PPI can support the secnario that PEI Foundation not in BFV.
>
>Cc: Michael D Kinney 
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Chasel Chiu 
>---
> MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53
>+
> MdePkg/MdePkg.dec  | 11 +--
> 2 files changed, 62 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h
>b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
>new file mode 100644
>index 00..c7bbbfb265
>--- /dev/null
>+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
>@@ -0,0 +1,53 @@
>+/** @file
>+  Header file for Pei Core FV Location PPI.
>+
>+  This PPI contains a pointer to the firmware volume which contains the PEI
>Foundation.
>+  If the PEI Foundation does not reside in the BFV, then SEC must pass
>+ this PPI as a part  of the PPI list provided to the PEI Foundation
>+ Entry Point, otherwise the PEI Foundation  shall assume that it resides 
>within
>the BFV.
>+
>+  Copyright (c) 2019, 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.
>+
>+  @par Revision Reference:
>+  This PPI is defined in UEFI Platform Initialization Specification 1.7 
>Volume 1:
>+  Standards
>+
>+**/
>+
>+
>+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
>+#define _EFI_PEI_CORE_FV_LOCATION_H_
>+
>+///
>+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI /// #define
>+EFI_PEI_CORE_FV_LOCATION_GUID \
>+  { \
>+0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0,
>+0xca, 0xf4 } \
>+  }
>+
>+///
>+/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI /// typedef
>+struct _EFI_PEI_CORE_FV_LOCATION_PPI
>EFI_PEI_CORE_FV_LOCATION_PPI;
>+
>+///
>+/// This PPI provides location of EFI PeiCoreFv.
>+///
>+struct _EFI_PEI_CORE_FV_LOCATION_PPI {
>+  ///
>+  /// Pointer to the first byte of the firmware volume which contains the PEI
>Foundation.
>+  ///
>+  VOID*PeiCoreFvLocation;
>+};
>+
>+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
>+
>+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
>diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
>a485408310..c859b4a511 100644
>--- a/MdePkg/MdePkg.dec
>+++ b/MdePkg/MdePkg.dec
>@@ -2,9 +2,9 @@
> # This Package provides all definitions, library classes and libraries 
> instances.
> #
> # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of -#
>EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
>+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
> #
>-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
>+# Copyright (c) 2007 - 2019, Intel Corporation. All rights
>+reserved.
> # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.  # 
> (C)
>Copyright 2016 Hewlett Packard Enterprise Development LP  # @@ -
>929,6 +929,13 @@
>   ## Include/Ppi/SecHobData.h
>   gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5,
>0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
>
>+  #
>+  # PPIs defined in PI 1.7.
>+  #
>+
>+  ## Include/Ppi/PeiCoreFvLocation.h
>+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f,
>0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
>+
> [Protocols]
>   ## Include/Protocol/Pcd.h
>   gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 
> 0x90, 0xD5,
>0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
>--
>2.13.3.windows.1
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] OVMF and TCP4, HTTP protocols

[Rebecca Cran via edk2-devel]

On 2/12/19 10:44 PM, Laszlo Ersek wrote:


In your case, I think it should suffice to call ConnectController() with
Recursive=FALSE, on the controller that already has SNP. The
HttpServiceBinding, Tcp6ServiceBinding and TCPv4ServiceBinding protocols
seem to be installed on the same handle. (The device path on this handle
ends with the MAC() node.)



Thanks again. Calling CloseProtocol, DisconnectController and then 
ConnectController got things working.



--

Rebecca Cran

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

Re: [edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

[Philippe Mathieu-Daudé]

Hi Laszlo,

On 2/13/19 9:37 AM, Laszlo Ersek wrote:
> Hi,
> 
> using QEMU, when I specify a nonzero LUN for the hard disk that sits on
> the "SCSI bus" that embodies the USB Bulk Only Transfer device, then
> UsbMassStorageDxe fails to recognize the hard disk.
> 
> (1) Consider the following QEMU command line snippet:
> 
>   -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
>   -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
>   -device usb-bot,bus=xhci1.0,port=3,id=bot1 \

Do you have a specific need to use the 'usb-bot' device?

>   -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \
> 
> For this (i.e., lun=0), I see the following INQUIRY exchange between
> UsbMassStorageDxe and QEMU:
> 
>> Lun=0 InquiryCmd {
>> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
>> Lun=0 InquiryCmd }
>> Lun=0 InquiryData {
>> Lun=0 InquiryData 00 00 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
>> Lun=0 InquiryData 10 51 45 4D 55 20 48 41 52 44 44 49 53 4B 20 20 20
>> Lun=0 InquiryData 20 32 2E 35 2B
>> Lun=0 InquiryData }
> 
> The vendor and product ID fields translate to "QEMU" and
> "QEMU_HARDDISK___", respectively. (For easier reading, I replaced the
> space characters with underscores, for the sake of better
> readability/wrapping.)
> 
> In this case, edk2 recognizes the disk and things work fine.
> 
> (In fact, for lun=0, the QemuBootOrderLib pattern matching / translation
> works fine as well -- verifying which was my original goal, before I ran
> into the issues below, for nonzero LUNs. But, I digress.)
> 
> 
> (2) If I change the cmdline to "lun=5", then the exchange is:

>From qemu/docs/usb-storage.txt:

  The LUN numbers must be continuous, i.e. for three devices you must
  use 0+1+2. The 0+1+5 numbering from the "usb-uas" example isn't going
  to work with "usb-bot".

A failure is expected :/

> 
>> Lun=0 InquiryCmd {
>> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
>> Lun=0 InquiryCmd }
>> Lun=0 InquiryData {
>> Lun=0 InquiryData 00 3F 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
>> Lun=0 InquiryData 10 51 45 4D 55 20 54 41 52 47 45 54 20 20 20 20 20
>> Lun=0 InquiryData 20 32 2E 35 00
>> Lun=0 InquiryData }
> 
> Here the product ID is unchanged, but the vendor ID becomes
> "QEMU_TARGET_".
> 
> And, edk2 fails to recognize the device:
> 
>> UsbBootGetParams: Found an unsupported peripheral type[31]
>> UsbMassInitMedia: UsbBootGetParams (Unsupported)
>> UsbMassInitNonLun: UsbMassInitMedia (Unsupported)
>> USBMassDriverBindingStart: UsbMassInitNonLun (Unsupported)
> 
> This happens because the first byte in the response is 0x3F. QEMU sets
> this byte in
> 
>> r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;
> 
> in function scsi_target_emulate_inquiry(), file "hw/scsi/scsi-bus.c".
> 
> And edk2 parses the byte in UsbBootInquiry() and UsbBootGetParams()
> (file "MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c"):
> 
>> typedef struct {
>>   UINT8 Pdt;///< Peripheral Device Type (low 5 bits)
>>   UINT8 Removable;  ///< Removable Media (highest bit)
>>   UINT8 Reserved0[2];
>>   UINT8 AddLen; ///< Additional length
>>   UINT8 Reserved1[3];
>>   UINT8 VendorID[8];
>>   UINT8 ProductID[16];
>>   UINT8 ProductRevision[4];
>> } USB_BOOT_INQUIRY_DATA;
> 
>> #define USB_BOOT_PDT(Pdt)   ((Pdt) & 0x1f)
> 
>>   UsbMass->Pdt  = (UINT8) (USB_BOOT_PDT (UsbMass->InquiryData.Pdt));
> 
>>   //
>>   // According to USB Mass Storage Specification for Bootability, only 
>> following
>>   // 4 Peripheral Device Types are in spec.
>>   //
>>   if ((UsbMass->Pdt != USB_PDT_DIRECT_ACCESS) &&
>>(UsbMass->Pdt != USB_PDT_CDROM) &&
>>(UsbMass->Pdt != USB_PDT_OPTICAL) &&
>>(UsbMass->Pdt != USB_PDT_SIMPLE_DIRECT)) {
>> DEBUG ((EFI_D_ERROR, "UsbBootGetParams: Found an unsupported peripheral 
>> type[%d]\n", UsbMass->Pdt));
>> return EFI_UNSUPPORTED;
>>   }
> 
> It looks likely that at least one of edk2 and QEMU has a bug. Which one
> is it?

Both implementation looks correct at first glance, QEMU replies a
standard SCSI INQUIRY format, while EDK2 expects a "USB Mass Storage
Specification
 for Bootability, Revision 1.0" INQUIRY format.

> Or is the command line incorrect perhaps? (I.e., is it invalid to
> specify *only* lun=5? Does the LUN space have to be contiguous?)
> 
> 
> (3) Starting again from the original command line, if I change "lun=0"
> to "lun=1" (rather than to "lun=5"), then OVMF even hangs, with the
> following log:
> 
>> UsbMassInitMultiLun: Start to initialize No.0 logic unit
>> Lun=0 InquiryCmd {
>> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
>> Lun=0 InquiryCmd }
>> Lun=0 InquiryData {
>> Lun=0 InquiryData 00 3F 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
>> Lun=0 InquiryData 10 51 45 4D 55 20 54 41 52 47 45 54 20 20 20 20 20
>> Lun=0 InquiryData 20 32 

Re: [edk2] [PATCH edk2-non-osi v1 5/7] Hisilicon/D06: Use new flash layout

[Ming Huang]

On 2/12/2019 11:26 PM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 10:25:05PM +0800, Ming Huang wrote:
>> In new flash layout, BIOS fd change from offset 1M to 8M in 16M
>> spi flash.
> 
> I think I covered all of the layout questions in the corresponding
> edk2-platforms patch.
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang 
>> ---
>>  Platform/Hisilicon/D06/CustomData.Fv | Bin 
>> 0 -> 65536 bytes
>>  Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.lib | Bin 
>> 61892 -> 31696 bytes
> 
> But can you explain why the size of this lib is _halved_?

This library had delete some useless functions and global variables.

Thanks

> 
> /
> Leif
> 
>>  Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv | Bin 
>> 1048576 -> 1048576 bytes
>>  3 files changed, 0 insertions(+), 0 deletions(-)
>>
>> diff --git a/Platform/Hisilicon/D06/CustomData.Fv 
>> b/Platform/Hisilicon/D06/CustomData.Fv
>> new file mode 100644
>> index 000..22ef62b
>> Binary files /dev/null and b/Platform/Hisilicon/D06/CustomData.Fv differ
>> diff --git 
>> a/Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.lib 
>> b/Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.lib
>> index 7e1f6b2..851c2c3 100644
>> Binary files 
>> a/Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.lib and 
>> b/Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.lib differ
>> diff --git a/Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv 
>> b/Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv
>> index 247e44e..7f75bc6 100644
>> Binary files a/Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv and 
>> b/Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv differ
>> -- 
>> 2.9.5
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-non-osi v2 1/1] Platform/Socionext: uprev TF-A binary

[Ard Biesheuvel]

On Tue, 12 Feb 2019 at 14:19, Sumit Garg  wrote:
>
> Update TF-A to upstream v2.0 release + synquacer-spm changes
> (Commit: e86e202c2e4e).
> Also update OP-TEE to upstream v3.4.0 release (Commit: 406c609bbf08).
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sumit Garg 
> ---
>
> Changes in v2:
>
> Include synquacer-spm patches and some internal workarounds for TF-A.
>
> Following is reference repo to pull this patch:
> ssh://g...@git.linaro.org/people/sumit.garg/edk2-non-osi.git
>
>  Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin | Bin 415251 -> 402960 
> bytes
>  1 file changed, 0 insertions(+), 0 deletions(-)
>
> diff --git a/Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin 
> b/Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin
> index bc6f4563a5d6..f6f4f5063edf 100644
> Binary files a/Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin and 
> b/Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin differ

Thanks Sumit

Pushed as 1e2ca640be54..fa45deea33b6 (and included in f/w build as of #52)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Ming Huang]

On 2/13/2019 5:42 PM, Leif Lindholm wrote:
> On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
>>> Should it not then also delete #include  from
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
>>> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
>>> ?
>>>
>>> Meanwhile,
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
>>> and
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
>>> both include this header, but
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
>>> and 
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
>>> do not declare the dependency.
>>
>> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
>> SerdesLib.h, other .c files can not remove the header file.
> 
> If they are using definitions from the library header, but not the
> library itself, there is something suspicious about the code
> structuring.

Yes, there are maybe some unreasonable design in code structuring
for history reason.

> 
> But in the meantime, if they are referencing library header files,
> they need to list those libraryclasses in their .inf.

Do you mean add SerdesLib back to SmbiosMiscDxe.inf? If yes, maybe this
patch should be droped.

Thanks

> 
>>> Can you investigate and submit an updated patch addressing all of the
>>> unnecessary references?
>>
>> This may takes a lot of time, as Hi1620(D06) is our important project,
>> maybe we should focus on D06.
> 
> Feel free to submit deletions for all and any platforms you are
> unwilling to maintain.
> 
> Best Regards,
> 
> Leif
> 
> 
>> Thanks
>>
>>>
>>> Best Regards,
>>>
>>> Leif
>>>
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Ming Huang 
 ---
  Platform/Hisilicon/D06/D06.dsc   | 2 --
  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
  2 files changed, 3 deletions(-)

 diff --git a/Platform/Hisilicon/D06/D06.dsc 
 b/Platform/Hisilicon/D06/D06.dsc
 index 396bd03c9d24..cbbd99e4a659 100644
 --- a/Platform/Hisilicon/D06/D06.dsc
 +++ b/Platform/Hisilicon/D06/D06.dsc
 @@ -64,8 +64,6 @@ [LibraryClasses.common]
  
CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
  
 -  
 SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
 -
TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

 RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

 OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
 diff --git 
 a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf 
 b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
 index 61cead7779b9..8e5c56fa41fd 100644
 --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
 +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
 @@ -77,7 +77,6 @@ [LibraryClasses]
  
IpmiCmdLib
  
 -  SerdesLib
  
  [Protocols]
gEfiSmbiosProtocolGuid   # PROTOCOL ALWAYS_CONSUMED
 -- 
 2.9.5

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

Re: [edk2] [PATCH v2 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Wang, Jian J]

Reviewed-by: Jian J Wang 

> -Original Message-
> From: Chiu, Chasel
> Sent: Wednesday, February 13, 2019 5:47 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A ;
> Ni, Ray ; Zeng, Star ; Gao, Liming
> ; Chiu, Chasel 
> Subject: [PATCH v2 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI
> should be checked to see if PeiCore not in BFV, otherwise
> just shadowing PeiCore from BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Cc: Ray Ni 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> +-
>  MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..ba0f2b7b69 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, 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
> @@ -80,23 +80,55 @@ ShadowPeiCore (
>IN PEI_CORE_INSTANCE  *PrivateData
>)
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS   Status;
> -  UINT32   AuthenticationState;
> +  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS EntryPoint;
> +  EFI_STATUS   Status;
> +  UINT32   AuthenticationState;
> +  UINTNIndex;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> 
>PeiCoreFileHandle = NULL;
> 
>//
> -  // Find the PEI Core in the BFV
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI indicated
> FV or BFV
>//
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -   PrivateData->Fv[0].FvPpi,
> -   EFI_FV_FILETYPE_PEI_CORE,
> -   PrivateData->Fv[0].FvHandle,
> -   
> -   );
> -  ASSERT_EFI_ERROR (Status);
> +  Status = PeiServicesLocatePpi (
> + ,
> + 0,
> + NULL,
> + (VOID **) 
> + );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +//
> +// If PeiCoreFvLocation present, the PEI Core should be found from 
> indicated
> FV.
> +//
> +for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +  if (PrivateData->Fv[Index].FvHandle != PeiCoreFvLocationPpi-
> >PeiCoreFvLocation) {
> +continue;
> +  }
> +  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> +   PrivateData->Fv[Index].FvPpi,
> +   EFI_FV_FILETYPE_PEI_CORE,
> +   
> PrivateData->Fv[Index].FvHandle,
> +   
> +   );
> +  if (!EFI_ERROR (Status)) {
> +break;
> +  }
> +}
> +ASSERT (Index < PrivateData->FvCount);
> +  } else {
> +//
> +// Find PEI Core from BFV
> +//
> +Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> + PrivateData->Fv[0].FvPpi,
> + EFI_FV_FILETYPE_PEI_CORE,
> + PrivateData->Fv[0].FvHandle,
> + 
> + );
> +ASSERT_EFI_ERROR (Status);
> +  }
> 
>//
>// Shadow PEI Core into memory so it will run faster
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h
> index 322e7cd845..38542ab072 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>Definition of Pei Core Structures and Services
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> 

Re: [edk2] [PATCH v2 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chasel, 
Chiu
Sent: Wednesday, February 13, 2019 5:47 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Gao, Liming 

Subject: [edk2] [PATCH v2 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

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

Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on PI spec 1.7, Section 
6.3.9.
This PPI can support the secnario that PEI Foundation not in BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53 
+
 MdePkg/MdePkg.dec  | 11 +--
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h 
b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
new file mode 100644
index 00..c7bbbfb265
--- /dev/null
+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
@@ -0,0 +1,53 @@
+/** @file
+  Header file for Pei Core FV Location PPI.
+
+  This PPI contains a pointer to the firmware volume which contains the PEI 
Foundation.
+  If the PEI Foundation does not reside in the BFV, then SEC must pass 
+ this PPI as a part  of the PPI list provided to the PEI Foundation 
+ Entry Point, otherwise the PEI Foundation  shall assume that it resides 
within the BFV.
+
+  Copyright (c) 2019, 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.
+
+  @par Revision Reference:
+  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 
1:
+  Standards
+
+**/
+
+
+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
+#define _EFI_PEI_CORE_FV_LOCATION_H_
+
+///
+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI /// #define 
+EFI_PEI_CORE_FV_LOCATION_GUID \
+  { \
+0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 
+0xca, 0xf4 } \
+  }
+
+///
+/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI /// typedef 
+struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
+
+///
+/// This PPI provides location of EFI PeiCoreFv.
+///
+struct _EFI_PEI_CORE_FV_LOCATION_PPI {
+  ///
+  /// Pointer to the first byte of the firmware volume which contains the PEI 
Foundation.
+  ///
+  VOID*PeiCoreFvLocation;
+};
+
+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
+
+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index a485408310..c859b4a511 
100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,9 +2,9 @@
 # This Package provides all definitions, library classes and libraries 
instances.
 #
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of -# 
EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights 
+reserved.
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.  # 
(C) Copyright 2016 Hewlett Packard Enterprise Development LP  # @@ -929,6 
+929,13 @@
   ## Include/Ppi/SecHobData.h
   gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 
0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
 
+  #
+  # PPIs defined in PI 1.7.
+  #
+
+  ## Include/Ppi/PeiCoreFvLocation.h
+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 
0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
+
 [Protocols]
   ## Include/Protocol/Pcd.h
   gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 
0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
--
2.13.3.windows.1

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

Re: [edk2] [PATCH v2 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: Chiu, Chasel 
Sent: Wednesday, February 13, 2019 5:47 PM
To: edk2-devel@lists.01.org
Cc: Wang, Jian J ; Wu, Hao A ; Ni, 
Ray ; Zeng, Star ; Gao, Liming 
; Chiu, Chasel 
Subject: [PATCH v2 2/3] MdeModulePkg/PeiMain: Support 
EFI_PEI_CORE_FV_LOCATION_PPI

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

When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be checked to 
see if PeiCore not in BFV, otherwise just shadowing PeiCore from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58 
+-
 MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c 
b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 4da80a8222..ba0f2b7b69 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Main Entry Point
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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 @@ -80,23 +80,55 @@ 
ShadowPeiCore (
   IN PEI_CORE_INSTANCE  *PrivateData
   )
 {
-  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
-  EFI_PHYSICAL_ADDRESS EntryPoint;
-  EFI_STATUS   Status;
-  UINT32   AuthenticationState;
+  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
+  EFI_PHYSICAL_ADDRESS EntryPoint;
+  EFI_STATUS   Status;
+  UINT32   AuthenticationState;
+  UINTNIndex;
+  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
 
   PeiCoreFileHandle = NULL;
 
   //
-  // Find the PEI Core in the BFV
+  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI 
+ indicated FV or BFV
   //
-  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
-   PrivateData->Fv[0].FvPpi,
-   EFI_FV_FILETYPE_PEI_CORE,
-   PrivateData->Fv[0].FvHandle,
-   
-   );
-  ASSERT_EFI_ERROR (Status);
+  Status = PeiServicesLocatePpi (
+ ,
+ 0,
+ NULL,
+ (VOID **) 
+ );
+  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation != 
NULL)) {
+//
+// If PeiCoreFvLocation present, the PEI Core should be found from 
indicated FV.
+//
+for (Index = 0; Index < PrivateData->FvCount; Index ++) {
+  if (PrivateData->Fv[Index].FvHandle != 
PeiCoreFvLocationPpi->PeiCoreFvLocation) {
+continue;
+  }
+  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
+   PrivateData->Fv[Index].FvPpi,
+   EFI_FV_FILETYPE_PEI_CORE,
+   PrivateData->Fv[Index].FvHandle,
+   
+   );
+  if (!EFI_ERROR (Status)) {
+break;
+  }
+}
+ASSERT (Index < PrivateData->FvCount);  } else {
+//
+// Find PEI Core from BFV
+//
+Status = PrivateData->Fv[0].FvPpi->FindFileByType (
+ PrivateData->Fv[0].FvPpi,
+ EFI_FV_FILETYPE_PEI_CORE,
+ PrivateData->Fv[0].FvHandle,
+ 
+ );
+ASSERT_EFI_ERROR (Status);
+  }
 
   //
   // Shadow PEI Core into memory so it will run faster diff --git 
a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h index 
322e7cd845..38542ab072 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -1,7 +1,7 @@
 /** @file
   Definition of Pei Core Structures and Services
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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 @@ -49,6 +49,7 @@ 
WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 ///
 /// It is an FFS type 

[edk2] [PATCH v2 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

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

EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform
when PeiCore not in BFV so SecCore has to search PeiCore
either from the FV location provided by
EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 UefiCpuPkg/SecCore/SecMain.c   | 35 +--
 UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index b24e190617..b99072599d 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -1,7 +1,7 @@
 /** @file
   C functions in SEC
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 2019, 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
@@ -228,26 +228,49 @@ SecStartupPhase2(
 {
   EFI_SEC_PEI_HAND_OFF*SecCoreData;
   EFI_PEI_PPI_DESCRIPTOR  *PpiList;
+  EFI_PEI_PPI_DESCRIPTOR  *PpiListTmp;
   UINT32  Index;
   EFI_PEI_PPI_DESCRIPTOR  *AllSecPpiList;
   EFI_PEI_CORE_ENTRY_POINTPeiCoreEntryPoint;
 
+  PeiCoreEntryPoint = NULL;
   SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
   AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData->PeiTemporaryRamBase;
+
   //
   // Find Pei Core entry point. It will report SEC and Pei Core debug 
information if remote debug
   // is enabled.
   //
-  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
SecCoreData->BootFirmwareVolumeBase, );
-  if (PeiCoreEntryPoint == NULL)
-  {
-CpuDeadLoop ();
+  PpiList = SecPlatformMain (SecCoreData);
+  PpiListTmp = PpiList;
+  for (;;) {
+if (CompareGuid (PpiListTmp->Guid, ) && 
(((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation != 0)) {
+  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation, 
);
+  if (PeiCoreEntryPoint != NULL) {
+break;
+  }
+}
+if ((PpiListTmp->Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) == 
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) {
+  //
+  // Continue until the end of the PPI List.
+  //
+  break;
+}
+PpiListTmp++;
+  }
+  //
+  // If EFI_PEI_CORE_FV_LOCATION_PPI not found or no PeiCore found by the 
pointer in provided PPI, try to locate PeiCore from BFV.
+  //
+  if (PeiCoreEntryPoint == NULL) {
+FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
SecCoreData->BootFirmwareVolumeBase, );
+if (PeiCoreEntryPoint == NULL) {
+  CpuDeadLoop ();
+}
   }
 
   //
   // Perform platform specific initialization before entering PeiCore.
   //
-  PpiList = SecPlatformMain (SecCoreData);
   if (PpiList != NULL) {
 //
 // Remove the terminal flag from the terminal PPI
diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
index 442f663911..fc1485b5cb 100644
--- a/UefiCpuPkg/SecCore/SecCore.inf
+++ b/UefiCpuPkg/SecCore/SecCore.inf
@@ -7,7 +7,7 @@
 #  protected mode, setup flat memory model, enable temporary memory and
 #  call into SecStartup().
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, 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
@@ -73,6 +73,7 @@
   ## NOTIFY
   ## SOMETIMES_CONSUMES
   gPeiSecPerformancePpiGuid
+  gEfiPeiCoreFvLocationPpiGuid
 
 [Guids]
   ## SOMETIMES_PRODUCES   ## HOB
diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
index 83244af119..80045d34f4 100644
--- a/UefiCpuPkg/SecCore/SecMain.h
+++ b/UefiCpuPkg/SecCore/SecMain.h
@@ -1,7 +1,7 @@
 /** @file
   Master header file for SecCore.
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 2019, 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
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
-- 
2.13.3.windows.1

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

[edk2] [PATCH v2 0/3] Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

PI spec 1.7 section 6.3.9 has defined a PPI to support the scenario
that PEI Foundation not in BFV. EFI_PEI_CORE_FV_LOCATION_PPI reports
the FV which contains PEI Foundation and should be passed by SEC as
part of PPI list. Otherwise PEI Foundation shall assume that it
resides in BFV.

Patch1: Add EFI_PEI_CORE_FV_LOCATION_PPI definition.
Patch2: Support PeiCore not in BFV scenario when shadowing.
Patch3: SecCore to find PeiCore from either non-BFV or BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 

Chasel, Chiu (3):
  MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI
  MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI
  UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58 
+-
 UefiCpuPkg/SecCore/SecMain.c| 35 
+--
 MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
 MdePkg/Include/Ppi/PeiCoreFvLocation.h  | 53 
+
 MdePkg/MdePkg.dec   | 11 +--
 UefiCpuPkg/SecCore/SecCore.inf  |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h|  3 ++-
 8 files changed, 144 insertions(+), 25 deletions(-)
 create mode 100644 MdePkg/Include/Ppi/PeiCoreFvLocation.h

-- 
2.13.3.windows.1

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

[edk2] [PATCH v2 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

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

When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI
should be checked to see if PeiCore not in BFV, otherwise
just shadowing PeiCore from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58 
+-
 MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c 
b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 4da80a8222..ba0f2b7b69 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Main Entry Point
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -80,23 +80,55 @@ ShadowPeiCore (
   IN PEI_CORE_INSTANCE  *PrivateData
   )
 {
-  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
-  EFI_PHYSICAL_ADDRESS EntryPoint;
-  EFI_STATUS   Status;
-  UINT32   AuthenticationState;
+  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
+  EFI_PHYSICAL_ADDRESS EntryPoint;
+  EFI_STATUS   Status;
+  UINT32   AuthenticationState;
+  UINTNIndex;
+  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
 
   PeiCoreFileHandle = NULL;
 
   //
-  // Find the PEI Core in the BFV
+  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI indicated FV 
or BFV
   //
-  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
-   PrivateData->Fv[0].FvPpi,
-   EFI_FV_FILETYPE_PEI_CORE,
-   PrivateData->Fv[0].FvHandle,
-   
-   );
-  ASSERT_EFI_ERROR (Status);
+  Status = PeiServicesLocatePpi (
+ ,
+ 0,
+ NULL,
+ (VOID **) 
+ );
+  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation != 
NULL)) {
+//
+// If PeiCoreFvLocation present, the PEI Core should be found from 
indicated FV.
+//
+for (Index = 0; Index < PrivateData->FvCount; Index ++) {
+  if (PrivateData->Fv[Index].FvHandle != 
PeiCoreFvLocationPpi->PeiCoreFvLocation) {
+continue;
+  }
+  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
+   PrivateData->Fv[Index].FvPpi,
+   EFI_FV_FILETYPE_PEI_CORE,
+   PrivateData->Fv[Index].FvHandle,
+   
+   );
+  if (!EFI_ERROR (Status)) {
+break;
+  }
+}
+ASSERT (Index < PrivateData->FvCount);
+  } else {
+//
+// Find PEI Core from BFV
+//
+Status = PrivateData->Fv[0].FvPpi->FindFileByType (
+ PrivateData->Fv[0].FvPpi,
+ EFI_FV_FILETYPE_PEI_CORE,
+ PrivateData->Fv[0].FvHandle,
+ 
+ );
+ASSERT_EFI_ERROR (Status);
+  }
 
   //
   // Shadow PEI Core into memory so it will run faster
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index 322e7cd845..38542ab072 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -1,7 +1,7 @@
 /** @file
   Definition of Pei Core Structures and Services
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 ///
 /// It is an FFS type extension used for PeiFindFileEx. It indicates current
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf 
b/MdeModulePkg/Core/Pei/PeiMain.inf
index 140cb1..5bab2aab8c 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -6,7 +6,7 @@
 # 2) Dispatch PEIM from discovered FV.
 # 

[edk2] [PATCH v2 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

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

Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on
PI spec 1.7, Section 6.3.9.
This PPI can support the secnario that PEI Foundation
not in BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53 
+
 MdePkg/MdePkg.dec  | 11 +--
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h 
b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
new file mode 100644
index 00..c7bbbfb265
--- /dev/null
+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
@@ -0,0 +1,53 @@
+/** @file
+  Header file for Pei Core FV Location PPI.
+
+  This PPI contains a pointer to the firmware volume which contains the PEI 
Foundation.
+  If the PEI Foundation does not reside in the BFV, then SEC must pass this 
PPI as a part
+  of the PPI list provided to the PEI Foundation Entry Point, otherwise the 
PEI Foundation
+  shall assume that it resides within the BFV.
+
+  Copyright (c) 2019, 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.
+
+  @par Revision Reference:
+  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 
1:
+  Standards
+
+**/
+
+
+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
+#define _EFI_PEI_CORE_FV_LOCATION_H_
+
+///
+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+#define EFI_PEI_CORE_FV_LOCATION_GUID \
+  { \
+0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 
0xf4 } \
+  }
+
+///
+/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+typedef struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
+
+///
+/// This PPI provides location of EFI PeiCoreFv.
+///
+struct _EFI_PEI_CORE_FV_LOCATION_PPI {
+  ///
+  /// Pointer to the first byte of the firmware volume which contains the PEI 
Foundation.
+  ///
+  VOID*PeiCoreFvLocation;
+};
+
+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
+
+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index a485408310..c859b4a511 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,9 +2,9 @@
 # This Package provides all definitions, library classes and libraries 
instances.
 #
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
-# EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
 # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
@@ -929,6 +929,13 @@
   ## Include/Ppi/SecHobData.h
   gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 
0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
 
+  #
+  # PPIs defined in PI 1.7.
+  #
+
+  ## Include/Ppi/PeiCoreFvLocation.h
+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 
0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
+
 [Protocols]
   ## Include/Protocol/Pcd.h
   gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 
0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
-- 
2.13.3.windows.1

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

Re: [edk2] [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

[Leif Lindholm]

On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
> > Should it not then also delete #include  from
> > Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
> > Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
> > Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
> > ?
> > 
> > Meanwhile,
> > Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
> > and
> > Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
> > both include this header, but
> > Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
> > and 
> > Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
> > do not declare the dependency.
> 
> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
> SerdesLib.h, other .c files can not remove the header file.

If they are using definitions from the library header, but not the
library itself, there is something suspicious about the code
structuring.

But in the meantime, if they are referencing library header files,
they need to list those libraryclasses in their .inf.

> > Can you investigate and submit an updated patch addressing all of the
> > unnecessary references?
> 
> This may takes a lot of time, as Hi1620(D06) is our important project,
> maybe we should focus on D06.

Feel free to submit deletions for all and any platforms you are
unwilling to maintain.

Best Regards,

Leif


> Thanks
> 
> > 
> > Best Regards,
> > 
> > Leif
> > 
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang 
> >> ---
> >>  Platform/Hisilicon/D06/D06.dsc   | 2 --
> >>  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
> >>  2 files changed, 3 deletions(-)
> >>
> >> diff --git a/Platform/Hisilicon/D06/D06.dsc 
> >> b/Platform/Hisilicon/D06/D06.dsc
> >> index 396bd03c9d24..cbbd99e4a659 100644
> >> --- a/Platform/Hisilicon/D06/D06.dsc
> >> +++ b/Platform/Hisilicon/D06/D06.dsc
> >> @@ -64,8 +64,6 @@ [LibraryClasses.common]
> >>  
> >>CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
> >>  
> >> -  
> >> SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
> >> -
> >>TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
> >>
> >> RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
> >>
> >> OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
> >> diff --git 
> >> a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf 
> >> b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> >> index 61cead7779b9..8e5c56fa41fd 100644
> >> --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> >> +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> >> @@ -77,7 +77,6 @@ [LibraryClasses]
> >>  
> >>IpmiCmdLib
> >>  
> >> -  SerdesLib
> >>  
> >>  [Protocols]
> >>gEfiSmbiosProtocolGuid   # PROTOCOL ALWAYS_CONSUMED
> >> -- 
> >> 2.9.5
> >>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Zeng, Star]

With this addressed, Reviewed-by: Star Zeng .

-Original Message-
From: Chiu, Chasel 
Sent: Wednesday, February 13, 2019 11:59 AM
To: Wang, Jian J ; edk2-devel@lists.01.org
Cc: Wu, Hao A ; Ni, Ray ; Zeng, Star 
; Gao, Liming 
Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support 
EFI_PEI_CORE_FV_LOCATION_PPI


No issue, I will remove UINT32 casting. Thanks!

> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, February 13, 2019 9:14 AM
> To: Chiu, Chasel ; edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ray ; Zeng, 
> Star ; Gao, Liming 
> Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support 
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> Chasel,
> 
> 
> > -Original Message-
> > From: Chiu, Chasel
> > Sent: Tuesday, February 12, 2019 9:20 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Jian J ; Wu, Hao A 
> > ; Ni, Ray ; Zeng, Star 
> > ; Gao, Liming ; Chiu, 
> > Chasel 
> > Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support 
> > EFI_PEI_CORE_FV_LOCATION_PPI
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> >
> > When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be 
> > checked to see if PeiCore not in BFV, otherwise just shadowing 
> > PeiCore from BFV.
> >
> > Test: Verified on internal platform and booting successfully.
> >
> > Cc: Jian J Wang 
> > Cc: Hao Wu 
> > Cc: Ray Ni 
> > Cc: Star Zeng 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chasel Chiu 
> > ---
> >  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> > +-
> >  MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
> >  MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
> >  3 files changed, 49 insertions(+), 15 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > index 4da80a8222..408f24c216 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >Pei Core Main Entry Point
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights 
> > reserved.
> > +Copyright (c) 2006 - 2019, 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 @@ -80,23 +80,55 @@ ShadowPeiCore (
> >IN PEI_CORE_INSTANCE  *PrivateData
> >)
> >  {
> > -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > -  EFI_PHYSICAL_ADDRESS EntryPoint;
> > -  EFI_STATUS   Status;
> > -  UINT32   AuthenticationState;
> > +  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > +  EFI_PHYSICAL_ADDRESS EntryPoint;
> > +  EFI_STATUS   Status;
> > +  UINT32   AuthenticationState;
> > +  UINTNIndex;
> > +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> >
> >PeiCoreFileHandle = NULL;
> >
> >//
> > -  // Find the PEI Core in the BFV
> > +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI 
> > + indicated
> > FV or BFV
> >//
> > -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > -   PrivateData->Fv[0].FvPpi,
> > -   EFI_FV_FILETYPE_PEI_CORE,
> > -   PrivateData->Fv[0].FvHandle,
> > -   
> > -   );
> > -  ASSERT_EFI_ERROR (Status);
> > +  Status = PeiServicesLocatePpi (
> > + ,
> > + 0,
> > + NULL,
> > + (VOID **) 
> > + );
> > +  if (!EFI_ERROR (Status) && 
> > + (PeiCoreFvLocationPpi->PeiCoreFvLocation
> > + !=
> > NULL)) {
> > +//
> > +// If PeiCoreFvLocation present, the PEI Core should be found 
> > + from indicated
> > FV.
> > +//
> > +for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> > +  if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> > PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> 
> I think the UINT32 type cast is not necessary. FvHandle and 
> PeiCoreFvLocation are actually type of VOID*. Do you encounter any compiler 
> error here?
> 
> Regards,
> Jian
> 
> > +continue;
> > +  }
> > +  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> > +   
> > PrivateData->Fv[Index].FvPpi,
> > +   EFI_FV_FILETYPE_PEI_CORE,
> > +   
> > PrivateData->Fv[Index].FvHandle,
> > +   
> > +   );
> > +  if (!EFI_ERROR (Status)) {
> > +break;
> > +  }
> > +}
> > +ASSERT (Index < PrivateData->FvCount);  } else {

Re: [edk2] [PATCH 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Zeng, Star]

On 2019/2/12 21:19, Chasel, Chiu wrote:

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

Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on
PI spec 1.7, Section 6.3.9.
This PPI can support the secnario that PEI Foundation
not in BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 


Reviewed-by: Star Zeng 


---
  MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53 
+
  MdePkg/MdePkg.dec  | 11 +--
  2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h 
b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
new file mode 100644
index 00..c7bbbfb265
--- /dev/null
+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
@@ -0,0 +1,53 @@
+/** @file
+  Header file for Pei Core FV Location PPI.
+
+  This PPI contains a pointer to the firmware volume which contains the PEI 
Foundation.
+  If the PEI Foundation does not reside in the BFV, then SEC must pass this 
PPI as a part
+  of the PPI list provided to the PEI Foundation Entry Point, otherwise the 
PEI Foundation
+  shall assume that it resides within the BFV.
+
+  Copyright (c) 2019, 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.
+
+  @par Revision Reference:
+  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 
1:
+  Standards
+
+**/
+
+
+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
+#define _EFI_PEI_CORE_FV_LOCATION_H_
+
+///
+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+#define EFI_PEI_CORE_FV_LOCATION_GUID \
+  { \
+0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 
0xf4 } \
+  }
+
+///
+/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+typedef struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
+
+///
+/// This PPI provides location of EFI PeiCoreFv.
+///
+struct _EFI_PEI_CORE_FV_LOCATION_PPI {
+  ///
+  /// Pointer to the first byte of the firmware volume which contains the PEI 
Foundation.
+  ///
+  VOID*PeiCoreFvLocation;
+};
+
+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
+
+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index a485408310..c859b4a511 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,9 +2,9 @@
  # This Package provides all definitions, library classes and libraries 
instances.
  #
  # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
-# EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
  #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
  # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
  #
@@ -929,6 +929,13 @@
## Include/Ppi/SecHobData.h
gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 
0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
  
+  #

+  # PPIs defined in PI 1.7.
+  #
+
+  ## Include/Ppi/PeiCoreFvLocation.h
+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 
0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
+
  [Protocols]
## Include/Protocol/Pcd.h
gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 
0x90, 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}



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

Re: [edk2] [Patch edk2 Wiki] Add three features for edk2-stable201903

[Chiu, Chasel]

Reviewed-by: Chasel Chiu 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Liming Gao
> Sent: Wednesday, February 13, 2019 12:24 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch edk2 Wiki] Add three features for edk2-stable201903
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao 
> ---
>  EDK-II-Release-Planning.md | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/EDK-II-Release-Planning.md b/EDK-II-Release-Planning.md index
> f302be3..095da69 100644
> --- a/EDK-II-Release-Planning.md
> +++ b/EDK-II-Release-Planning.md
> @@ -24,7 +24,9 @@
>  * [Remove PcdPeiCoreMaxXXX
> PCDs](https://bugzilla.tianocore.org/show_bug.cgi?id=1405)
>  * [Remove unused tool logic in BaseTools
> C\Python](https://bugzilla.tianocore.org/show_bug.cgi?id=1350)
>  * [BaseTools: Enable component override
> functionality](https://bugzilla.tianocore.org/show_bug.cgi?id=1449)
> -* [SMM CET support](https://bugzilla.tianocore.org/show_bug.cgi?id=1521)
> +* [Support PI1.7
> +EFI_PEI_CORE_FV_LOCATION_PPI](https://bugzilla.tianocore.org/show_bug.
> c
> +gi?id=1524)
> +* [Remove unused tool chain configuration in
> +tools_def.template](https://bugzilla.tianocore.org/show_bug.cgi?id=1377
> +)
> +* [BaseTools supports to the driver
> +combination](https://bugzilla.tianocore.org/show_bug.cgi?id=1520)
>  * Standalone MM build of authenticated variable stack (bugzilla link TBD)
>  * TBD Bugzilla List
> 
> --
> 2.13.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

Re: [edk2] [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT

[Leif Lindholm]

On Wed, Feb 13, 2019 at 10:59:17AM +0800, Ming Huang wrote:
> On 2/12/2019 2:51 AM, Leif Lindholm wrote:
> > On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote:
> >> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
> >> output in kernel:
> >> [  103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ming Huang 
> >> ---
> >>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64 
> >> 
> >>  1 file changed, 64 insertions(+)
> >>
> >> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl 
> >> b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> >> index 4d9d9d95be68..86d8728b82f2 100644
> >> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> >> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> >> @@ -17,6 +17,50 @@
> >>  **/
> >>  
> >>  //#include "ArmPlatform.h"
> >> +
> >> +/*
> >> +  See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
> >> +*/
> >> +#define PCI_OSC_SUPPORT() \
> > 
> > PCI0 and PCI6 already have _OSC entries.
> > This macro ends up being used for 1-5 and 7-B.
> > So calling it PCI_OSC_SUPPORT seems somewhat misleading.
> > 
> > Then again, there is a lot of similarities between this macro and the
> > existing entries. Could the same macro be used for 0 and 6? Or could
> > the macro be split up into multiple parts and reused?
> 
> When I make this patch, I try to rewrite PCI0/6 with the same macro, but
> the macro don't support parameter. For spliting up multiple parts, if modify
> something in future, the parts need to split up to smaller parts. So, if
> need to rewrite PCI0/6 with macro, is it applicable to add another macro
> PCI_OSC_SUPPORT_HOTPLUG?

Yes, that sounds like a good solution to me.

Regards,

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

[edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

[Laszlo Ersek]

Hi,

using QEMU, when I specify a nonzero LUN for the hard disk that sits on
the "SCSI bus" that embodies the USB Bulk Only Transfer device, then
UsbMassStorageDxe fails to recognize the hard disk.

(1) Consider the following QEMU command line snippet:

  -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
  -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
  -device usb-bot,bus=xhci1.0,port=3,id=bot1 \
  -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \

For this (i.e., lun=0), I see the following INQUIRY exchange between
UsbMassStorageDxe and QEMU:

> Lun=0 InquiryCmd {
> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
> Lun=0 InquiryCmd }
> Lun=0 InquiryData {
> Lun=0 InquiryData 00 00 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
> Lun=0 InquiryData 10 51 45 4D 55 20 48 41 52 44 44 49 53 4B 20 20 20
> Lun=0 InquiryData 20 32 2E 35 2B
> Lun=0 InquiryData }

The vendor and product ID fields translate to "QEMU" and
"QEMU_HARDDISK___", respectively. (For easier reading, I replaced the
space characters with underscores, for the sake of better
readability/wrapping.)

In this case, edk2 recognizes the disk and things work fine.

(In fact, for lun=0, the QemuBootOrderLib pattern matching / translation
works fine as well -- verifying which was my original goal, before I ran
into the issues below, for nonzero LUNs. But, I digress.)


(2) If I change the cmdline to "lun=5", then the exchange is:

> Lun=0 InquiryCmd {
> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
> Lun=0 InquiryCmd }
> Lun=0 InquiryData {
> Lun=0 InquiryData 00 3F 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
> Lun=0 InquiryData 10 51 45 4D 55 20 54 41 52 47 45 54 20 20 20 20 20
> Lun=0 InquiryData 20 32 2E 35 00
> Lun=0 InquiryData }

Here the product ID is unchanged, but the vendor ID becomes
"QEMU_TARGET_".

And, edk2 fails to recognize the device:

> UsbBootGetParams: Found an unsupported peripheral type[31]
> UsbMassInitMedia: UsbBootGetParams (Unsupported)
> UsbMassInitNonLun: UsbMassInitMedia (Unsupported)
> USBMassDriverBindingStart: UsbMassInitNonLun (Unsupported)

This happens because the first byte in the response is 0x3F. QEMU sets
this byte in

> r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;

in function scsi_target_emulate_inquiry(), file "hw/scsi/scsi-bus.c".

And edk2 parses the byte in UsbBootInquiry() and UsbBootGetParams()
(file "MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c"):

> typedef struct {
>   UINT8 Pdt;///< Peripheral Device Type (low 5 bits)
>   UINT8 Removable;  ///< Removable Media (highest bit)
>   UINT8 Reserved0[2];
>   UINT8 AddLen; ///< Additional length
>   UINT8 Reserved1[3];
>   UINT8 VendorID[8];
>   UINT8 ProductID[16];
>   UINT8 ProductRevision[4];
> } USB_BOOT_INQUIRY_DATA;

> #define USB_BOOT_PDT(Pdt)   ((Pdt) & 0x1f)

>   UsbMass->Pdt  = (UINT8) (USB_BOOT_PDT (UsbMass->InquiryData.Pdt));

>   //
>   // According to USB Mass Storage Specification for Bootability, only 
> following
>   // 4 Peripheral Device Types are in spec.
>   //
>   if ((UsbMass->Pdt != USB_PDT_DIRECT_ACCESS) &&
>(UsbMass->Pdt != USB_PDT_CDROM) &&
>(UsbMass->Pdt != USB_PDT_OPTICAL) &&
>(UsbMass->Pdt != USB_PDT_SIMPLE_DIRECT)) {
> DEBUG ((EFI_D_ERROR, "UsbBootGetParams: Found an unsupported peripheral 
> type[%d]\n", UsbMass->Pdt));
> return EFI_UNSUPPORTED;
>   }

It looks likely that at least one of edk2 and QEMU has a bug. Which one
is it?

Or is the command line incorrect perhaps? (I.e., is it invalid to
specify *only* lun=5? Does the LUN space have to be contiguous?)


(3) Starting again from the original command line, if I change "lun=0"
to "lun=1" (rather than to "lun=5"), then OVMF even hangs, with the
following log:

> UsbMassInitMultiLun: Start to initialize No.0 logic unit
> Lun=0 InquiryCmd {
> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
> Lun=0 InquiryCmd }
> Lun=0 InquiryData {
> Lun=0 InquiryData 00 3F 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
> Lun=0 InquiryData 10 51 45 4D 55 20 54 41 52 47 45 54 20 20 20 20 20
> Lun=0 InquiryData 20 32 2E 35 00
> Lun=0 InquiryData }
> UsbBootGetParams: Found an unsupported peripheral type[31]
> UsbMassInitMedia: UsbBootGetParams (Unsupported)
> UsbMassInitMultiLun: UsbMassInitMedia (Unsupported)
> UsbMassInitMultiLun: Start to initialize No.1 logic unit
> Lun=32 InquiryCmd {
> Lun=32 InquiryCmd 00 12 20 00 00 24 00 00 00 00 00 00 00
> Lun=32 InquiryCmd }
> Lun=32 InquiryData {
> Lun=32 InquiryData 00 00 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
> Lun=32 InquiryData 10 51 45 4D 55 20 48 41 52 44 44 49 53 4B 20 20 20
> Lun=32 InquiryData 20 32 2E 35 2B
> Lun=32 InquiryData }
> UsbBootExecCmd: Success to Exec 0x0 Cmd (Result = 1)
> UsbBootRequestSense: (Invalid Parameter) with error code