[edk2-devel] Updated Event: TianoCore Design Meeting - APAC/NAMO - Friday, 3 April 2020 #cal-invite

[devel@edk2.groups.io Calendar]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:PUBLISH
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:g7lq.1578029159272351097.u...@groups.io
DTSTAMP:20200401T052016Z
ORGANIZER;CN=Ray Ni:mailto:ray...@intel.com
DTSTART:20200403T013000Z
DTEND:20200403T023000Z
SUMMARY:TianoCore Design Meeting - APAC/NAMO
DESCRIPTION:# TOPIC\n\n* EDK2 Redfish Implementation Review (Abner Chang/
 HPE)\n\n  Slides: https://edk2.groups.io/g/devel/files/Designs/2020/0403/
 EDK2%20Redfish%20Implementation%20Review.pdf\n\n\n## Join Zoom Meeting\n\
 nhttps://zoom.com.cn/j/971006237\n\nMeeting ID: 971 006 237\n\nOne tap mo
 bile\n\n+16465588656,,971006237# US (New York)\n\n+14086380968,,971006237
 # US (San Jose)\n\nDial by your location\n\n+1 646 558 8656 US (N
 ew York)\n\n+1 408 638 0968 US (San Jose)\n\nMeeting ID: 971 006 
 237\n\nFind your local number: https://zoom.com.cn/u/idwYt3asgU
LOCATION:https://zoom.com.cn/j/971006237
RECURRENCE-ID:20200403T013000Z
SEQUENCE:0
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

Re: [edk2-devel] [Patch V2 3/3] Features/Intel: Correct wrong codes and remove unnecessary codes

[Dong, Eric]

Reviewed-by: Eric Dong 

> -Original Message-
> From: Luo, Heng
> Sent: Tuesday, March 31, 2020 11:49 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Gao, Liming ;
> Dong, Eric ; Ni, Ray 
> Subject: [Patch V2 3/3] Features/Intel: Correct wrong codes and remove
> unnecessary codes
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2644
> 
> Correct wrong codes and remove unnecessary codes in LogoFeaturePkg.
> 
> Cc: Dandan Bi 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Signed-off-by: Heng Luo 
> ---
>  Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc | 9
> -
> Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf  | 2
> +-
>  2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git
> a/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc
> b/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc
> index fca0bfd540..d2dcdeb36a 100644
> ---
> a/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc
> +++
> b/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.ds
> +++ c
> @@ -25,15 +25,6 @@
>!error "DXE_ARCH must be specified to build this feature!" !endif -
> ##
> ##-#-# Packages Section - Make sure PCD can be
> directly used in a conditional statement-# in a DSC which includes this DSC
> file.-#-
> ##
> ##-[Packages]-
> LogoFeaturePkg/LogoFeaturePkg.dec-
> ##
> ## # # Library Class section - list of all Library
> Classes needed by this feature.diff --git
> a/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
> b/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
> index 080c87223c..fead9f3b02 100644
> ---
> a/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
> +++
> b/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
> @@ -6,7 +6,7 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent # ##-!if
> gSmbiosFeaturePkgTokenSpaceGuid.PcdJpgEnable == TRUE+!if
> gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable == TRUE   INF
> LogoFeaturePkg/LogoDxe/JpegLogoDxe.inf !else   INF
> LogoFeaturePkg/LogoDxe/LogoDxe.inf--
> 2.24.0.windows.2


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

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

Re: [edk2-devel] [Patch V2 2/3] Features/Intel: Add LogoFeaturePkg to TemporaryBuildWorkaround

[Dong, Eric]

Reviewed-by: Eric Dong 

> -Original Message-
> From: Luo, Heng
> Sent: Tuesday, March 31, 2020 11:49 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Gao, Liming ;
> Dong, Eric ; Ni, Ray 
> Subject: [Patch V2 2/3] Features/Intel: Add LogoFeaturePkg to
> TemporaryBuildWorkaround
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2644
> 
> Need to add LogoFeaturePkg to TemporaryBuildWorkaround because
> OpenBoard still includes TemporaryBuildWorkaround for building BIOS.
> 
> Cc: Dandan Bi 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Signed-off-by: Heng Luo 
> ---
> 
> Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempora
> ryBuildWorkaround.dsc | 4 +++-
> Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempora
> ryBuildWorkaround.inf | 5 -
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git
> a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> raryBuildWorkaround.dsc
> b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> raryBuildWorkaround.dsc
> index 227ae00908..c62f9ecc6e 100644
> ---
> a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> raryBuildWorkaround.dsc
> +++
> b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> ra
> +++ ryBuildWorkaround.dsc
> @@ -13,7 +13,7 @@
>  # When the BaseTools update is complete, this file can entirely be removed
> # from this package. #-# Copyright (c) 2019, Intel Corporation. All rights
> reserved.+# Copyright (c) 2019 - 2020, Intel Corporation. All rights
> reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -49,6
> +49,8 @@
>gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable
> |FALSE
> gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable
> |FALSE
> gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnabl
> e|FALSE+  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable
> |FALSE+  gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable
> |FALSE !endif  #diff --git
> a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> raryBuildWorkaround.inf
> b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> raryBuildWorkaround.inf
> index 74176d1989..00818fbe0a 100644
> ---
> a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> raryBuildWorkaround.inf
> +++
> b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempo
> ra
> +++ ryBuildWorkaround.inf
> @@ -13,7 +13,7 @@
>  # When the BaseTools update is complete, this file can entirely be removed
> # from this package. #-# Copyright (c) 2019, Intel Corporation. All rights
> reserved.+# Copyright (c) 2019 - 2020, Intel Corporation. All rights
> reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -40,6
> +40,7 @@
>PowerManagement/S3FeaturePkg/S3FeaturePkg.dec
> SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dec
> UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec+
> UserInterface/LogoFeaturePkg/LogoFeaturePkg.dec  [FeaturePcd]
> gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable@@ -
> 49,6 +50,8 @@
>gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable
> gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable
> gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnabl
> e+  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable+
> gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable  [Sources]
> TemporaryBuildWorkaround.c--
> 2.24.0.windows.2


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

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

Re: [edk2-devel] [Patch V2 1/3] Platform/Intel: Add all pathes of feature domains to package path

[Dong, Eric]

Reviewed-by: Eric Dong 

> -Original Message-
> From: Luo, Heng
> Sent: Tuesday, March 31, 2020 11:49 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Gao, Liming ;
> Dong, Eric ; Ni, Ray 
> Subject: [Patch V2 1/3] Platform/Intel: Add all pathes of feature domains to
> package path
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2644
> 
> Add all pathes of feature domains to package path in build_bios.py.
> 
> Cc: Dandan Bi 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Signed-off-by: Heng Luo 
> ---
> 
> Notes:
> v2:
>   - Skip adding folders that contains package contents to the
> PACKAGES_PATH. [Ray Ni]
> 
>  Platform/Intel/build_bios.py | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Intel/build_bios.py b/Platform/Intel/build_bios.py index
> 1ef35aca0a..8f855f63eb 100644
> --- a/Platform/Intel/build_bios.py
> +++ b/Platform/Intel/build_bios.py
> @@ -3,7 +3,7 @@
>  # Builds BIOS using configuration files and dynamically  # imported functions
> from board directory  # -# Copyright (c) 2019, Intel Corporation. All rights
> reserved.
> +# Copyright (c) 2019 - 2020, Intel Corporation. All rights
> +reserved.
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  #
> 
> @@ -16,6 +16,7 @@ imported functions from board directory  import os
> import re  import sys
> +import glob
>  import signal
>  import shutil
>  import argparse
> @@ -120,6 +121,13 @@ def pre_build(build_config, build_type="DEBUG",
> silent=False, toolchain=None):
>  config["PACKAGES_PATH"] += os.pathsep +
> config["WORKSPACE_SILICON"]
>  config["PACKAGES_PATH"] += os.pathsep +
> config["WORKSPACE_SILICON_BIN"]
>  config["PACKAGES_PATH"] += os.pathsep +
> config["WORKSPACE_FEATURES"]
> +# add all feature domains in WORKSPACE_FEATURES to package path
> +for filename in os.listdir(config["WORKSPACE_FEATURES"]):
> +filepath = os.path.join(config["WORKSPACE_FEATURES"], filename)
> +# feature domains folder does not contain dec file
> +if os.path.isdir(filepath) and \
> +  not glob.glob(os.path.join(filepath, "*.dec")):
> +config["PACKAGES_PATH"] += os.pathsep + filepath
>  config["PACKAGES_PATH"] += os.pathsep +
> config["WORKSPACE_DRIVERS"]
>  config["PACKAGES_PATH"] += os.pathsep + \
>  os.path.join(config["WORKSPACE"], "FSP")
> --
> 2.24.0.windows.2


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

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

[edk2-devel] How to change RAID Card's configuration without going into its setup ui

[Tiger Liu(BJ-RD)]

Hi, Experts:

I am studying how to remote control some hardware's configuration.

Such as changing RAID Card's configuration without going into its setup ui.

So, one question confused me.

If RAID Card's firmware is written with UEFI driver model, so I can use some 
standard UEFI protocol to pass some configurations to RAID card's firmware?

Or must use RAID Card vendor's private protocol to transfer new configurations?

Such as:

EFI Adapter Information Protocol, SetInformation function.

Thanks

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

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

Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

[Guomin Jiang]

It is ok, I have no others confusion.
Reviewed-by: Guomin Jiang 

> -Original Message-
> From: Michael Kubacki [mailto:michael.kuba...@outlook.com]
> Sent: Wednesday, April 1, 2020 1:12 AM
> To: Jiang, Guomin ; devel@edk2.groups.io; Wang,
> Jian J 
> Cc: Bret Barkelew ; Gao, Liming
> ; Kinney, Michael D ;
> Wu, Hao A 
> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
> GetVariable() attr if EFI_BUFFER_TOO_SMALL
> 
> That has been spelled incorrectly for about 9 years. The file (like many
> others) also has other spelling errors such as the following. I suggest this 
> be
> fixed in a separate commit/series focused on fixing spelling errors.
> 
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -102,7 +102,7 @@ AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut;
> This function writes data to the FWH at the correct LBA even if the LBAs
> are fragmented.
> 
> -  @param Global  Pointer to VARAIBLE_GLOBAL structure.
> +  @param Global  Pointer to VARIABLE_GLOBAL structure.
> @param VolatilePoint out the Variable is Volatile or
> Non-Volatile.
> @param SetByIndex  TRUE if target pointer is given as index.
>FALSE if target pointer is absolute.
> @@ -504,7 +504,7 @@ InitializeVariableQuota (
> 
> @return EFI_SUCCESS  Reclaim operation has finished
> successfully.
> @return EFI_OUT_OF_RESOURCES No enough memory resources or
> variable space.
> -  @return Others   Unexpect error happened during
> reclaim operation.
> +  @return Others   Unexpected error happened during
> reclaim operation.
> 
>   **/
>   EFI_STATUS
> @@ -2561,7 +2561,7 @@ VariableServiceSetVariable (
> }
> 
> //
> -  // Check for reserverd bit in variable attribute.
> +  // Check for reserved bit in variable attribute.
> // EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated but we
> still allow
> // the delete operation of common authenticated variable at user physical
> presence.
> // So leave EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute
> check to AuthVariableLib @@ -3381,7 +3381,7 @@
> ConvertNormalVarStorageToAuthVarStorage (
> VARIABLE_HEADER *StartPtr;
> UINT8   *NextPtr;
> VARIABLE_HEADER *EndPtr;
> -  UINTN   AuthVarStroageSize;
> +  UINTN   AuthVarStorageSize;
> AUTHENTICATED_VARIABLE_HEADER *AuthStartPtr;
> VARIABLE_STORE_HEADER *AuthVarStorage;
> 
> 
> On 3/31/2020 1:18 AM, Jiang, Guomin wrote:
> > There is a spell error in the comments of VariableServiceGetVariable() in
> Variable.c.
> > - @return EFI_BUFFER_TO_SMALL   DataSize is too small for the result.
> > + @return EFI_BUFFER_TOO_SMALLDataSize is too small for the result.
> >
> > Need create new bugs for it or fix in this comment directly?
> >
> >> -Original Message-
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >> Wang, Jian J
> >> Sent: Monday, March 30, 2020 12:16 PM
> >> To: michael.kuba...@outlook.com; devel@edk2.groups.io
> >> Cc: Bret Barkelew ; Gao, Liming
> >> ; Kinney, Michael D
> >> ; Wu, Hao A 
> >> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable:
> >> Return
> >> GetVariable() attr if EFI_BUFFER_TOO_SMALL
> >>
> >>
> >> Reviewed-by: Jian J Wang 
> >>
> >> Regards,
> >> Jian
> >>
> >>> -Original Message-
> >>> From: michael.kuba...@outlook.com 
> >>> Sent: Saturday, March 28, 2020 5:56 AM
> >>> To: devel@edk2.groups.io
> >>> Cc: Bret Barkelew ; Gao, Liming
> >>> ; Kinney, Michael D
> >>> ; Wang, Jian J ;
> >>> Wu, Hao A 
> >>> Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
> >>> attr if EFI_BUFFER_TOO_SMALL
> >>>
> >>> From: Michael Kubacki 
> >>>
> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >>>
> >>> The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> >>> "Attributes" parameter description states:
> >>>
> >>> "If not NULL, a pointer to the memory location to return the
> >>> attributes bitmask for the variable. See 'Related Definitions.'
> >>>   If not NULL, then Attributes is set on output both when
> >>> EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
> >>>
> >>> The attributes were previously only returned from the implementation
> >>> in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS
> >>> or EFI_BUFFER_TOO_SMALL according to spec.
> >>>
> >>> Cc: Bret Barkelew 
> >>> Cc: Liming Gao 
> >>> Cc: Michael D Kinney 
> >>> Cc: Jian J Wang 
> >>> Cc: Hao A Wu 
> >>> Signed-off-by: Michael Kubacki 
> >>> ---
> >>>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c  | 10
> >>> +++---
> >>>
> >>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> >> c |
> >>> 10 ++
> >>>   2 files changed, 13 insertions(+), 7 deletions(-)
> 

Re: [edk2-devel] [PATCH v3 0/3] Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

[Michael Kubacki]

I have those options set correctly in git config.

After a quick look, as far as I can tell, this is because the Message-Id 
in my email is modified such the the In-Reply-To and References headers 
do not refer to the actual Message-Id in the cover letter:


Cover letter:

Subject: [edk2-devel] [PATCH v3 0/3] Return GetVariable() attr if 
EFI_BUFFER_TOO_SMALL

Date: Fri, 27 Mar 2020 14:55:33 -0700
Message-Id: <160047f24e1d38f5.15...@groups.io>

The original Message-Id in the cover letter was moved to 
X-Microsoft-Original-Message-Id:


X-Microsoft-Original-Message-ID:
 <20200327215536.9556-1-michael.kuba...@outlook.com>

The first patch in the series:

Subject: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return 
GetVariable() attr if EFI_BUFFER_TOO_SMALL

Date: Fri, 27 Mar 2020 14:55:34 -0700
Message-Id: <160047f33e58ad06.17...@groups.io>
In-Reply-To: <20200327215536.9556-1-michael.kuba...@outlook.com>
References: <20200327215536.9556-1-michael.kuba...@outlook.com>

Please let me know if you have suggestions. I'll look into it more.

Thanks,
Michael

On 3/31/2020 4:55 PM, Laszlo Ersek wrote:

Hi Michael,

On 03/27/20 22:55, Michael Kubacki wrote:

From: Michael Kubacki 

This patch series updates the GetVariable() implementation
to return Attributes in the case EFI_BUFFER_TOO_SMALL is returned.

* [PATCH v3 1/3] Makes the functional change in the DXE/MM variable driver.
* [PATCH v3 2/3] Makes the functional change in the PEI variable driver.
* [PATCH v3 3/3] Removes a change made in NetworkPkg that worked around the
   previous behavior when EFI_BUFFER_TOO_SMALL is returned.

V3 changes:
* Apply the same GetVariable() behavior in VariablePei so it is consistent
   with the DXE/MM variable driver implementation.


Can you please configure your git-send-email machinery to post patch
series with "shallow threading"? Otherwise the patch emails are not
linked under the cover letter, and they fly apart.

I'm asking in particular because this series modifies both MdeModulePkg
and NetworkPkg. In such cases usually one of the affected maintainers
volunteers for merging the full set (once review is complete for the
entire set). And collecting the individual patches without proper
shallow threading is very difficult.

The git options are the following:

git config sendemail.chainreplyto false
git config sendemail.thread   true

They are also put in place (with many other useful settings) by
"BaseTools/Scripts/SetupGit.py".

Or is there another problem with posting the patches? (I assume you post
the full set with a single git-send-email invocation.)

Thanks!
Laszlo



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

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

[edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Wed, 04/01/2020 9:30am-10:30am #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Wednesday, 1 April 2020, 9:30am to 10:30am, (GMT+08:00) Asia/Chongqing

*Where:* https://zoom.com.cn/j/493235016

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

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

*Description:*

For more info please see:

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

Please download and import the following iCalendar (.ics) files to your 
calendar system.

Weekly: 
https://zoom.com.cn/meeting/uZ0udeqtrjosXIzjDALs3V00IXdg3REObA/ics?icsToken=98tyKu2hrTkpH9SUtlztArMtE535bPHukUlnsKl5vTP2UBYDMirOMuURAJdQEvmB

Join Zoom Meeting

https://zoom.com.cn/j/493235016

Meeting ID: 493 235 016

One tap mobile

+14086380968,,493235016# US (San Jose)

+16465588656,,493235016# US (New York)

Dial by your location

+1 408 638 0968 US (San Jose)

+1 646 558 8656 US (New York)

Meeting ID: 493 235 016

Find your local number: https://zoom.com.cn/u/idwYt3asgU

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

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

[edk2-devel] [PATCH] SecurityPkg/MeasureBootLib: Return EFI_ACCESS_DENIED after image check fail

[Guomin Jiang]

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

If check the File at the begin of function, it will only allow the File is
present and forbid image from buffer.
It is possible that image come from the memory buffer, so make it can run
and check the File after it.
It is improvement for 4b026f0d5af36faf3a3629a3ad49c51b5b3be12f.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Chao Zhang 
Signed-off-by: Guomin Jiang 
---
 .../DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c  | 14 +++---
 .../DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c| 14 +++---
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c 
b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
index f0e95e5ec0..fdb4758cbe 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
@@ -435,13 +435,6 @@ DxeTpm2MeasureBootHandler (
   EFI_PHYSICAL_ADDRESSFvAddress;
   UINT32  Index;
 
-  //
-  // Check for invalid parameters.
-  //
-  if (File == NULL) {
-return EFI_ACCESS_DENIED;
-  }
-
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
   if (EFI_ERROR (Status)) {
 //
@@ -615,6 +608,13 @@ DxeTpm2MeasureBootHandler (
   //
   Status = PeCoffLoaderGetImageInfo ();
   if (EFI_ERROR (Status)) {
+//
+// Check for invalid parameters.
+//
+if (File == NULL) {
+  Status = EFI_ACCESS_DENIED;
+}
+
 //
 // The information can't be got from the invalid PeImage
 //
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c 
b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index d499371e7a..20f7d94d6b 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -732,13 +732,6 @@ DxeTpmMeasureBootHandler (
   EFI_PHYSICAL_ADDRESSFvAddress;
   UINT32  Index;
 
-  //
-  // Check for invalid parameters.
-  //
-  if (File == NULL) {
-return EFI_ACCESS_DENIED;
-  }
-
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
   if (EFI_ERROR (Status)) {
 //
@@ -912,6 +905,13 @@ DxeTpmMeasureBootHandler (
   //
   Status = PeCoffLoaderGetImageInfo ();
   if (EFI_ERROR (Status)) {
+//
+// Check for invalid parameters.
+//
+if (File == NULL) {
+  return EFI_ACCESS_DENIED;
+}
+
 //
 // The information can't be got from the invalid PeImage
 //
-- 
2.25.1.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Allow pin file to be checked out when combo is not in project manifest

[Desimone, Ashley E]

When a pin file is based on a combo that is not in the project
manifest file but otherwise matches the project print a warning
instead of throwing and exception and allow the pin to be
checked out.

Signed-off-by: Ashley E Desimone 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/checkout_pin_command.py   | 10 --
 edkrepo/commands/humble/checkout_pin_humble.py |  4 +++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/edkrepo/commands/checkout_pin_command.py 
b/edkrepo/commands/checkout_pin_command.py
index 619fcf8..72075bf 100644
--- a/edkrepo/commands/checkout_pin_command.py
+++ b/edkrepo/commands/checkout_pin_command.py
@@ -90,11 +90,17 @@ class CheckoutPinCommand(EdkrepoCommand):
 elif not set(pin.remotes).issubset(set(manifest.remotes)):
 raise EdkrepoProjectMismatchException(humble.MANIFEST_MISMATCH)
 elif pin.general_config.current_combo not in [c.name for c in 
manifest.combinations]:
-raise EdkrepoProjectMismatchException(humble.MANIFEST_MISMATCH)
+
print(humble.COMBO_NOT_FOUND.format(pin.general_config.current_combo))
 combo_name = pin.general_config.current_combo
 pin_sources = pin.get_repo_sources(combo_name)
 pin_root_remote = {source.root:source.remote_name for source in 
pin_sources}
-manifest_sources = manifest.get_repo_sources(combo_name)
+try:
+# If the pin and the project manifest have the same combo get the
+# repo sources from that combo. Otherwise get the default combo's
+# repo sources
+manifest_sources = manifest.get_repo_sources(combo_name)
+except ValueError:
+manifest_sources = 
manifest.get_repo_sources(manifest.general_config.default_combo)
 manifest_root_remote = {source.root:source.remote_name for source in 
manifest_sources}
 if 
set(pin_root_remote.items()).isdisjoint(set(manifest_root_remote.items())):
 raise EdkrepoProjectMismatchException(humble.MANIFEST_MISMATCH)
diff --git a/edkrepo/commands/humble/checkout_pin_humble.py 
b/edkrepo/commands/humble/checkout_pin_humble.py
index ac7467d..ec6938d 100644
--- a/edkrepo/commands/humble/checkout_pin_humble.py
+++ b/edkrepo/commands/humble/checkout_pin_humble.py
@@ -12,4 +12,6 @@ NOT_FOUND = 'The selected PIN file was not found.'
 MANIFEST_MISMATCH = ('The selected PIN file does not refer to the same project 
'
  'as the local manifest file. {}'.format(CHP_EXIT))
 COMMIT_NOT_FOUND = 'The commit referenced by the PIN file does not exist. 
{}'.format(CHP_EXIT) 
-PIN_COMBO = 'Pin: {}'
\ No newline at end of file
+PIN_COMBO = 'Pin: {}'
+COMBO_NOT_FOUND = ('Warning: The combo listed in PIN file: {} is no longer '
+   'listed in the project manifest file.')
\ No newline at end of file
-- 
2.16.2.windows.1


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

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

Re: [edk2-devel] How to add ignore "IgnoreFiles" for CharEncodingCheck

[Sean via Groups.Io]

On Tue, Mar 31, 2020 at 08:52 AM, Zhang, Shenglei wrote:

> 
> 
> 
> "CharEncodingCheck": {
> 
> 
> 
> "IgnoreFiles":
> [[(MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/test/testc.c),(MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/windows/testc.c)]
> 
> 
> 
> 
> }
> 
> 

This syntax works for me.  The logic isn't very complex as it is just a case 
sensitive string match.

"CharEncodingCheck": {
"IgnoreFiles": 
["MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/test/testc.c", 
"MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/windows/testc.c"]
}

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

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

Re: [edk2-devel] [PATCH v2] NetworkPkg/Ip6Dxe: Improve Neightbor Discovery message validation.

[Laszlo Ersek]

On 03/31/20 14:22, Maciej Rabeda wrote:
> Hi Laszlo,
> 
> Thanks for trying this out!
> 
> The condition in the ASSERTs is reversed, consequently for the ASSERTs
> added in this function.
> I have added them to fire up when Ip6IsNDOptionValid() fails to properly
> react to invalid packet (return with an error and do not proceed with
> processing options).
> In this case, fire assert when current position within the packet
> (Offset) moved past the size of the option (Option.Length) * 8 (it's in
> 8-byte units) is outside the packet (further than first byte outside the
> packet).
> Offset one byte past the packet == last option ended at the end of the
> packet (packet size is Head->PayloadLength).

I understand. In your testing, both sides were probably equal, so the inverted 
relops didn't cause problems.

> Can you try swapping the >= to <= and rerun your test?
> This should apply to lines 2114, 2167 and 2337.
> 
> I will submit a patch for that.

The following patch works OK for me:

diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.c b/NetworkPkg/Ip6Dxe/Ip6Nd.c
index fd7f60b2f92c..0780a98cb325 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Nd.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c
@@ -2111,7 +2111,7 @@ Ip6ProcessRouterAdvertise (
   // Option size validity ensured by Ip6IsNDOptionValid().
   //
   ASSERT (LinkLayerOption.Length != 0);
-  ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 >= (UINT32) 
Head->PayloadLength);
+  ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 <= (UINT32) 
Head->PayloadLength);
 
   ZeroMem (, sizeof (EFI_MAC_ADDRESS));
   CopyMem (, LinkLayerOption.EtherAddr, 6);
@@ -2164,7 +2164,7 @@ Ip6ProcessRouterAdvertise (
   // Option size validity ensured by Ip6IsNDOptionValid().
   //
   ASSERT (PrefixOption.Length == 4);
-  ASSERT (Offset + (UINT32) PrefixOption.Length * 8 >= (UINT32) 
Head->PayloadLength);
+  ASSERT (Offset + (UINT32) PrefixOption.Length * 8 <= (UINT32) 
Head->PayloadLength);
 
   PrefixOption.ValidLifetime = NTOHL (PrefixOption.ValidLifetime);
   PrefixOption.PreferredLifetime = NTOHL (PrefixOption.PreferredLifetime);
@@ -2334,7 +2334,7 @@ Ip6ProcessRouterAdvertise (
   // Option size validity ensured by Ip6IsNDOptionValid().
   //
   ASSERT (MTUOption.Length == 1);
-  ASSERT (Offset + (UINT32) MTUOption.Length * 8 >= (UINT32) 
Head->PayloadLength);
+  ASSERT (Offset + (UINT32) MTUOption.Length * 8 <= (UINT32) 
Head->PayloadLength);
 
   //
   // Use IPv6 minimum link MTU 1280 bytes as the maximum packet size in 
order

If your posted patch will be identical to the above code changes, then you can 
add at once:

Tested-by: Laszlo Ersek 

Thanks!
Laszlo


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

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

Re: [edk2-devel] [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval"

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu 


> -Original Message-
> From: michael.kuba...@outlook.com 
> Sent: Saturday, March 28, 2020 5:56 AM
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek ; Fu, Siyuan ;
> Maciej Rabeda ; Wu, Jiaxin
> 
> Subject: [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix
> TlsCaCertificate attributes retrieval"
> 
> From: Michael Kubacki 
> 
> This reverts commit 6896efdec2709e530b23c688cf0f31706709a0c5.
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> 
> GetVariable() now returns attributes when it fails with
> EFI_BUFFER_TOO_SMALL. Therefore, commit 6896efdec270 is
> reverted since it is no longer relevant.
> 
> Cc: Laszlo Ersek 
> Cc: Siyuan Fu 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Signed-off-by: Michael Kubacki 
> Reviewed-by: Bret Barkelew 
> Reviewed-by: Laszlo Ersek 
> ---
>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 +---
>  1 file changed, 1 insertion(+), 26 deletions(-)
> 
> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> index 715bc3a0a941..2481d1098fa3 100644
> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> @@ -657,7 +657,6 @@ EnrollX509toVariable (
>EFI_SIGNATURE_LIST*CACert;
>EFI_SIGNATURE_DATA*CACertData;
>VOID  *Data;
> -  VOID  *CurrentData;
>UINTN DataSize;
>UINTN SigDataSize;
>UINT32Attr;
> @@ -669,7 +668,6 @@ EnrollX509toVariable (
>CACert= NULL;
>CACertData= NULL;
>Data  = NULL;
> -  CurrentData   = NULL;
>Attr  = 0;
> 
>Status = ReadFileContent (
> @@ -712,30 +710,11 @@ EnrollX509toVariable (
>Status = gRT->GetVariable(
>VariableName,
>,
> -  NULL,
> +  ,
>,
>NULL
>);
>if (Status == EFI_BUFFER_TOO_SMALL) {
> -//
> -// Per spec, we have to fetch the variable's contents, even though we're
> -// only interested in the variable's attributes.
> -//
> -CurrentData = AllocatePool (DataSize);
> -if (CurrentData == NULL) {
> -  Status = EFI_OUT_OF_RESOURCES;
> -  goto ON_EXIT;
> -}
> -Status = gRT->GetVariable(
> -VariableName,
> -,
> -,
> -,
> -CurrentData
> -);
> -if (EFI_ERROR (Status)) {
> -  goto ON_EXIT;
> -}
>  Attr |= EFI_VARIABLE_APPEND_WRITE;
>} else if (Status == EFI_NOT_FOUND) {
>  Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
> @@ -766,10 +745,6 @@ ON_EXIT:
>  FreePool (Data);
>}
> 
> -  if (CurrentData != NULL) {
> -FreePool (CurrentData);
> -  }
> -
>if (X509Data != NULL) {
>  FreePool (X509Data);
>}
> --
> 2.16.3.windows.1


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

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

Re: [edk2-devel] [PATCH v3 0/3] Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

[Laszlo Ersek]

Hi Michael,

On 03/27/20 22:55, Michael Kubacki wrote:
> From: Michael Kubacki 
> 
> This patch series updates the GetVariable() implementation
> to return Attributes in the case EFI_BUFFER_TOO_SMALL is returned.
> 
> * [PATCH v3 1/3] Makes the functional change in the DXE/MM variable driver.
> * [PATCH v3 2/3] Makes the functional change in the PEI variable driver.
> * [PATCH v3 3/3] Removes a change made in NetworkPkg that worked around the
>   previous behavior when EFI_BUFFER_TOO_SMALL is returned.
> 
> V3 changes:
> * Apply the same GetVariable() behavior in VariablePei so it is consistent
>   with the DXE/MM variable driver implementation.

Can you please configure your git-send-email machinery to post patch
series with "shallow threading"? Otherwise the patch emails are not
linked under the cover letter, and they fly apart.

I'm asking in particular because this series modifies both MdeModulePkg
and NetworkPkg. In such cases usually one of the affected maintainers
volunteers for merging the full set (once review is complete for the
entire set). And collecting the individual patches without proper
shallow threading is very difficult.

The git options are the following:

git config sendemail.chainreplyto false
git config sendemail.thread   true

They are also put in place (with many other useful settings) by
"BaseTools/Scripts/SetupGit.py".

Or is there another problem with posting the patches? (I assume you post
the full set with a single git-send-email invocation.)

Thanks!
Laszlo


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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Michael D Kinney]

Hi Sean,

This lib defines a global variable that is referenced when a compiler detects 
use of float/double types.  If the global is not referenced, then it should be 
optimized away, so the size impact should be zero.  That can be verified as 
part of the review of this feature.

Mike



From: devel@edk2.groups.io  On Behalf Of Sean via 
Groups.Io
Sent: Tuesday, March 31, 2020 3:58 PM
To: Laszlo Ersek ; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for 
float.

Does anyone know off hand if defining this and enabling floating point has any 
negative side effects if you don't need it?  Size? Optimization? Other?   That 
is my only concern for enabling in all modules which is why the initial 
proposal was for a new library.


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

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

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Display commands alphabetically

[Bjorge, Erik C]

Reviewed-by: Erik Bjorge 

-Original Message-
From: Desimone, Nathaniel L  
Sent: Friday, March 27, 2020 1:31 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E ; Pandya, Puja 
; Bjorge, Erik C ; Bret 
Barkelew ; Philippe Mathieu-Daudé 

Subject: [edk2-staging/EdkRepo] [PATCH] EdkRepo: Display commands alphabetically

Currently edkrepo --help shows a listing of commands that is not in 
alphabetical order. This is confusing.
This change makes the list shown in alphabetical order.

Cc: Ashley DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Bret Barkelew 
Cc: Philippe Mathieu-Daudé 
Signed-off-by: Nate DeSimone 
---
 edkrepo/commands/composite_command.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/edkrepo/commands/composite_command.py 
b/edkrepo/commands/composite_command.py
index b476445..ff53d3b 100644
--- a/edkrepo/commands/composite_command.py
+++ b/edkrepo/commands/composite_command.py
@@ -36,4 +36,4 @@ class CompositeCommand(object):
 command_names = []
 for command in self._commands:
 command_names.append(command.get_metadata()['name'])
-return command_names
+return sorted(command_names)
--
2.25.2


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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Sean via Groups.Io]

Does anyone know off hand if defining this and enabling floating point has any 
negative side effects if you don't need it?  Size? Optimization? Other?   That 
is my only concern for enabling in all modules which is why the initial 
proposal was for a new library.

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

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

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Improve state tracking when checking out pin files

[Bjorge, Erik C]

Reviewed-by: Erik Bjorge 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Desimone, Ashley 
E
Sent: Tuesday, March 31, 2020 2:00 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bjorge, Erik C ; Bret 
Barkelew ; Agyeman, Prince 

Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Improve state 
tracking when checking out pin files

Improves the state tracking when checking out onto a pin file
by: (1)moving the call to write_current_combo() after the succesfull checkout, 
(2)changing the name of the combo written to the format:
'Pin: {pinfilename}', (3)If the current combo is a knon pin file (starts with 
'Pin:') get_repo_sources() will return the repo sources from the default combo

Signed-off-by: Ashley E Desimone 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/checkout_pin_command.py   | 2 +-
 edkrepo/commands/humble/checkout_pin_humble.py | 3 ++-
 edkrepo_manifest_parser/edk_manifest.py| 4 
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/edkrepo/commands/checkout_pin_command.py 
b/edkrepo/commands/checkout_pin_command.py
index a2afc41..619fcf8 100644
--- a/edkrepo/commands/checkout_pin_command.py
+++ b/edkrepo/commands/checkout_pin_command.py
@@ -53,7 +53,6 @@ class CheckoutPinCommand(EdkrepoCommand):
 origin = repo.remotes.origin
 origin.fetch()
 self.__pin_matches_project(pin, manifest, workspace_path)
-manifest.write_current_combo(pin.general_config.current_combo)
 sparse_enabled = sparse_checkout_enabled(workspace_path, 
manifest_sources)
 if sparse_enabled:
 print(SPARSE_RESET)
@@ -61,6 +60,7 @@ class CheckoutPinCommand(EdkrepoCommand):
 pin_repo_sources = 
pin.get_repo_sources(pin.general_config.current_combo)
 try:
 checkout_repos(args.verbose, args.override, pin_repo_sources, 
workspace_path, manifest)
+
+ manifest.write_current_combo(humble.PIN_COMBO.format(args.pinfile))
 finally:
 if sparse_enabled:
 print(SPARSE_CHECKOUT)
diff --git a/edkrepo/commands/humble/checkout_pin_humble.py 
b/edkrepo/commands/humble/checkout_pin_humble.py
index b5a9cfb..ac7467d 100644
--- a/edkrepo/commands/humble/checkout_pin_humble.py
+++ b/edkrepo/commands/humble/checkout_pin_humble.py
@@ -11,4 +11,5 @@ CHP_EXIT = 'Exiting without checkout out PIN data.'
 NOT_FOUND = 'The selected PIN file was not found.'
 MANIFEST_MISMATCH = ('The selected PIN file does not refer to the same project 
'
  'as the local manifest file. {}'.format(CHP_EXIT)) 
-COMMIT_NOT_FOUND = 'The commit referenced by the PIN file does not exist. 
{}'.format(CHP_EXIT) \ No newline at end of file
+COMMIT_NOT_FOUND = 'The commit referenced by the PIN file does not 
+exist. {}'.format(CHP_EXIT) PIN_COMBO = 'Pin: {}'
\ No newline at end of file
diff --git a/edkrepo_manifest_parser/edk_manifest.py 
b/edkrepo_manifest_parser/edk_manifest.py
index dd3512b..2d3e79e 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -311,6 +311,10 @@ class ManifestXml(BaseXmlHelper):
 def get_repo_sources(self, combo_name):
 if combo_name in self.__combo_sources:
 return self._tuple_list(self.__combo_sources[combo_name])
+elif combo_name.startswith('Pin:'):
+# If currently checked out onto a pin file reture the sources in 
the
+# default combo
+return 
+ self._tuple_list(self.__combo_sources[self.general_config.default_comb
+ o])
 else:
 raise ValueError(COMB_INVALIDINPUT_ERROR.format(combo_name))
 
--
2.16.2.windows.1





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

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

[edk2-devel] [PATCH v2] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Liran Alon]

Previous to this change, PvScsiFreeRings() was not undoing all
operations that was done by PvScsiInitRings().
This is because PvScsiInitRings() was both preparing rings (Allocate
memory and map it for device DMA) and setup the rings against device by
issueing a device command. While PvScsiFreeRings() only unmaps the rings
and free their memory.

Driver do not have a functional error as it makes sure to reset device
before every call site to PvScsiFreeRings(). However, this is not
intuitive.

Therefore, prefer to refactor the setup of the ring against device to a
separate function than PvScsiInitRings().

Signed-off-by: Liran Alon 
---
 OvmfPkg/PvScsiDxe/PvScsi.c | 163 +++--
 1 file changed, 85 insertions(+), 78 deletions(-)

diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index 1ca50390c0e5..8c458ecceeb0 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.c
+++ b/OvmfPkg/PvScsiDxe/PvScsi.c
@@ -991,13 +991,6 @@ PvScsiInitRings (
   )
 {
   EFI_STATUS Status;
-  union {
-PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
-UINT32  Uint32;
-  } AlignedCmd;
-  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
-
-  Cmd = 
 
   Status = PvScsiAllocateSharedPages (
  Dev,
@@ -1032,6 +1025,69 @@ PvScsiInitRings (
   }
   ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
 
+  return EFI_SUCCESS;
+
+FreeRingReqs:
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingReqs,
+>RingDesc.RingReqsDmaDesc
+);
+
+FreeRingState:
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingState,
+>RingDesc.RingStateDmaDesc
+);
+
+  return Status;
+}
+
+STATIC
+VOID
+PvScsiFreeRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingCmps,
+>RingDesc.RingCmpsDmaDesc
+);
+
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingReqs,
+>RingDesc.RingReqsDmaDesc
+);
+
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingState,
+>RingDesc.RingStateDmaDesc
+);
+}
+
+STATIC
+EFI_STATUS
+PvScsiSetupRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  union {
+PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
+UINT32  Uint32;
+  } AlignedCmd;
+  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
+
+  Cmd = 
+
   ZeroMem (Cmd, sizeof (*Cmd));
   Cmd->ReqRingNumPages = 1;
   Cmd->CmpRingNumPages = 1;
@@ -1052,71 +1108,12 @@ PvScsiInitRings (
 sizeof (*Cmd) % sizeof (UINT32) == 0,
 "Cmd must be multiple of 32-bit words"
 );
-  Status = PvScsiWriteCmdDesc (
- Dev,
- PvScsiCmdSetupRings,
- (UINT32 *)Cmd,
- sizeof (*Cmd) / sizeof (UINT32)
- );
-  if (EFI_ERROR (Status)) {
-goto FreeRingCmps;
-  }
-
-  return EFI_SUCCESS;
-
-FreeRingCmps:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingCmps,
->RingDesc.RingCmpsDmaDesc
-);
-
-FreeRingReqs:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingReqs,
->RingDesc.RingReqsDmaDesc
-);
-
-FreeRingState:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingState,
->RingDesc.RingStateDmaDesc
-);
-
-  return Status;
-}
-
-STATIC
-VOID
-PvScsiFreeRings (
-  IN OUT PVSCSI_DEV *Dev
-  )
-{
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingCmps,
->RingDesc.RingCmpsDmaDesc
-);
-
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingReqs,
->RingDesc.RingReqsDmaDesc
-);
-
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingState,
->RingDesc.RingStateDmaDesc
-);
+  return PvScsiWriteCmdDesc (
+   Dev,
+   PvScsiCmdSetupRings,
+   (UINT32 *)Cmd,
+   sizeof (*Cmd) / sizeof (UINT32)
+   );
 }
 
 STATIC
@@ -1171,6 +1168,14 @@ PvScsiInit (
 goto FreeRings;
   }
 
+  //
+  // Setup rings against device
+  //
+  Status = PvScsiSetupRings (Dev);
+  if (EFI_ERROR (Status)) {
+goto FreeDMACommBuffer;
+  }
+
   //
   // Populate the exported interface's attributes
   //
@@ -1202,13 +1207,15 @@ PvScsiInit (
 
   return EFI_SUCCESS;
 
+FreeDMACommBuffer:
+  PvScsiFreeSharedPages (
+Dev,
+EFI_SIZE_TO_PAGES (sizeof (*Dev->DmaBuf)),
+Dev->DmaBuf,
+>DmaBufDmaDesc
+);
+
 FreeRings:
-  //
-  // Reset device to stop device usage of the rings.
-  // This is required to safely free the rings.
-  //
-  PvScsiResetAdapter (Dev);
-
   PvScsiFreeRings (Dev);
 
 RestorePciAttributes:
-- 
2.20.1


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

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

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

[Sean via Groups.Io]

I agree that safeintlib is not doing anything too interesting in this case but 
that's not really the point.  The argument for it is that it becomes the 
central point of code to check for safe conversions and an indicator that the 
developer was thoughtful about this conversion and didn't just cast to avoid 
the compiler complaining.  If everyone starts putting their own checks in place 
it leads to more code reviews, diversity in solutions, and opportunities for 
bugs.  All that said those are soft reasons for the change and that is up to 
you.

@Laszlo - On the ASSERT part, I have a different view point and am more curious 
about yours.  For release builds, I don't want to see CpuDeadLoops anywhere 
unless I am ok with the device being returned/refunded.  Our error path would 
be to exit the function with an error code and potentially log a 
ReportStatusCode.   I don't think you should continue in an invalid state as 
that just makes resolving the bug much much harder.    Given that the system 
can boot to at least a menu without this driver, it seems that failing out of 
the function would provide a better "RELEASE" experience.

Finally, given that this is contained in OVMF I am fine with whatever makes the 
most sense for your platform and usecase.

Thanks
Sean

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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 7/7] EdkRepo: Update List Repos for archived combos

[Bjorge, Erik C]

When running the List Repos command archived combos will not be listed
unless the archived flag is provided.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/list_repos_command.py | 37 ++
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/edkrepo/commands/list_repos_command.py 
b/edkrepo/commands/list_repos_command.py
index caf0373..b06a493 100644
--- a/edkrepo/commands/list_repos_command.py
+++ b/edkrepo/commands/list_repos_command.py
@@ -3,7 +3,7 @@
 ## @file
 # list_repos_command.py
 #
-# Copyright (c) 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -74,7 +74,10 @@ class ListReposCommand(EdkrepoCommand):
 xml_file = ci_index_xml.get_project_xml(project)
 manifest = 
ManifestXml(os.path.normpath(os.path.join(global_manifest_directory, xml_file)))
 manifests[project] = manifest
-for combo in [c.name for c in manifest.combinations]:
+combo_list = [c.name for c in manifest.combinations]
+if args.archived:
+combo_list.extend([c.name for c in 
manifest.archived_combinations])
+for combo in combo_list:
 sources = manifest.get_repo_sources(combo)
 for source in sources:
 repo_urls.add(self.get_repo_url(source.remote_url))
@@ -84,7 +87,7 @@ class ListReposCommand(EdkrepoCommand):
 project_justify = len(max(manifests.keys(), key=len))
 
 #Determine the names of the repositories
-self.generate_repo_names(repo_urls, manifests)
+self.generate_repo_names(repo_urls, manifests, args.archived)
 print(humble.REPOSITORIES)
 
 #If the user provided a list of repositories to view, check to make 
sure
@@ -103,7 +106,10 @@ class ListReposCommand(EdkrepoCommand):
 #Determine the list of branches that used by any branch 
combination in any manifest
 branches = set()
 for project_name in manifests:
-for combo in [c.name for c in 
manifests[project_name].combinations]:
+combo_list = [c.name for c in 
manifests[project_name].combinations]
+if args.archived:
+combo_list.extend([c.name for c in 
manifests[project_name].archived_combinations])
+for combo in combo_list:
 sources = manifests[project_name].get_repo_sources(combo)
 for source in sources:
 if self.get_repo_url(source.remote_url) == repo:
@@ -124,7 +130,10 @@ class ListReposCommand(EdkrepoCommand):
 #Determine the branch combinations that use that branch
 for project_name in manifests:
 combos = []
-for combo in [c.name for c in 
manifests[project_name].combinations]:
+combo_list = [c.name for c in 
manifests[project_name].combinations]
+if args.archived:
+combo_list.extend([c.name for c in 
manifests[project_name].archived_combinations])
+for combo in combo_list:
 sources = 
manifests[project_name].get_repo_sources(combo)
 for source in sources:
 if self.get_repo_url(source.remote_url) == repo 
and source.branch == branch:
@@ -165,11 +174,11 @@ class ListReposCommand(EdkrepoCommand):
 return name
 raise EdkrepoInvalidParametersException(humble.REPO_NAME_NOT_FOUND)
 
-def generate_repo_names(self, repo_urls, manifests):
+def generate_repo_names(self, repo_urls, manifests, archived=False):
 #Determine the names of the repositories
 self.repo_names = collections.OrderedDict()
 for repo_url in repo_urls:
-self.__repo_name_worker(repo_url, manifests)
+self.__repo_name_worker(repo_url, manifests, archived)
 
 #Sort the git repositories so they will be displayed alphabetically
 self.repo_names = 
collections.OrderedDict(sorted(self.repo_names.items()))
@@ -188,12 +197,15 @@ class ListReposCommand(EdkrepoCommand):
 for name_to_move in names_to_move:
 self.repo_names.move_to_end(name_to_move, False)
 
-def __repo_name_worker(self, repo_url, manifests):
+def __repo_name_worker(self, repo_url, manifests, archived=False):
 #This is a heuristic that guesses the "name" of a repository by looking
 #at the name given to it by the most manifest files.
 names = collections.defaultdict(int)
 for project_name in manifests:
-for combo in [c.name for c in 
manifests[project_name].combinations]:
+combo_list = [c.name for c in manifests[project_name].combinations]
+if 

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 5/7] EdkRepo: Update Checkout Pin for archived combos

[Bjorge, Erik C]

Added support for archived combos in the Checkout Pin command.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/checkout_pin_command.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/edkrepo/commands/checkout_pin_command.py 
b/edkrepo/commands/checkout_pin_command.py
index a2afc41..858271a 100644
--- a/edkrepo/commands/checkout_pin_command.py
+++ b/edkrepo/commands/checkout_pin_command.py
@@ -15,7 +15,7 @@ from edkrepo.commands.edkrepo_command import EdkrepoCommand, 
OverrideArgument
 import edkrepo.commands.arguments.checkout_pin_args as arguments
 import edkrepo.commands.humble.checkout_pin_humble as humble
 from edkrepo.common.common_repo_functions import sparse_checkout_enabled, 
reset_sparse_checkout, sparse_checkout
-from edkrepo.common.common_repo_functions import check_dirty_repos, 
checkout_repos
+from edkrepo.common.common_repo_functions import check_dirty_repos, 
checkout_repos, combinations_in_manifest
 from edkrepo.common.humble import SPARSE_CHECKOUT, SPARSE_RESET
 from edkrepo.common.edkrepo_exception import 
EdkrepoInvalidParametersException, EdkrepoProjectMismatchException
 from edkrepo.config.config_factory import get_workspace_path, 
get_workspace_manifest
@@ -89,7 +89,7 @@ class CheckoutPinCommand(EdkrepoCommand):
 raise EdkrepoProjectMismatchException(humble.MANIFEST_MISMATCH)
 elif not set(pin.remotes).issubset(set(manifest.remotes)):
 raise EdkrepoProjectMismatchException(humble.MANIFEST_MISMATCH)
-elif pin.general_config.current_combo not in [c.name for c in 
manifest.combinations]:
+elif pin.general_config.current_combo not in 
combinations_in_manifest(manifest):
 raise EdkrepoProjectMismatchException(humble.MANIFEST_MISMATCH)
 combo_name = pin.general_config.current_combo
 pin_sources = pin.get_repo_sources(combo_name)
-- 
2.21.0.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 3/7] EdkRepo: Update Checkout for archived combos

[Bjorge, Erik C]

Now either an active or archived branch combination can be checked out.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/common/common_repo_functions.py | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/edkrepo/common/common_repo_functions.py 
b/edkrepo/common/common_repo_functions.py
index 288dd27..160127b 100644
--- a/edkrepo/common/common_repo_functions.py
+++ b/edkrepo/common/common_repo_functions.py
@@ -494,8 +494,14 @@ def sort_commits(manifest, workspace_path, 
max_commits=None):
 return sorted_commit_list
 
 
-def combination_is_in_manifest(combination, manifest):
+def combinations_in_manifest(manifest):
 combination_names = [c.name for c in manifest.combinations]
+combination_names.extend([c.name for c in manifest.archived_combinations])
+return combination_names
+
+
+def combination_is_in_manifest(combination, manifest):
+combination_names = combinations_in_manifest(manifest)
 return combination in combination_names
 
 
@@ -557,7 +563,7 @@ def checkout(combination_or_sha, verbose=False, 
override=False, log=None):
 combo_or_sha = combination_or_sha
 try:
 # Try to handle normalize combo name to match the manifest file.
-combo_or_sha = case_insensitive_single_match(combo_or_sha, [x.name for 
x in manifest.combinations])
+combo_or_sha = case_insensitive_single_match(combo_or_sha, 
combinations_in_manifest())
 except:
 # No match so leave it alone.  It must be a SHA1 or a bad combo name.
 pass
-- 
2.21.0.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 2/7] EdkRepo: Added ability to display archived combinations

[Bjorge, Erik C]

Added support for using the -a / --archived flags to include archived
combinations.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/arguments/combo_args.py |  5 +++--
 edkrepo/commands/combo_command.py| 19 +--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/edkrepo/commands/arguments/combo_args.py 
b/edkrepo/commands/arguments/combo_args.py
index af3ded9..dabb464 100644
--- a/edkrepo/commands/arguments/combo_args.py
+++ b/edkrepo/commands/arguments/combo_args.py
@@ -3,7 +3,7 @@
 ## @file
 # combo_args.py
 #
-# Copyright (c) 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -11,4 +11,5 @@
 combo command meta data.
 '''
 
-COMMAND_DESCRIPTION = 'Displays the currently checked out combination and 
lists all available combinations.'
\ No newline at end of file
+COMMAND_DESCRIPTION = 'Displays the currently checked out combination and 
lists all available combinations.'
+ARCHIVED_HELP = 'Include a listing of archived combinations.'
diff --git a/edkrepo/commands/combo_command.py 
b/edkrepo/commands/combo_command.py
index 68d6854..9e13f47 100644
--- a/edkrepo/commands/combo_command.py
+++ b/edkrepo/commands/combo_command.py
@@ -3,10 +3,11 @@
 ## @file
 # combo_command.py
 #
-# Copyright (c) 2017- 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2017- 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 from colorama import Fore
+from colorama import Style
 
 from edkrepo.commands.edkrepo_command import EdkrepoCommand
 from edkrepo.commands.edkrepo_command import ColorArgument
@@ -25,6 +26,11 @@ class ComboCommand(EdkrepoCommand):
 metadata['help-text'] = arguments.COMMAND_DESCRIPTION
 args = []
 metadata['arguments'] = args
+args.append({'name': 'archived',
+ 'short-name': 'a',
+ 'positional': False,
+ 'required': False,
+ 'help-text': arguments.ARCHIVED_HELP})
 args.append(ColorArgument)
 return metadata
 
@@ -32,9 +38,18 @@ class ComboCommand(EdkrepoCommand):
 init_color_console(args.color)
 
 manifest = get_workspace_manifest()
-for combo in [c.name for c in manifest.combinations]:
+combo_archive = []
+combo_list = [c.name for c in manifest.combinations]
+if args.archived:
+combo_archive = [c.name for c in manifest.archived_combinations]
+combo_list.extend(combo_archive)
+if manifest.general_config.current_combo not in combo_list:
+combo_list.append(manifest.general_config.current_combo)
+for combo in sorted(combo_list):
 if combo == manifest.general_config.current_combo:
 print("* {}{}{}".format(Fore.GREEN, combo, Fore.RESET))
+elif combo in combo_archive:
+print("  {}{}{}{}".format(Fore.YELLOW, Style.BRIGHT, combo, 
Style.RESET_ALL))
 else:
 print("  {}".format(combo))
 if args.verbose:
-- 
2.21.0.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 4/7] EdkRepo: Update Sync for archived combos

[Bjorge, Erik C]

Added support for archived combos in Sync command.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/sync_command.py | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/edkrepo/commands/sync_command.py b/edkrepo/commands/sync_command.py
index 71c600a..daa558f 100644
--- a/edkrepo/commands/sync_command.py
+++ b/edkrepo/commands/sync_command.py
@@ -3,7 +3,7 @@
 ## @file
 # sync_command.py
 #
-# Copyright (c) 2017- 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2017- 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -19,7 +19,7 @@ from git import Repo
 # Our modules
 from edkrepo.commands.edkrepo_command import EdkrepoCommand
 from edkrepo.commands.edkrepo_command import DryRunArgument, 
SubmoduleSkipArgument
-import edkrepo.commands.arguments.sync_args as arguments
+import edkrepo.commands.arguments.sync_args as arguments
 from edkrepo.common.progress_handler import GitProgressHandler
 from edkrepo.common.edkrepo_exception import 
EdkrepoUncommitedChangesException, EdkrepoManifestNotFoundException
 from edkrepo.common.edkrepo_exception import EdkrepoManifestChangedException
@@ -38,7 +38,7 @@ from edkrepo.common.common_repo_functions import 
generate_name_for_obsolete_back
 from edkrepo.common.common_repo_functions import update_editor_config
 from edkrepo.common.common_repo_functions import update_repo_commit_template, 
get_latest_sha
 from edkrepo.common.common_repo_functions import has_primary_repo_remote, 
fetch_from_primary_repo, in_sync_with_primary
-from edkrepo.common.common_repo_functions import update_hooks, 
maintain_submodules
+from edkrepo.common.common_repo_functions import update_hooks, 
maintain_submodules, combinations_in_manifest
 from edkrepo.common.common_repo_functions import write_included_config, 
remove_included_config
 from edkrepo.common.ui_functions import init_color_console
 from edkrepo.config.config_factory import get_workspace_path, 
get_workspace_manifest, get_edkrepo_global_data_directory
@@ -53,22 +53,22 @@ class SyncCommand(EdkrepoCommand):
 def get_metadata(self):
 metadata = {}
 metadata['name'] = 'sync'
-metadata['help-text'] = arguments.COMMAND_DESCRIPTION
+metadata['help-text'] = arguments.COMMAND_DESCRIPTION
 args = []
 metadata['arguments'] = args
 args.append({'name' : 'fetch',
  'positional' : False,
  'required' : False,
- 'help-text': arguments.FETCH_HELP})
+ 'help-text': arguments.FETCH_HELP})
 args.append({'name' : 'update-local-manifest',
  'short-name': 'u',
  'required' : False,
- 'help-text' : arguments.UPDATE_LOCAL_MANIFEST_HELP})
+ 'help-text' : arguments.UPDATE_LOCAL_MANIFEST_HELP})
 args.append({'name' : 'override',
  'short-name': 'o',
  'positional' : False,
  'required' : False,
- 'help-text' : arguments.OVERRIDE_HELP})
+ 'help-text' : arguments.OVERRIDE_HELP})
 args.append(SubmoduleSkipArgument)
 return metadata
 
@@ -222,7 +222,7 @@ class SyncCommand(EdkrepoCommand):
 if initial_manifest_remotes[remote_name] != 
new_manifest_remotes[remote_name]:
 raise 
EdkrepoManifestChangedException(SYNC_URL_CHANGE.format(remote_name))
 #check to see if the currently checked out combo exists in the new 
manifest.
-new_combos = [c.name for c in new_manifest_to_check.combinations]
+new_combos = combinations_in_manifest(new_manifest_to_check)
 if current_combo not in new_combos:
 raise 
EdkrepoManifestChangedException(SYNC_COMBO_CHANGE.format(current_combo,

initial_manifest.project_info.codename))
-- 
2.21.0.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 6/7] EdkRepo: Update clone for archived combos

[Bjorge, Erik C]

Adding support for archived combos in the clone command.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/clone_command.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/edkrepo/commands/clone_command.py 
b/edkrepo/commands/clone_command.py
index 2400272..701a853 100644
--- a/edkrepo/commands/clone_command.py
+++ b/edkrepo/commands/clone_command.py
@@ -3,7 +3,7 @@
 ## @file
 # clone_command.py
 #
-# Copyright (c) 2017- 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2017- 2020, Intel Corporation. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
@@ -16,7 +16,7 @@ import edkrepo.commands.arguments.clone_args as arguments
 from edkrepo.common.common_repo_functions import pull_latest_manifest_repo, 
clone_repos, sparse_checkout, verify_manifest_data
 from edkrepo.common.common_repo_functions import 
case_insensitive_single_match, update_editor_config
 from edkrepo.common.common_repo_functions import write_included_config, 
write_conditional_include
-from edkrepo.common.common_repo_functions import find_project_in_index
+from edkrepo.common.common_repo_functions import find_project_in_index, 
combinations_in_manifest
 from edkrepo.common.edkrepo_exception import 
EdkrepoInvalidParametersException, EdkrepoManifestInvalidException
 from edkrepo.common.humble import CLONE_INVALID_WORKSPACE, 
CLONE_INVALID_PROJECT_ARG, CLONE_INVALID_COMBO_ARG
 from edkrepo.common.humble import SPARSE_CHECKOUT, CLONE_INVALID_LOCAL_ROOTS
@@ -99,7 +99,7 @@ class CloneCommand(EdkrepoCommand):
 combo_name = None
 if args.Combination is not None:
 try:
-combo_name = case_insensitive_single_match(args.Combination, 
[x.name for x in manifest.combinations])
+combo_name = case_insensitive_single_match(args.Combination, 
combinations_in_manifest(manifest))
 except:
 #remove the repo directory and Manifest.xml from the workspace 
so the next time the user trys to clone
 #they will have an empty workspace and then raise an exception
-- 
2.21.0.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 1/7] EdkRepo: Adding support for archiving combos

[Bjorge, Erik C]

Adding support to check the archived attribute on branch combos.  This
allows a combo to be archived and available if required but not dirty
up the combo list.

Signed-off-by: Erik Bjorge 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo_manifest_parser/edk_manifest.py | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/edkrepo_manifest_parser/edk_manifest.py 
b/edkrepo_manifest_parser/edk_manifest.py
index dd3512b..7b513dc 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -306,7 +306,11 @@ class ManifestXml(BaseXmlHelper):
 
 @property
 def combinations(self):
-return self._tuple_list(self.__combinations.values())
+return self._tuple_list([x for x in self.__combinations.values() if 
not x.archived])
+
+@property
+def archived_combinations(self):
+return self._tuple_list([x for x in self.__combinations.values() if 
x.archived])
 
 def get_repo_sources(self, combo_name):
 if combo_name in self.__combo_sources:
@@ -645,6 +649,10 @@ class _Combination():
 self.description = element.attrib['description']
 except:
 self.description = None   #description is optional attribute
+try:
+self.archived = (element.attrib['archived'].lower() == 'true')
+except:
+self.archived = False
 
 @property
 def tuple(self):
-- 
2.21.0.windows.1


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 0/7] Adding support for archiving branch combos

[Bjorge, Erik C]

Adding the ability to mark a branch combination as archived.  This will
remove it from the list of valid combinations by default.  It should not
limit users from accessing the branch combination.  The archive flag
will allow users to list archived branch combinations in the combo
command.

Erik Bjorge (7):
  EdkRepo: Adding support for archiving combos
  EdkRepo: Added ability to display archived combinations
  EdkRepo: Update Checkout for archived combos
  EdkRepo: Update Sync for archived combos
  EdkRepo: Update Checkout Pin for archived combos
  EdkRepo: Update clone for archived combos
  EdkRepo: Update List Repos for archived combos

 edkrepo/commands/arguments/combo_args.py |  5 ++--
 edkrepo/commands/checkout_pin_command.py |  4 +--
 edkrepo/commands/clone_command.py|  6 ++--
 edkrepo/commands/combo_command.py| 19 ++--
 edkrepo/commands/list_repos_command.py   | 37 +---
 edkrepo/commands/sync_command.py | 16 +-
 edkrepo/common/common_repo_functions.py  | 10 +--
 edkrepo_manifest_parser/edk_manifest.py  | 10 ++-
 8 files changed, 76 insertions(+), 31 deletions(-)

-- 
2.21.0.windows.1


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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Laszlo Ersek]

On 03/31/20 16:36, Kinney, Michael D wrote:
> ARM and AARCH64 have a compiler intrinsic lib that is linked against all 
> modules.
> 
> [LibraryClasses.ARM, LibraryClasses.AARCH64]
>   #
>   # It is not possible to prevent ARM compiler calls to generic intrinsic 
> functions.
>   # This library provides the instrinsic functions generated by a given 
> compiler.
>   # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
>   #
>   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> 
> Can we use this same technique for IA32/X64

It's not a problem for in-tree platforms (I do hope the edk2 patch
series will include the patch for OvmfPkg), but it will require all
out-of-tree platforms to be updated.

> VS builds?

Yes; I think the library instance should consist of one totally empty C
file, and an |MSFT specific C file providing the external definition of
_fltused. When building for GCC, the lib instance should compile to an
empty object (archive) file.

In my opinion, of course.

Thanks
Laszlo


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

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

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Liran Alon]

On 01/04/2020 1:19, Laszlo Ersek wrote:

Hi Liran,

On 03/31/20 13:47, Liran Alon wrote:

Previous to this change, PvScsiFreeRings() was not undoing all
operations that was done by PvScsiInitRings().
This is because PvScsiInitRings() was both preparing rings (Allocate
memory and map it for device DMA) and setup the rings against device by
issueing a device command. While PvScsiFreeRings() only unmaps the rings
and free their memory.

Driver do not have a functional error as it makes sure to reset device
before every call site to PvScsiFreeRings(). However, this is not
intuitive.

Therefore, prefer to refactor the setup of the ring against device to a
separate function than PvScsiInitRings().

Signed-off-by: Liran Alon 
---
  OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++--
  1 file changed, 76 insertions(+), 74 deletions(-)

Thanks for the patch.

I didn't expect this approach -- hoisting the rings exposure from
SetupRings to Init --, but it turns out that I like it more than my own
previous suggestion -- pushing ResetAdapter into FreeRings :)
Yeah I thought about it later and liked it to. Hoped you will think the 
same ;)


May I just trouble you with one further request?


diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index 1ca50390c0e5..5b7fdcbda10b 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.c
+++ b/OvmfPkg/PvScsiDxe/PvScsi.c
@@ -991,13 +991,6 @@ PvScsiInitRings (
)
  {
EFI_STATUS Status;
-  union {
-PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
-UINT32  Uint32;
-  } AlignedCmd;
-  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
-
-  Cmd = 
  
Status = PvScsiAllocateSharedPages (

   Dev,
@@ -1032,6 +1025,69 @@ PvScsiInitRings (
}
ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
  
+  return EFI_SUCCESS;

+
+FreeRingReqs:
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingReqs,
+>RingDesc.RingReqsDmaDesc
+);
+
+FreeRingState:
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingState,
+>RingDesc.RingStateDmaDesc
+);
+
+  return Status;
+}
+
+STATIC
+VOID
+PvScsiFreeRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingCmps,
+>RingDesc.RingCmpsDmaDesc
+);
+
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingReqs,
+>RingDesc.RingReqsDmaDesc
+);
+
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingState,
+>RingDesc.RingStateDmaDesc
+);
+}
+
+STATIC
+EFI_STATUS
+PvScsiSetupRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  union {
+PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
+UINT32  Uint32;
+  } AlignedCmd;
+  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
+
+  Cmd = 
+
ZeroMem (Cmd, sizeof (*Cmd));
Cmd->ReqRingNumPages = 1;
Cmd->CmpRingNumPages = 1;
@@ -1052,71 +1108,12 @@ PvScsiInitRings (
  sizeof (*Cmd) % sizeof (UINT32) == 0,
  "Cmd must be multiple of 32-bit words"
  );
-  Status = PvScsiWriteCmdDesc (
- Dev,
- PvScsiCmdSetupRings,
- (UINT32 *)Cmd,
- sizeof (*Cmd) / sizeof (UINT32)
- );
-  if (EFI_ERROR (Status)) {
-goto FreeRingCmps;
-  }
-
-  return EFI_SUCCESS;
-
-FreeRingCmps:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingCmps,
->RingDesc.RingCmpsDmaDesc
-);
-
-FreeRingReqs:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingReqs,
->RingDesc.RingReqsDmaDesc
-);
-
-FreeRingState:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingState,
->RingDesc.RingStateDmaDesc
-);
-
-  return Status;
-}
-
-STATIC
-VOID
-PvScsiFreeRings (
-  IN OUT PVSCSI_DEV *Dev
-  )
-{
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingCmps,
->RingDesc.RingCmpsDmaDesc
-);
-
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingReqs,
->RingDesc.RingReqsDmaDesc
-);
-
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingState,
->RingDesc.RingStateDmaDesc
-);
+  return PvScsiWriteCmdDesc (
+   Dev,
+   PvScsiCmdSetupRings,
+   (UINT32 *)Cmd,
+   sizeof (*Cmd) / sizeof (UINT32)
+   );
  }
  
  STATIC

@@ -1157,6 +1154,10 @@ PvScsiInit (
if (EFI_ERROR (Status)) {
  goto RestorePciAttributes;
}
+  Status = PvScsiSetupRings (Dev);
+  if (EFI_ERROR (Status)) {
+goto FreeRings;
+  }

If you could move the PvScsiSetupRings() call *below* the DMA buffer
allocation (and rework the error path accordingly -- basically replacing
the "UnsetupRings" action with a "FreeDmaBuffer" action), then this
patch would be *perfect*.

Because then, the PvScsiUninit() order of actions would be a 100% mirror
for PvScsiInit()'s; namely with PvScsiResetAdapter() at the top of
PvScsiUninit() perfectly matching PvScsiSetupRings() at the end of
PvScsiInit()!

I don't want to annoy you too much, so if you really want to stick with
this 

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Laszlo Ersek]

Hi Liran,

On 03/31/20 13:47, Liran Alon wrote:
> Previous to this change, PvScsiFreeRings() was not undoing all
> operations that was done by PvScsiInitRings().
> This is because PvScsiInitRings() was both preparing rings (Allocate
> memory and map it for device DMA) and setup the rings against device by
> issueing a device command. While PvScsiFreeRings() only unmaps the rings
> and free their memory.
> 
> Driver do not have a functional error as it makes sure to reset device
> before every call site to PvScsiFreeRings(). However, this is not
> intuitive.
> 
> Therefore, prefer to refactor the setup of the ring against device to a
> separate function than PvScsiInitRings().
> 
> Signed-off-by: Liran Alon 
> ---
>  OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++--
>  1 file changed, 76 insertions(+), 74 deletions(-)

Thanks for the patch.

I didn't expect this approach -- hoisting the rings exposure from
SetupRings to Init --, but it turns out that I like it more than my own
previous suggestion -- pushing ResetAdapter into FreeRings :)

May I just trouble you with one further request?

> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index 1ca50390c0e5..5b7fdcbda10b 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -991,13 +991,6 @@ PvScsiInitRings (
>)
>  {
>EFI_STATUS Status;
> -  union {
> -PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
> -UINT32  Uint32;
> -  } AlignedCmd;
> -  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
> -
> -  Cmd = 
>  
>Status = PvScsiAllocateSharedPages (
>   Dev,
> @@ -1032,6 +1025,69 @@ PvScsiInitRings (
>}
>ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
>  
> +  return EFI_SUCCESS;
> +
> +FreeRingReqs:
> +  PvScsiFreeSharedPages (
> +Dev,
> +1,
> +Dev->RingDesc.RingReqs,
> +>RingDesc.RingReqsDmaDesc
> +);
> +
> +FreeRingState:
> +  PvScsiFreeSharedPages (
> +Dev,
> +1,
> +Dev->RingDesc.RingState,
> +>RingDesc.RingStateDmaDesc
> +);
> +
> +  return Status;
> +}
> +
> +STATIC
> +VOID
> +PvScsiFreeRings (
> +  IN OUT PVSCSI_DEV *Dev
> +  )
> +{
> +  PvScsiFreeSharedPages (
> +Dev,
> +1,
> +Dev->RingDesc.RingCmps,
> +>RingDesc.RingCmpsDmaDesc
> +);
> +
> +  PvScsiFreeSharedPages (
> +Dev,
> +1,
> +Dev->RingDesc.RingReqs,
> +>RingDesc.RingReqsDmaDesc
> +);
> +
> +  PvScsiFreeSharedPages (
> +Dev,
> +1,
> +Dev->RingDesc.RingState,
> +>RingDesc.RingStateDmaDesc
> +);
> +}
> +
> +STATIC
> +EFI_STATUS
> +PvScsiSetupRings (
> +  IN OUT PVSCSI_DEV *Dev
> +  )
> +{
> +  union {
> +PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
> +UINT32  Uint32;
> +  } AlignedCmd;
> +  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
> +
> +  Cmd = 
> +
>ZeroMem (Cmd, sizeof (*Cmd));
>Cmd->ReqRingNumPages = 1;
>Cmd->CmpRingNumPages = 1;
> @@ -1052,71 +1108,12 @@ PvScsiInitRings (
>  sizeof (*Cmd) % sizeof (UINT32) == 0,
>  "Cmd must be multiple of 32-bit words"
>  );
> -  Status = PvScsiWriteCmdDesc (
> - Dev,
> - PvScsiCmdSetupRings,
> - (UINT32 *)Cmd,
> - sizeof (*Cmd) / sizeof (UINT32)
> - );
> -  if (EFI_ERROR (Status)) {
> -goto FreeRingCmps;
> -  }
> -
> -  return EFI_SUCCESS;
> -
> -FreeRingCmps:
> -  PvScsiFreeSharedPages (
> -Dev,
> -1,
> -Dev->RingDesc.RingCmps,
> ->RingDesc.RingCmpsDmaDesc
> -);
> -
> -FreeRingReqs:
> -  PvScsiFreeSharedPages (
> -Dev,
> -1,
> -Dev->RingDesc.RingReqs,
> ->RingDesc.RingReqsDmaDesc
> -);
> -
> -FreeRingState:
> -  PvScsiFreeSharedPages (
> -Dev,
> -1,
> -Dev->RingDesc.RingState,
> ->RingDesc.RingStateDmaDesc
> -);
> -
> -  return Status;
> -}
> -
> -STATIC
> -VOID
> -PvScsiFreeRings (
> -  IN OUT PVSCSI_DEV *Dev
> -  )
> -{
> -  PvScsiFreeSharedPages (
> -Dev,
> -1,
> -Dev->RingDesc.RingCmps,
> ->RingDesc.RingCmpsDmaDesc
> -);
> -
> -  PvScsiFreeSharedPages (
> -Dev,
> -1,
> -Dev->RingDesc.RingReqs,
> ->RingDesc.RingReqsDmaDesc
> -);
> -
> -  PvScsiFreeSharedPages (
> -Dev,
> -1,
> -Dev->RingDesc.RingState,
> ->RingDesc.RingStateDmaDesc
> -);
> +  return PvScsiWriteCmdDesc (
> +   Dev,
> +   PvScsiCmdSetupRings,
> +   (UINT32 *)Cmd,
> +   sizeof (*Cmd) / sizeof (UINT32)
> +   );
>  }
>  
>  STATIC
> @@ -1157,6 +1154,10 @@ PvScsiInit (
>if (EFI_ERROR (Status)) {
>  goto RestorePciAttributes;
>}
> +  Status = PvScsiSetupRings (Dev);
> +  if (EFI_ERROR (Status)) {
> +goto FreeRings;
> +  }

If you could move the PvScsiSetupRings() call *below* the DMA buffer
allocation (and rework the error path accordingly -- basically replacing
the "UnsetupRings" action with a "FreeDmaBuffer" action), then this
patch would be *perfect*.

Because then, the PvScsiUninit() order of actions would be a 100% mirror

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

[Liran Alon]

On 01/04/2020 1:13, Liran Alon wrote:


On 01/04/2020 0:56, Laszlo Ersek wrote:

On 03/31/20 17:53, Sean via Groups.Io wrote:

A couple of thoughts.
1. I would suggest that ASSERT should not be the only protection for 
an invalid operation as ASSERT is usually disabled on release builds.
2. We do have a library to make this more explicit and common. 
https://urldefense.com/v3/__https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/SafeIntLib.h*L548__;Iw!!GqivPVa7Brio!LNp76poqa4dly7M8C8F3aX66wzZA68yStF_9jS-pD3izw3uJ24i_mDygFJEBsLc$ 


In this case, when "Response->ScsiStatus" does not fit in
"Packet->TargetStatus", the device model is obviously (and blatantly)
misbehaving, so I would agree with Liran that trying to recover from
that (or to cover it up with a nice error code passed out) is futile.

Exactly.


I do agree with the observation however that ASSERT()s disappear from
RELEASE builds.

Mike Kinney taught me a pattern to deal with this. There are various
ways to write it; one example (for this case) is:

   ASSERT (Response->ScsiStatus <= MAX_UINT8);
   if (Response->ScsiStatus > MAX_UINT8) {
 CpuDeadLoop ();
   }
   Packet->TargetStatus = (UINT8)Response->ScsiStatus;

An alternative way to write it is (by moving the ASSERT into the block):

   if (Response->ScsiStatus > MAX_UINT8) {
 ASSERT (Response->ScsiStatus <= MAX_UINT8);
 CpuDeadLoop ();
   }
   Packet->TargetStatus = (UINT8)Response->ScsiStatus;

Yet another (simply assert FALSE in the block):

   if (Response->ScsiStatus > MAX_UINT8) {
 ASSERT (FALSE);
 CpuDeadLoop ();
   }
   Packet->TargetStatus = (UINT8)Response->ScsiStatus;


Why:

- in DEBUG builds, the assertion failure will be logged, and the proper
assertion failure action will be triggered (CpuDeadLoop / exception /
..., as configured by the platform)

- in RELEASE builds, we'll still hang, and might have a chance to
investigate (get a stack dump perhaps).

Regarding SafeIntLib, I'm a fan in general. In this case, I did not
think of it (possible integer truncations seem so rare in this driver).
For this patch, I'm OK either way (with or without using SafeIntLib), as
long as we add both the ASSERT and the explicit CpuDeadLoop (either
variant of the three).

Thanks
Laszlo
Honestly, I don't quite understand why using SafeIntLib is useful in 
this case.
It just internally does even more branches and checks for exactly same 
overflow and return RETURN_BUFFER_TOO_SMALL in case value is bigger 
than MAX_UINT8.
Externally, I would still need to do a check on SafeUint16ToUint8() 
return-value. So what's the benefit?... Seems to me to just be an 
useless overhead.
I believe checking against MAX_UINT8 and casting immediately one line 
afterwards, is explicit enough.


Regarding above comment that ASSERT() doesn't do anything for RELEASE 
builds:
The point in ASSERT() is to be able to check a condition early to 
assist debugging but not worth putting this condition in RELEASE as it 
should never happen and just waste CPU cycles.
I thought this is the case we have here. If a weird ScsiStatus would 
return, it is highly unlikely that boot would just succeed as usual, 
and if it does, does the user really care?
In contrast, when boot fails because of this, it makes sense to build 
in DEBUG in attempt to verify what happened.
Note that if this condition will ever evaluate to FALSE (i.e. 
ScsiStatus is bigger than MAX_UINT8), then it is probably very 
deterministic. As it means PVSCSI device emulation on host is broken
and broke because of a very specific commit on host userspace VMM 
(E.g. QEMU).
Therefore, I still think an ASSERT() is what fits here best. But if 
you still think otherwise, then I have no objection to change it (Or 
Laszlo change it when applying).
I would say though, that if the pattern above is common, why isn't 
there a macro in EDK2 for that instead of manually writing it? 
Something like:


#define RELEASE_ASSERT(Cond) \
   if (!(Cond)) {    \
 ASSERT (FALSE);    \
 CpuDeadLoop ();   \
   }

That would be more elegant.

-Liran

I would also mention that there are some bizzare code in EDK2 that 
defines it's own ASSERT() macro that just does CpuDeadLoop().
E.g. ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c and 
OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c)


-Liran



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

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

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

[Liran Alon]

On 01/04/2020 0:56, Laszlo Ersek wrote:

On 03/31/20 17:53, Sean via Groups.Io wrote:

A couple of thoughts.
1. I would suggest that ASSERT should not be the only protection for an invalid 
operation as ASSERT is usually disabled on release builds.
2. We do have a library to make this more explicit and common. 
https://urldefense.com/v3/__https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/SafeIntLib.h*L548__;Iw!!GqivPVa7Brio!LNp76poqa4dly7M8C8F3aX66wzZA68yStF_9jS-pD3izw3uJ24i_mDygFJEBsLc$

In this case, when "Response->ScsiStatus" does not fit in
"Packet->TargetStatus", the device model is obviously (and blatantly)
misbehaving, so I would agree with Liran that trying to recover from
that (or to cover it up with a nice error code passed out) is futile.

Exactly.


I do agree with the observation however that ASSERT()s disappear from
RELEASE builds.

Mike Kinney taught me a pattern to deal with this. There are various
ways to write it; one example (for this case) is:

   ASSERT (Response->ScsiStatus <= MAX_UINT8);
   if (Response->ScsiStatus > MAX_UINT8) {
 CpuDeadLoop ();
   }
   Packet->TargetStatus = (UINT8)Response->ScsiStatus;

An alternative way to write it is (by moving the ASSERT into the block):

   if (Response->ScsiStatus > MAX_UINT8) {
 ASSERT (Response->ScsiStatus <= MAX_UINT8);
 CpuDeadLoop ();
   }
   Packet->TargetStatus = (UINT8)Response->ScsiStatus;

Yet another (simply assert FALSE in the block):

   if (Response->ScsiStatus > MAX_UINT8) {
 ASSERT (FALSE);
 CpuDeadLoop ();
   }
   Packet->TargetStatus = (UINT8)Response->ScsiStatus;


Why:

- in DEBUG builds, the assertion failure will be logged, and the proper
assertion failure action will be triggered (CpuDeadLoop / exception /
..., as configured by the platform)

- in RELEASE builds, we'll still hang, and might have a chance to
investigate (get a stack dump perhaps).

Regarding SafeIntLib, I'm a fan in general. In this case, I did not
think of it (possible integer truncations seem so rare in this driver).
For this patch, I'm OK either way (with or without using SafeIntLib), as
long as we add both the ASSERT and the explicit CpuDeadLoop (either
variant of the three).

Thanks
Laszlo
Honestly, I don't quite understand why using SafeIntLib is useful in 
this case.
It just internally does even more branches and checks for exactly same 
overflow and return RETURN_BUFFER_TOO_SMALL in case value is bigger than 
MAX_UINT8.
Externally, I would still need to do a check on SafeUint16ToUint8() 
return-value. So what's the benefit?... Seems to me to just be an 
useless overhead.
I believe checking against MAX_UINT8 and casting immediately one line 
afterwards, is explicit enough.


Regarding above comment that ASSERT() doesn't do anything for RELEASE 
builds:
The point in ASSERT() is to be able to check a condition early to assist 
debugging but not worth putting this condition in RELEASE as it should 
never happen and just waste CPU cycles.
I thought this is the case we have here. If a weird ScsiStatus would 
return, it is highly unlikely that boot would just succeed as usual, and 
if it does, does the user really care?
In contrast, when boot fails because of this, it makes sense to build in 
DEBUG in attempt to verify what happened.
Note that if this condition will ever evaluate to FALSE (i.e. ScsiStatus 
is bigger than MAX_UINT8), then it is probably very deterministic. As it 
means PVSCSI device emulation on host is broken
and broke because of a very specific commit on host userspace VMM (E.g. 
QEMU).
Therefore, I still think an ASSERT() is what fits here best. But if you 
still think otherwise, then I have no objection to change it (Or Laszlo 
change it when applying).
I would say though, that if the pattern above is common, why isn't there 
a macro in EDK2 for that instead of manually writing it? Something like:


#define RELEASE_ASSERT(Cond) \
   if (!(Cond)) {    \
 ASSERT (FALSE);    \
 CpuDeadLoop ();   \
   }

That would be more elegant.

-Liran


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

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

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

[Laszlo Ersek]

On 03/31/20 17:53, Sean via Groups.Io wrote:
> A couple of thoughts.
> 1. I would suggest that ASSERT should not be the only protection for an 
> invalid operation as ASSERT is usually disabled on release builds.
> 2. We do have a library to make this more explicit and common. 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/SafeIntLib.h#L548

In this case, when "Response->ScsiStatus" does not fit in
"Packet->TargetStatus", the device model is obviously (and blatantly)
misbehaving, so I would agree with Liran that trying to recover from
that (or to cover it up with a nice error code passed out) is futile.

I do agree with the observation however that ASSERT()s disappear from
RELEASE builds.

Mike Kinney taught me a pattern to deal with this. There are various
ways to write it; one example (for this case) is:

  ASSERT (Response->ScsiStatus <= MAX_UINT8);
  if (Response->ScsiStatus > MAX_UINT8) {
CpuDeadLoop ();
  }
  Packet->TargetStatus = (UINT8)Response->ScsiStatus;

An alternative way to write it is (by moving the ASSERT into the block):

  if (Response->ScsiStatus > MAX_UINT8) {
ASSERT (Response->ScsiStatus <= MAX_UINT8);
CpuDeadLoop ();
  }
  Packet->TargetStatus = (UINT8)Response->ScsiStatus;

Yet another (simply assert FALSE in the block):

  if (Response->ScsiStatus > MAX_UINT8) {
ASSERT (FALSE);
CpuDeadLoop ();
  }
  Packet->TargetStatus = (UINT8)Response->ScsiStatus;


Why:

- in DEBUG builds, the assertion failure will be logged, and the proper
assertion failure action will be triggered (CpuDeadLoop / exception /
..., as configured by the platform)

- in RELEASE builds, we'll still hang, and might have a chance to
investigate (get a stack dump perhaps).

Regarding SafeIntLib, I'm a fan in general. In this case, I did not
think of it (possible integer truncations seem so rare in this driver).
For this patch, I'm OK either way (with or without using SafeIntLib), as
long as we add both the ASSERT and the explicit CpuDeadLoop (either
variant of the three).

Thanks
Laszlo


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

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

Re: [edk2-devel] [PATCH] Maintainers.txt: Add Liran and Nikita as OvmfPkg/PvScsiDxe reviewers

[Nikita Leshenko]

> On 31 Mar 2020, at 14:02, Liran Alon  wrote:
> 
> Laszlo suggested that as I have contributed the OvmfPkg PVSCSI driver, I
> will also register myself as a reviewer in Maintainers.txt.
> 
> In addition, as Nikita have assisted the development of the PVSCSI
> driver and have developed another similar OvmfPkg SCSI driver, add him
> as a reviewer to PVSCSI driver as-well.
> 
> Cc: Nikita Leshenko 
> Suggested-by: Laszlo Ersek 
> Signed-off-by: Liran Alon 
> ---
> Maintainers.txt | 5 +
> 1 file changed, 5 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 342bb8d0850c..de443ba7ba1f 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -435,6 +435,11 @@ OvmfPkg: CSM modules
> F: OvmfPkg/Csm/
> R: David Woodhouse 
> 
> +OvmfPkg: PVSCSI driver
> +F: OvmfPkg/PvScsiDxe
> +R: Liran Alon 
> +R: Nikita Leshenko 
> +
> PcAtChipsetPkg
> F: PcAtChipsetPkg/
> W: 
> https://urldefense.com/v3/__https://github.com/tianocore/tianocore.github.io/wiki/PcAtChipsetPkg__;!!GqivPVa7Brio!MOGqjthxmkgBKMVAnKhDxWbILitfHbgG_puzaCEDOatoBRhq0l6DSnrM-WOGI_dh-GTfLw$
>  
> -- 
> 2.20.1

Reviewed-by: Nikita Leshenko 
> 
> 
> 
> 
> 


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

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

Re: [edk2-devel] [PATCH] Maintainers.txt: Add Liran and Nikita as OvmfPkg/PvScsiDxe reviewers

[Laszlo Ersek]

On 03/31/20 13:02, Liran Alon wrote:
> Laszlo suggested that as I have contributed the OvmfPkg PVSCSI driver, I
> will also register myself as a reviewer in Maintainers.txt.
> 
> In addition, as Nikita have assisted the development of the PVSCSI
> driver and have developed another similar OvmfPkg SCSI driver, add him
> as a reviewer to PVSCSI driver as-well.
> 
> Cc: Nikita Leshenko 
> Suggested-by: Laszlo Ersek 
> Signed-off-by: Liran Alon 
> ---
>  Maintainers.txt | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 342bb8d0850c..de443ba7ba1f 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -435,6 +435,11 @@ OvmfPkg: CSM modules
>  F: OvmfPkg/Csm/
>  R: David Woodhouse 
>  
> +OvmfPkg: PVSCSI driver
> +F: OvmfPkg/PvScsiDxe

A trailing slash character is missing from this pattern (see the "F:"
documentation near the top of this file), but I do take the blame for
that, because my suggestion didn't contain the trailing slash either. So
I can fix this up for you, before I push.

Reviewed-by: Laszlo Ersek 

Nikita, can you please give your R-b as well?

Thanks,
Laszlo


> +R: Liran Alon 
> +R: Nikita Leshenko 
> +
>  PcAtChipsetPkg
>  F: PcAtChipsetPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/PcAtChipsetPkg
> 


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

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

[edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Improve state tracking when checking out pin files

[Desimone, Ashley E]

Improves the state tracking when checking out onto a pin file
by: (1)moving the call to write_current_combo() after the succesfull
checkout, (2)changing the name of the combo written to the format:
'Pin: {pinfilename}', (3)If the current combo is a knon pin file
(starts with 'Pin:') get_repo_sources() will return the repo sources
from the default combo

Signed-off-by: Ashley E Desimone 
Cc: Nate DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Bret Barkelew 
Cc: Prince Agyeman 
---
 edkrepo/commands/checkout_pin_command.py   | 2 +-
 edkrepo/commands/humble/checkout_pin_humble.py | 3 ++-
 edkrepo_manifest_parser/edk_manifest.py| 4 
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/edkrepo/commands/checkout_pin_command.py 
b/edkrepo/commands/checkout_pin_command.py
index a2afc41..619fcf8 100644
--- a/edkrepo/commands/checkout_pin_command.py
+++ b/edkrepo/commands/checkout_pin_command.py
@@ -53,7 +53,6 @@ class CheckoutPinCommand(EdkrepoCommand):
 origin = repo.remotes.origin
 origin.fetch()
 self.__pin_matches_project(pin, manifest, workspace_path)
-manifest.write_current_combo(pin.general_config.current_combo)
 sparse_enabled = sparse_checkout_enabled(workspace_path, 
manifest_sources)
 if sparse_enabled:
 print(SPARSE_RESET)
@@ -61,6 +60,7 @@ class CheckoutPinCommand(EdkrepoCommand):
 pin_repo_sources = 
pin.get_repo_sources(pin.general_config.current_combo)
 try:
 checkout_repos(args.verbose, args.override, pin_repo_sources, 
workspace_path, manifest)
+manifest.write_current_combo(humble.PIN_COMBO.format(args.pinfile))
 finally:
 if sparse_enabled:
 print(SPARSE_CHECKOUT)
diff --git a/edkrepo/commands/humble/checkout_pin_humble.py 
b/edkrepo/commands/humble/checkout_pin_humble.py
index b5a9cfb..ac7467d 100644
--- a/edkrepo/commands/humble/checkout_pin_humble.py
+++ b/edkrepo/commands/humble/checkout_pin_humble.py
@@ -11,4 +11,5 @@ CHP_EXIT = 'Exiting without checkout out PIN data.'
 NOT_FOUND = 'The selected PIN file was not found.'
 MANIFEST_MISMATCH = ('The selected PIN file does not refer to the same project 
'
  'as the local manifest file. {}'.format(CHP_EXIT))
-COMMIT_NOT_FOUND = 'The commit referenced by the PIN file does not exist. 
{}'.format(CHP_EXIT) 
\ No newline at end of file
+COMMIT_NOT_FOUND = 'The commit referenced by the PIN file does not exist. 
{}'.format(CHP_EXIT) 
+PIN_COMBO = 'Pin: {}'
\ No newline at end of file
diff --git a/edkrepo_manifest_parser/edk_manifest.py 
b/edkrepo_manifest_parser/edk_manifest.py
index dd3512b..2d3e79e 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -311,6 +311,10 @@ class ManifestXml(BaseXmlHelper):
 def get_repo_sources(self, combo_name):
 if combo_name in self.__combo_sources:
 return self._tuple_list(self.__combo_sources[combo_name])
+elif combo_name.startswith('Pin:'):
+# If currently checked out onto a pin file reture the sources in 
the
+# default combo
+return 
self._tuple_list(self.__combo_sources[self.general_config.default_combo])
 else:
 raise ValueError(COMB_INVALIDINPUT_ERROR.format(combo_name))
 
-- 
2.16.2.windows.1


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

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

Re: [edk2-devel] [PATCH] NetworkPkg/UefiPxeBcDxe: handle competing DHCP servers (more) gracefully

[Maciej Rabeda]

Always better than not detecting such stuff at all (or by ASSERT in 
debug builds). Thanks for the patch!


Reviewed-by: Maciej Rabeda 

On 31-Mar-20 02:47, Laszlo Ersek wrote:

When DHCP is misconfigured on a network segment, such that two DHCP
servers attempt to reply to requests (and therefore race with each other),
the edk2 PXE client can confuse itself.

In PxeBcDhcp4BootInfo() / PxeBcDhcp6BootInfo(), the client may refer to a
DHCP reply packet as an "earlier" packet from the "same" DHCP server, when
in reality both packets are unrelated, and arrive from different DHCP
servers.

While the edk2 PXE client can do nothing to fix this, it should at least
not ASSERT() -- ASSERT() is for catching programming errors (violations of
invariants that are under the control of the programmer). ASSERT()s should
in particular not refer to external data (such as network packets). What's
more, in RELEASE builds, we get NULL pointer references.

Check the problem conditions with actual "if"s, and return
EFI_PROTOCOL_ERROR. This will trickle out to PxeBcLoadBootFile(), and be
reported as "PXE-E99: Unexpected network error".

Cc: Jiaxin Wu 
Cc: Maciej Rabeda 
Cc: Philippe Mathieu-Daudé 
Cc: Siyuan Fu 
Signed-off-by: Laszlo Ersek 
---

Notes:
 Repo:   https://pagure.io/lersek/edk2.git
 Branch: dhcp_assert

  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 30 ++--
  1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index 10bbb06f7593..d062a526077b 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -482,7 +482,20 @@ PxeBcDhcp4BootInfo (
  Cache4 = >DhcpAck.Dhcp4;
}
  
-  ASSERT (Cache4->OptList[PXEBC_DHCP4_TAG_INDEX_BOOTFILE] != NULL);

+  if (Cache4->OptList[PXEBC_DHCP4_TAG_INDEX_BOOTFILE] == NULL) {
+//
+// This should never happen in a correctly configured DHCP / PXE
+// environment. One misconfiguration that can cause it is two DHCP servers
+// mistakenly running on the same network segment at the same time, and
+// racing each other in answering DHCP requests. Thus, the DHCP packets
+// that the edk2 PXE client considers "belonging together" may actually be
+// entirely independent, coming from two (competing) DHCP servers.
+//
+// Try to deal with this gracefully. Note that this check is not
+// comprehensive, as we don't try to identify all such errors.
+//
+return EFI_PROTOCOL_ERROR;
+  }
  
//

// Parse the boot server address.
@@ -612,7 +625,20 @@ PxeBcDhcp6BootInfo (
  Cache6 = >DhcpAck.Dhcp6;
}
  
-  ASSERT (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] != NULL);

+  if (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] == NULL) {
+//
+// This should never happen in a correctly configured DHCP / PXE
+// environment. One misconfiguration that can cause it is two DHCP servers
+// mistakenly running on the same network segment at the same time, and
+// racing each other in answering DHCP requests. Thus, the DHCP packets
+// that the edk2 PXE client considers "belonging together" may actually be
+// entirely independent, coming from two (competing) DHCP servers.
+//
+// Try to deal with this gracefully. Note that this check is not
+// comprehensive, as we don't try to identify all such errors.
+//
+return EFI_PROTOCOL_ERROR;
+  }
  
//

// Set the station address to IP layer.


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

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

Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow.

[Maciej Rabeda]

Much appreciated, I'll submit the patch tomorrow :)

On 31-Mar-20 16:50, Gao, Zhichao wrote:

Acked-by: Zhichao Gao 


-Original Message-
From: Fu, Siyuan 
Sent: Tuesday, March 31, 2020 7:54 PM
To: devel@edk2.groups.io; ler...@redhat.com; Ni, Ray ;
Gao, Zhichao 
Cc: maciej.rab...@linux.intel.com
Subject: RE: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive
flow.

Reviewed-by: Siyuan Fu 


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Laszlo
Ersek
Sent: 2020年3月25日 19:34
To: Ni, Ray ; Gao, Zhichao 
Cc: devel@edk2.groups.io; maciej.rab...@linux.intel.com
Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4
receive flow.

Ray, Zhichao,

On 02/27/20 12:02, Maciej Rabeda wrote:

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

'ping' command's receive flow utilizes a single Rx token which it
attempts to reuse before recycling the previously received packet.
This causes a situation where under ICMP traffic,
Ping6OnEchoReplyReceived() function will receive an already recycled
packet with EFI_SUCCESS token status and finally dereference invalid
pointers from RxData structure.

Cc: Ray Ni 
Cc: Zhichao Gao 
Signed-off-by: Maciej Rabeda 
---
  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

can you please review this ShellPkg patch? It's been on the list for
almost a month now.

Thanks
Laszlo


diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c

b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c

index 23567fa2c1bb..a3fa32515192 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived (

  ON_EXIT:

+  //
+  // Recycle the packet before reusing RxToken  //
+ gBS->SignalEvent (Private->IpChoice ==

PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-

RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private
- RxToken.Packet.RxData)->RecycleSignal);
+
if (Private->RxCount < Private->SendNum) {
  //
  // Continue to receive icmp echo reply packets.
@@ -632,10 +637,6 @@ ON_EXIT:
  //
  Private->Status = EFI_SUCCESS;
}
-  //
-  // Singal to recycle the each rxdata here, not at the end of process.
-  //
-  gBS->SignalEvent (Private->IpChoice ==

PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-

RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private
- RxToken.Packet.RxData)->RecycleSignal);
  }

  /**










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

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

Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

[Michael Kubacki]

That has been spelled incorrectly for about 9 years. The file (like many 
others) also has other spelling errors such as the following. I suggest 
this be fixed in a separate commit/series focused on fixing spelling errors.


--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -102,7 +102,7 @@ AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut;
   This function writes data to the FWH at the correct LBA even if the LBAs
   are fragmented.

-  @param Global  Pointer to VARAIBLE_GLOBAL structure.
+  @param Global  Pointer to VARIABLE_GLOBAL structure.
   @param VolatilePoint out the Variable is Volatile or 
Non-Volatile.

   @param SetByIndex  TRUE if target pointer is given as index.
  FALSE if target pointer is absolute.
@@ -504,7 +504,7 @@ InitializeVariableQuota (

   @return EFI_SUCCESS  Reclaim operation has finished 
successfully.
   @return EFI_OUT_OF_RESOURCES No enough memory resources or 
variable space.
-  @return Others   Unexpect error happened during 
reclaim operation.
+  @return Others   Unexpected error happened during 
reclaim operation.


 **/
 EFI_STATUS
@@ -2561,7 +2561,7 @@ VariableServiceSetVariable (
   }

   //
-  // Check for reserverd bit in variable attribute.
+  // Check for reserved bit in variable attribute.
   // EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated but we 
still allow
   // the delete operation of common authenticated variable at user 
physical presence.
   // So leave EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute check 
to AuthVariableLib

@@ -3381,7 +3381,7 @@ ConvertNormalVarStorageToAuthVarStorage (
   VARIABLE_HEADER *StartPtr;
   UINT8   *NextPtr;
   VARIABLE_HEADER *EndPtr;
-  UINTN   AuthVarStroageSize;
+  UINTN   AuthVarStorageSize;
   AUTHENTICATED_VARIABLE_HEADER *AuthStartPtr;
   VARIABLE_STORE_HEADER *AuthVarStorage;


On 3/31/2020 1:18 AM, Jiang, Guomin wrote:

There is a spell error in the comments of VariableServiceGetVariable() in 
Variable.c.
- @return EFI_BUFFER_TO_SMALL   DataSize is too small for the result.
+ @return EFI_BUFFER_TOO_SMALLDataSize is too small for the result.

Need create new bugs for it or fix in this comment directly?


-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Wang, Jian J
Sent: Monday, March 30, 2020 12:16 PM
To: michael.kuba...@outlook.com; devel@edk2.groups.io
Cc: Bret Barkelew ; Gao, Liming
; Kinney, Michael D ;
Wu, Hao A 
Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
GetVariable() attr if EFI_BUFFER_TOO_SMALL


Reviewed-by: Jian J Wang 

Regards,
Jian


-Original Message-
From: michael.kuba...@outlook.com 
Sent: Saturday, March 28, 2020 5:56 AM
To: devel@edk2.groups.io
Cc: Bret Barkelew ; Gao, Liming
; Kinney, Michael D
; Wang, Jian J ;
Wu, Hao A 
Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
attr if EFI_BUFFER_TOO_SMALL

From: Michael Kubacki 

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

The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
"Attributes" parameter description states:

"If not NULL, a pointer to the memory location to return the
attributes bitmask for the variable. See 'Related Definitions.'
  If not NULL, then Attributes is set on output both when  EFI_SUCCESS
and when EFI_BUFFER_TOO_SMALL is returned."

The attributes were previously only returned from the implementation
in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
EFI_BUFFER_TOO_SMALL according to spec.

Cc: Bret Barkelew 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Jian J Wang 
Cc: Hao A Wu 
Signed-off-by: Michael Kubacki 
---
  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c  | 10
+++---


MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
c |

10 ++
  2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index d23aea4bc712..1e71fc642c76 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -18,6 +18,8 @@

  Copyright (c) 2006 - 2020, Intel Corporation. All rights
reserved.
  (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP
+Copyright (c) Microsoft Corporation.
+
  SPDX-License-Identifier: BSD-2-Clause-Patent

  **/
@@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
  }

  CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
-if (Attributes != NULL) {
-  *Attributes = Variable.CurrPtr->Attributes;
-}

  *DataSize = VarDataSize;
  UpdateVariableInfo (VariableName, VendorGuid, 

Re: [edk2-devel] [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

[Ard Biesheuvel]

On 3/31/20 6:26 PM, Sean via Groups.Io wrote:

On Mon, Mar 30, 2020 at 11:41 PM, Ard Biesheuvel wrote:

Not sure I follow. Which command line are we talking about?

@Ard - In this Platform CI, ArmVirt is building and running AARCH64 but 
not ARM 32bit.  Would it be valuable to build for ARM too?




Yes, absolutely.

I prototyped it but want to make sure I am calling QEMU with the 
parameters you would expect to work with ArmVirtPkg


qemu-system-arm -M virt -cpu cortex-a15 -pflash 
/home/vsts/work/1/s/Build/ArmVirtQemu-ARM/DEBUG_GCC5/FV/QEMU_EFI.fd -m 
1024 -net none -serial stdio -drive 
file=fat:rw:/home/vsts/work/1/s/Build/ArmVirtQemu-ARM/DEBUG_GCC5/VirtualDrive,format=raw,media=disk 
-display none




The parameters look fine. Note that you can use the qemu-system-aarch64 
binary as well, so no need to install both.




PR build results can be seen here: 
https://dev.azure.com/tianocore/edk2-ci-play/_build/results?buildId=5074=results

PR code change: https://github.com/spbrogan/edk2/pull/14


Yep, looking good. The main focus is obviously on X64 and AARCH64, but 
that only increases the risk that we might regress on IA32 or ARM 
without anyone noticing. So having both IA32 and ARM is a good thing.



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

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

Re: [edk2-devel] [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

[Sean via Groups.Io]

On Mon, Mar 30, 2020 at 11:41 PM, Ard Biesheuvel wrote:

> 
> Not sure I follow. Which command line are we talking about?

@Ard - In this Platform CI, ArmVirt is building and running AARCH64 but not ARM 
32bit.  Would it be valuable to build for ARM too?

I prototyped it but want to make sure I am calling QEMU with the parameters you 
would expect to work with ArmVirtPkg

qemu-system-arm -M virt -cpu cortex-a15 -pflash 
/home/vsts/work/1/s/Build/ArmVirtQemu-ARM/DEBUG_GCC5/FV/QEMU_EFI.fd -m 1024 
-net none -serial stdio -drive 
file=fat:rw:/home/vsts/work/1/s/Build/ArmVirtQemu-ARM/DEBUG_GCC5/VirtualDrive,format=raw,media=disk
 -display none

PR build results can be seen here: 
https://dev.azure.com/tianocore/edk2-ci-play/_build/results?buildId=5074=results
PR code change: https://github.com/spbrogan/edk2/pull/14

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

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

Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

[Sean via Groups.Io]

A couple of thoughts.
1. I would suggest that ASSERT should not be the only protection for an invalid 
operation as ASSERT is usually disabled on release builds.
2. We do have a library to make this more explicit and common. 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/SafeIntLib.h#L548

Thanks

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

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

[edk2-devel] How to add ignore "IgnoreFiles" for CharEncodingCheck

[Zhang, Shenglei]

Hi Sean,

I am introducing third party project oniguruma as submodule into edk2, and want 
to skip CharEncodingCheck for certain files in oniguruma.
I tried to add changes like below, but CI build failed.
"CharEncodingCheck": {
"IgnoreFiles": 
[[(MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/test/testc.c),(MdeModulePkg/Universal/RegularExpressionDxe/oniguruma/windows/testc.c)]
}

So what should I do in CharEncodingCheck_plug_in.yaml? Or how do you handle 
this kind of case like openssl?

I see you add this to edk2 and hope you could resolve my query. Thanks in 
advance.

Best Regards,
Shenglei


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

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

Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow.

[Gao, Zhichao]

Acked-by: Zhichao Gao 

> -Original Message-
> From: Fu, Siyuan 
> Sent: Tuesday, March 31, 2020 7:54 PM
> To: devel@edk2.groups.io; ler...@redhat.com; Ni, Ray ;
> Gao, Zhichao 
> Cc: maciej.rab...@linux.intel.com
> Subject: RE: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive
> flow.
> 
> Reviewed-by: Siyuan Fu 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Laszlo
> > Ersek
> > Sent: 2020年3月25日 19:34
> > To: Ni, Ray ; Gao, Zhichao 
> > Cc: devel@edk2.groups.io; maciej.rab...@linux.intel.com
> > Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4
> > receive flow.
> >
> > Ray, Zhichao,
> >
> > On 02/27/20 12:02, Maciej Rabeda wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032
> > >
> > > 'ping' command's receive flow utilizes a single Rx token which it
> > > attempts to reuse before recycling the previously received packet.
> > > This causes a situation where under ICMP traffic,
> > > Ping6OnEchoReplyReceived() function will receive an already recycled
> > > packet with EFI_SUCCESS token status and finally dereference invalid
> > > pointers from RxData structure.
> > >
> > > Cc: Ray Ni 
> > > Cc: Zhichao Gao 
> > > Signed-off-by: Maciej Rabeda 
> > > ---
> > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > can you please review this ShellPkg patch? It's been on the list for
> > almost a month now.
> >
> > Thanks
> > Laszlo
> >
> > > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > index 23567fa2c1bb..a3fa32515192 100644
> > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > @@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived (
> > >
> > >  ON_EXIT:
> > >
> > > +  //
> > > +  // Recycle the packet before reusing RxToken  //
> > > + gBS->SignalEvent (Private->IpChoice ==
> > PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
> > >RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private
> > >- RxToken.Packet.RxData)->RecycleSignal);
> > > +
> > >if (Private->RxCount < Private->SendNum) {
> > >  //
> > >  // Continue to receive icmp echo reply packets.
> > > @@ -632,10 +637,6 @@ ON_EXIT:
> > >  //
> > >  Private->Status = EFI_SUCCESS;
> > >}
> > > -  //
> > > -  // Singal to recycle the each rxdata here, not at the end of process.
> > > -  //
> > > -  gBS->SignalEvent (Private->IpChoice ==
> > PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
> > >RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private
> > >- RxToken.Packet.RxData)->RecycleSignal);
> > >  }
> > >
> > >  /**
> > >
> >
> >
> > 


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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Michael D Kinney]

ARM and AARCH64 have a compiler intrinsic lib that is linked against all 
modules.

[LibraryClasses.ARM, LibraryClasses.AARCH64]
  #
  # It is not possible to prevent ARM compiler calls to generic intrinsic 
functions.
  # This library provides the instrinsic functions generated by a given 
compiler.
  # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
  #
  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

Can we use this same technique for IA32/X64 VS builds?

Mike


> -Original Message-
> From: devel@edk2.groups.io  On
> Behalf Of Laszlo Ersek
> Sent: Tuesday, March 31, 2020 5:42 AM
> To: Ard Biesheuvel ; edk2-
> devel-groups-io ;
> mac...@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> Add FltUsedLib for float.
> 
> On 03/30/20 23:41, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 23:29, Matthew Carlson via
> Groups.Io
> >  wrote:
> >>
> >> So it's not required by OpenSSL, it's required by the
> compiler whenever floating point is used, which can be in
> multiple places. For example, this is used in mu_plus
> (the Microsoft UEFI value add to EDK2) by the
> OnScreenKeyboard driver as well as the UiToolKit driver.
> 
> OK, so this is the part that was not obvious to me. This
> is basically
> code that exists on top of edk2 (consumes edk2) *AND*
> uses floating
> point *internally*.
> 
> > If you happen to use BaseCryptLib or the intrinsic in a
> driver that happens to need crypto as well, it can break
> due to multiple.
> >>
> >> I do agree, this only applies to MSVC and requiring
> every platform to add a line in their DSC would be a
> situation I would prefer to avoid if possible. Is there a
> way to specify a library dependency only if a toolchain
> is being used?
> >
> > Yes, so this either belongs in one of the
> IntrinsicsLibs we have, or
> > in the various Entrypoint libraries we have for PEIMs,
> DXEs, etc.
> >
> > However, given that we are talking about static
> libraries here, adding
> > a source file that *only* defines __fltused (and
> nothing else) to any
> > library should also work, as the resulting object file
> will only be
> > incorporated by the linker if it is needed to satisfy a
> symbol
> > dependency, and so it can never cause a conflict if it
> is the only
> > symbol in the object.
> 
> IMO: if edk2 intends to support out-of-tree modules that
> themselves
> utilize floating point, *with or without* a dependency on
> OpensslLib,
> then we should add Ard's suggestion:
> 
>  [Sources]
> +  FloatUsed.c | MSFT
> 
> to some of the "most core" edk2 library instances, i.e.
> those that *all*
> modules of the given module type inevitably depend on.
> The entry point
> libs look like a great idea to me.
> 
> That should link the _fltused external definition exactly
> once into all
> modules built with MSVC, regardless of whether each such
> module uses
> floating point or not.
> 
> Thanks
> Laszlo
> 
> 
> 


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

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

Re: [edk2-devel] API breakages and their implications. Was: [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

[Laszlo Ersek]

On 03/31/20 11:22, Leif Lindholm wrote:
> On Tue, Mar 31, 2020 at 01:53:21 +, Ni, Ray wrote:
>> Leif,
>> Please understand that the concern of this change is all the platforms that 
>> uses
>> this serial port lib must be changed otherwise build breaks.
> 
> Yes. This is the nature of collaborative development.
> This is something we on the ARM side have lived with since we got
> involved with EDK2, being the less-deployed user of the codebase, and
> that is fine.
> 
> TianoCore specifically produced the UDK to make life easier for those
> who are unable to track upstream, and we have followed that up with
> stable tags. I would highly recommend that any team that feels that
> this change would be too much effort to move to edk2-stable202002. Of
> course, they would then need to manually cherry-pick any improvements
> and security fixes from master. That is their choice.
> 
> Nevertheless, breaking existing platforms is a side effect, not a
> goal. So if an alternative solution can be found (which is not a hack
> that is going to affect the codebase negatively over time and simply
> need to be fixed properly at a later date), then clearly that is
> preferable.
> 
> "This breaks many platforms" is a good argument for seeing if a
> solution can be found that does not break (as) many platforms. It is
> not an argument for duplicating drivers when the change needed for
> those platforms is trivial.
> 
> These days, Linux tends to deal with API breakages (and other things)
> using a semantic patch tool called Coccinelle[1]. It would not be
> unreasonable, and indeed it would be helpful to us on the non-x86 side
> if something similar was adopted (and semantic patches mandated) for
> API breakages in EDK2.
> 
> [1] http://coccinelle.lip6.fr/sp.php

Two comments:

(1) One of the reasons why I would like to keep all platforms in a
single tree is to deal with API changes like this. That way, someone
proposing an API change would at least have the chance to fix up all the
consumer sites. Of course it would require diligent review from the
other pkg maintainers, but it could be implemented without any temprary
breakage in the git history even.


(2) Specifically about this problem. The vendor GUID approach is not a
bad one. How about the following alternative:

(2.1) Patch#1:

in the "MdeModulePkg/Library/BaseSerialPortLib16550" directory,
introduce a header file called "GetClock.h". This header file should
declare:

UINT32
GetClock (
  VOID
  );

Note: EFIAPI not needed.

The header file should be added to the existent
"BaseSerialPortLib16550.inf" file, in the [Sources] section.

Furthermore, in the same patch, introduce a new source file called
"GetClockPcd.c". (Add this file to the INF file as well.) The source
file should do:

#include 
#include "GetClock.h"

UINT32
GetClock (
  VOID
  )
{
  return PcdGet32 (PcdSerialClockRate);
}

Finally, in the same patch#1, modify "BaseSerialPortLib16550.c" to
#include "GetClock.h", and to call GetClock() in place of the current
PcdGet32 (PcdSerialClockRate) calls.

This patch basically splits the PcdGet32 (PcdSerialClockRate) call to a
separate translation unit, hiding it behind the more abstract GetClock()
API.


(2.2) Patch#2:

In the *same* "MdeModulePkg/Library/BaseSerialPortLib16550" directory,
introduce three new files:

- BaseSerialPortLibDynamicFrequency16550.inf
- BaseSerialPortLibDynamicFrequency16550.uni
- GetClockDynamicFrequency.c

I think the contents of these files must be fairly obvious at this
point, but anyway:

(2.2.1) INF file:

- modified copy of the INF file from (2.1)

- Update file-top comment, copyright notice, BASE_NAME, MODULE_UNI_FILE,
FILE_GUID.

- [LibraryClasses]: add dependency on SerialUartClockLib

- [Sources]: replace "GetClockPcd.c" with "GetClockDynamicFrequency.c"

- [Pcd]: drop PcdSerialClockRate

(2.2.2) UNI file: copy and modify the existent UNI file as needed.

(2.2.3) GetClockDynamicFrequency.c:

#include 
#include "GetClock.h"

UINT32
GetClock (
  VOID
  )
{
  return BaseSerialPortGetClock ();
}


This way, platforms currently consuming
"MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf"
will see no change whatsoever.

Platforms needing the dynamic frequency can resolve SerialPortLib to
"MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLibDynamicFrequency16550.inf",
*and* also resolve SerialUartClockLib to an instance that matches their
needs.

All implementation except the GetClock() one will be shared between the
two lib instances "BaseSerialPortLib16550" and
"BaseSerialPortLibDynamicFrequency16550".


This is a frequent pattern in edk2 -- refer to the various module
directories that contain two or more INF files. For example:

$ git ls-files -- \
MdePkg/*inf \
MdeModulePkg/*inf \
NetworkPkg/*inf \
PcAtChipsetPkg/*inf \
SecurityPkg/*inf \
ShellPkg/*inf \
UefiCpuPkg/*inf \
| rev \
| cut -f 2- -d / \
| rev \
| sort \
| uniq -d

This will produce a list of directories 

Re: [edk2-devel] API breakages and their implications. Was: [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

[Leif Lindholm]

Hi Ray,

I think it's good to start doing it voluntarily, or for changes
expected to affect many platforms. Over time, as people become more
familiar with the tool, it would make sense to make it first
recommended and then mandatory.

For Linux, the coccinelle diff is frequently included in the commit
message, or (especially when used for changing deprecated interfaces
or poor coding practice) they are copmmitted to the repository (under
scripts/coccinelle).

Regards,

Leif

On Tue, Mar 31, 2020 at 12:11:03 +, Ni, Ray wrote:
> Leif,
> Thanks for introducing such an interesting tool.
> I see this too is very useful for code refactoring.
> It's a game changing tool
> 
> To help me understand, do you suggest MAYBE when incompatible changes like 
> this
> happen, the change owners propose the semantic patches for all platforms?
> 
> 
> Thanks,
> Ray
> 
> 
> > -Original Message-
> > From: Leif Lindholm 
> > Sent: Tuesday, March 31, 2020 5:22 PM
> > To: Ni, Ray 
> > Cc: Ard Biesheuvel ; Jiang, Guomin 
> > ; devel@edk2.groups.io; Andrew
> > Fish ; Laszlo Ersek ; Kinney, Michael D 
> > 
> > Subject: Re: [edk2-devel] API breakages and their implications. Was: [PATCH 
> > 1/1] MdeModulePkg: UART Dynamic clock freq
> > Support
> > 
> > On Tue, Mar 31, 2020 at 01:53:21 +, Ni, Ray wrote:
> > > Leif,
> > > Please understand that the concern of this change is all the platforms 
> > > that uses
> > > this serial port lib must be changed otherwise build breaks.
> > 
> > Yes. This is the nature of collaborative development.
> > This is something we on the ARM side have lived with since we got
> > involved with EDK2, being the less-deployed user of the codebase, and
> > that is fine.
> > 
> > TianoCore specifically produced the UDK to make life easier for those
> > who are unable to track upstream, and we have followed that up with
> > stable tags. I would highly recommend that any team that feels that
> > this change would be too much effort to move to edk2-stable202002. Of
> > course, they would then need to manually cherry-pick any improvements
> > and security fixes from master. That is their choice.
> > 
> > Nevertheless, breaking existing platforms is a side effect, not a
> > goal. So if an alternative solution can be found (which is not a hack
> > that is going to affect the codebase negatively over time and simply
> > need to be fixed properly at a later date), then clearly that is
> > preferable.
> > 
> > "This breaks many platforms" is a good argument for seeing if a
> > solution can be found that does not break (as) many platforms. It is
> > not an argument for duplicating drivers when the change needed for
> > those platforms is trivial.
> > 
> > These days, Linux tends to deal with API breakages (and other things)
> > using a semantic patch tool called Coccinelle[1]. It would not be
> > unreasonable, and indeed it would be helpful to us on the non-x86 side
> > if something similar was adopted (and semantic patches mandated) for
> > API breakages in EDK2.
> > 
> > [1] http://coccinelle.lip6.fr/sp.php
> > 
> > Regards,
> > 
> > Leif
> > 
> > > Ard,
> > > Using Guided HOB sounds a good idea to me: )
> > > The benefits of using HOB is:
> > >   Length field in the HOB header can be used for extension if more 
> > > parameters are needed.
> > >   DXE can have the HOB access as well.
> > >
> > > EFI_SEC_HOB_DATA_PPI can be used to return the new Guided HOB from SEC 
> > > phase if needed.
> > >
> > > Thanks,
> > > Ray
> > >
> > >
> > > > -Original Message-
> > > > From: Ard Biesheuvel 
> > > > Sent: Monday, March 30, 2020 3:45 PM
> > > > To: Leif Lindholm 
> > > > Cc: Jiang, Guomin ; devel@edk2.groups.io; 
> > > > pankaj.ban...@nxp.com; Ni, Ray
> > ;
> > > > Wang, Jian J ; Wu, Hao A ; 
> > > > Ma, Maurice ; Dong,
> > > > Guo ; You, Benjamin ; 
> > > > Meenakshi Aggarwal
> > > > ; Varun Sethi ; Samer 
> > > > El-Haj-Mahmoud  > > > mahm...@arm.com>
> > > > Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: UART Dynamic clock 
> > > > freq Support
> > > >
> > > > On Mon, 30 Mar 2020 at 09:35, Leif Lindholm  wrote:
> > > > >
> > > > > Hi Jiang,
> > > > >
> > > > > It is not a question of effort of copying a driver, it is a question
> > > > > that copying drivers is something that should be avoided wherever
> > > > > practically possible. I did not think this topic was still under
> > > > > debate.
> > > > >
> > > > > If the existing 16550 SerialPortLib is overspecialised to the point
> > > > > where it only works on a subset of 16550 implementations, then it
> > > > > should change. There are going to be more non-PC systems turning up
> > > > > with 16550 UARTs - should they each copy/modify their drivers?
> > > > >
> > > > > If there are better ways of solving that problem, please suggest.
> > > > > But more duplicated drivers is not the answer.
> > > > >
> > > >
> > > > Could we use a GUIDed HOB? If it exists, we use its contents, and if
> > > > it doesn't, we use the default set by the 

Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow.

[Siyuan, Fu]

Reviewed-by: Siyuan Fu 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo
> Ersek
> Sent: 2020年3月25日 19:34
> To: Ni, Ray ; Gao, Zhichao 
> Cc: devel@edk2.groups.io; maciej.rab...@linux.intel.com
> Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive
> flow.
> 
> Ray, Zhichao,
> 
> On 02/27/20 12:02, Maciej Rabeda wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032
> >
> > 'ping' command's receive flow utilizes a single Rx token which it
> > attempts to reuse before recycling the previously received packet.
> > This causes a situation where under ICMP traffic,
> > Ping6OnEchoReplyReceived() function will receive an already
> > recycled packet with EFI_SUCCESS token status and finally
> > dereference invalid pointers from RxData structure.
> >
> > Cc: Ray Ni 
> > Cc: Zhichao Gao 
> > Signed-off-by: Maciej Rabeda 
> > ---
> >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> can you please review this ShellPkg patch? It's been on the list for
> almost a month now.
> 
> Thanks
> Laszlo
> 
> > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > index 23567fa2c1bb..a3fa32515192 100644
> > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > @@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived (
> >
> >  ON_EXIT:
> >
> > +  //
> > +  // Recycle the packet before reusing RxToken
> > +  //
> > +  gBS->SignalEvent (Private->IpChoice ==
> PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
> >RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private-
> >RxToken.Packet.RxData)->RecycleSignal);
> > +
> >if (Private->RxCount < Private->SendNum) {
> >  //
> >  // Continue to receive icmp echo reply packets.
> > @@ -632,10 +637,6 @@ ON_EXIT:
> >  //
> >  Private->Status = EFI_SUCCESS;
> >}
> > -  //
> > -  // Singal to recycle the each rxdata here, not at the end of process.
> > -  //
> > -  gBS->SignalEvent (Private->IpChoice ==
> PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
> >RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private-
> >RxToken.Packet.RxData)->RecycleSignal);
> >  }
> >
> >  /**
> >
> 
> 
> 


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

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

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Maintainers: switch to my @arm.com email address

[Ard Biesheuvel]

On 3/31/20 2:33 PM, Leif Lindholm wrote:

On Tue, Mar 31, 2020 at 13:12:28 +0200, Ard Biesheuvel wrote:

I no longer work for Linaro so switch to my ARM email address.

Signed-off-by: Ard Biesheuvel 


Reviewed-by: Leif Lindholm 
(Not that I can be trusted on the spelling...)



Thanks

Pushed as 864987702e01433f6c46c9d974a2233f436b8394


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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Laszlo Ersek]

On 03/30/20 23:41, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 23:29, Matthew Carlson via Groups.Io
>  wrote:
>>
>> So it's not required by OpenSSL, it's required by the compiler whenever 
>> floating point is used, which can be in multiple places. For example, this 
>> is used in mu_plus (the Microsoft UEFI value add to EDK2) by the 
>> OnScreenKeyboard driver as well as the UiToolKit driver.

OK, so this is the part that was not obvious to me. This is basically
code that exists on top of edk2 (consumes edk2) *AND* uses floating
point *internally*.

> If you happen to use BaseCryptLib or the intrinsic in a driver that happens 
> to need crypto as well, it can break due to multiple.
>>
>> I do agree, this only applies to MSVC and requiring every platform to add a 
>> line in their DSC would be a situation I would prefer to avoid if possible. 
>> Is there a way to specify a library dependency only if a toolchain is being 
>> used?
> 
> Yes, so this either belongs in one of the IntrinsicsLibs we have, or
> in the various Entrypoint libraries we have for PEIMs, DXEs, etc.
> 
> However, given that we are talking about static libraries here, adding
> a source file that *only* defines __fltused (and nothing else) to any
> library should also work, as the resulting object file will only be
> incorporated by the linker if it is needed to satisfy a symbol
> dependency, and so it can never cause a conflict if it is the only
> symbol in the object.

IMO: if edk2 intends to support out-of-tree modules that themselves
utilize floating point, *with or without* a dependency on OpensslLib,
then we should add Ard's suggestion:

 [Sources]
+  FloatUsed.c | MSFT

to some of the "most core" edk2 library instances, i.e. those that *all*
modules of the given module type inevitably depend on. The entry point
libs look like a great idea to me.

That should link the _fltused external definition exactly once into all
modules built with MSVC, regardless of whether each such module uses
floating point or not.

Thanks
Laszlo


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

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

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Maintainers: switch to my @arm.com email address

[Leif Lindholm]

On Tue, Mar 31, 2020 at 13:12:28 +0200, Ard Biesheuvel wrote:
> I no longer work for Linaro so switch to my ARM email address.
> 
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 
(Not that I can be trusted on the spelling...)

> ---
>  Maintainers.txt | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 475f7d530b85..0e50b6fdf36a 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -80,7 +80,7 @@ EDK II Platforms Packages:
>  
>  96Boards
>  F: Platform/96Boards/
> -M: Ard Biesheuvel 
> +M: Ard Biesheuvel 
>  M: Leif Lindholm 
>  
>  AMD Seattle
> @@ -88,24 +88,24 @@ F: Platform/AMD/OverdriveBoard/
>  F: Platform/LeMaker/CelloBoard/
>  F: Platform/SoftIron/
>  F: Silicon/AMD/Styx/
> -M: Ard Biesheuvel 
> +M: Ard Biesheuvel 
>  M: Leif Lindholm 
>  
>  ARM
>  F: Platform/ARM/
> -R: Ard Biesheuvel 
> +R: Ard Biesheuvel 
>  R: Thomas Abraham 
>  M: Leif Lindholm 
>  
>  BeagleBoard:
>  F: Platform/BeagleBoard/
>  F: Silicon/TexasInstruments/
> -R: Ard Biesheuvel 
> +R: Ard Biesheuvel 
>  M: Leif Lindholm 
>  
>  Comcast
>  F: Platform/Comcast/
> -M: Ard Biesheuvel 
> +M: Ard Biesheuvel 
>  M: Leif Lindholm 
>  
>  OptionRomPkg
> @@ -116,14 +116,14 @@ M: Ray Ni 
>  DisplayLink
>  F: Drivers/DisplayLink/
>  M: Leif Lindholm 
> -M: Ard Biesheuvel 
> +M: Ard Biesheuvel 
>  R: Andy Hayes 
>  
>  HiSilicon
>  F: Platform/Hisilicon/
>  F: Silicon/Hisilicon/
>  M: Leif Lindholm 
> -R: Ard Biesheuvel 
> +R: Ard Biesheuvel 
>  
>  Features/Intel
>  F: Features/Intel/
> @@ -241,7 +241,7 @@ Miscellaneous drivers
>  F: Silicon/Atmel/
>  F: Silicon/Openmoko/
>  F: Silicon/Synopsys/DesignWare/
> -R: Ard Biesheuvel 
> +R: Ard Biesheuvel 
>  M: Leif Lindholm 
>  
>  NXP platforms and silicon
> @@ -253,7 +253,7 @@ R: Meenakshi Aggarwal 
>  Raspberry Pi platforms and silicon
>  F: Platform/RaspberryPi/
>  F: Silicon/Broadcom/
> -M: Ard Biesheuvel 
> +M: Ard Biesheuvel 
>  M: Leif Lindholm 
>  R: Pete Batard 
>  
> @@ -261,5 +261,5 @@ Socionext platforms and silicon
>  F: Platform/Socionext/
>  F: Silicon/NXP/Library/Pcf8563RealTimeClockLib/
>  F: Silicon/Socionext/
> -M: Ard Biesheuvel 
> +M: Ard Biesheuvel 
>  M: Leif Lindholm 
> -- 
> 2.17.1
> 

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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Laszlo Ersek]

On 03/31/20 09:13, Ard Biesheuvel wrote:
> On Tue, 31 Mar 2020 at 03:40, Jiang, Guomin  wrote:
>>
>> Hi Laszlo,
>>
>> Thanks for you spending time review the changes.
>>
>> And I just want to present how to reproduce the build error.
>>
>> When build OvmfPkgX64, you can encounter this issue with your local change. 
>> The error as below:
>> TcgPei.lib(AutoGen.obj) : error LNK2001: unresolved external symbol _fltused
>> c:\sourcecodes\tiano\Build\OvmfX64\DEBUG_VS2019\X64\SecurityPkg\Tcg\TcgPei\TcgPei\DEBUG\TcgPei.dll
>>  : fatal error LNK1120: 1 unresolved externals
>>
>> The build command: build -p OvmfPkg\OvmfPkgX64.dsc -b DEBUG -t VS2019 -a X64 
>> -D TPM_ENABLE
>> The code changes for reproducing this symptom:
>> -  int  GLOBAL_USED _fltused = 1;
>> + //int  GLOBAL_USED _fltused = 1;
>> The machine: WIN10
>> The branch: edk2 master
>>
> 
> Doesn't the build error go away as well if you simply add FloatUsed.c
> to all BaseCryptLib flavours if Visual Studio is being used?
> 
> Something like
> 
> diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> index 1bbe4f435aec..f40bf18e7f5d 100644
> --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -27,6 +27,7 @@ [Defines]
>  #
> 
>  [Sources]
> +  FloatUsed.c | MSFT
>InternalCryptLib.h
>Hash/CryptMd4.c
>Hash/CryptMd5.c
> 

This is *exactly* what I've had in mind!

(With the additional (optional) tweak that IMO "FloatUsed.c" belongs
with the openssl INF files, as those are where the floating point usage
originates from. The cryptlib instances themselves have no floating
point code, AIUI.)

Thanks
Laszlo


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

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

Re: [edk2-devel] [PATCH v2 05/28] Silicon/Maxim: Add comments in Ds1307RtcLib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:20 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Add comments to explain the register read and write operation
> on Ds1307. These comments have been referred from data sheet:
> 
> https://datasheets.maximintegrated.com/en/ds/DS1307.pdf
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c 
> b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
> index fd7a8696e405..444e01124811 100644
> --- a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
> +++ b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
> @@ -28,6 +28,11 @@ STATIC EFI_I2C_MASTER_PROTOCOL*mI2cMaster;
>  
>  /**
>Read RTC register.
> +  Data Read-Slave Transmitter Mode
> +
> +   
>  
> +
> +  The first byte is received and handled as in the slave receiver mode.
>  
>@param  RtcRegAddr   Register offset of RTC to be read.
>  
> @@ -69,6 +74,9 @@ RtcRead (
>  
>  /**
>Write RTC register.
> +  Data Write-Slave Receiver Mode
> +
> +  
>  
>@param  RtcRegAddr   Register offset of RTC to write.
>@param  Val  Value to be written
> -- 
> 2.17.1
> 

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

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

Re: [edk2-devel] [PATCH v2 04/28] Silicon/Maxim: Fix bug in RtcWrite in Ds1307RtcLib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:19 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> There was a bug in I2C DXE implementation, which caused the Ds1307 RTC
> device to issue two operation for register write, while this is a single
> operation task. refer page 12 (Slave Receiver Mode (Write Mode)) on
> 
> https://datasheets.maximintegrated.com/en/ds/DS1307.pdf
>
> Modify ds1307 RtcWrite code accordingly.
> 
> Signed-off-by: Pankaj Bansal 

So, I'm OK with this patch, but I'll mention that I prefer the design
in Silicon/NXP/Library/Pcf8563RealTimeClockLib which I think could
also be applied here. I think that might have avoided the confusion
that caused the bug.

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c 
> b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
> index 88dc198ffec8..fd7a8696e405 100644
> --- a/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
> +++ b/Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c
> @@ -5,7 +5,7 @@
>EmbeddedPkg/Library/TemplateRealTimeClockLib/RealTimeClockLib.c
>  
>Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> -  Copyright 2017 NXP
> +  Copyright 2017, 2020 NXP
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -84,16 +84,15 @@ RtcWrite (
>  {
>RTC_I2C_REQUEST  Req;
>EFI_STATUS   Status;
> +  UINT8Buffer[2];
>  
> -  Req.OperationCount = 2;
> +  Req.OperationCount = 1;
> +  Buffer[0] = RtcRegAddr;
> +  Buffer[1] = Val;
>  
>Req.SetAddressOp.Flags = 0;
> -  Req.SetAddressOp.LengthInBytes = sizeof (RtcRegAddr);
> -  Req.SetAddressOp.Buffer = 
> -
> -  Req.GetSetDateTimeOp.Flags = 0;
> -  Req.GetSetDateTimeOp.LengthInBytes = sizeof (Val);
> -  Req.GetSetDateTimeOp.Buffer = 
> +  Req.SetAddressOp.LengthInBytes = sizeof (Buffer);
> +  Req.SetAddressOp.Buffer = Buffer;
>  
>Status = mI2cMaster->StartRequest (mI2cMaster, FixedPcdGet8 
> (PcdI2cSlaveAddress),
>   (VOID *),
> -- 
> 2.17.1
> 

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

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

Re: [edk2-devel] [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

[Laszlo Ersek]

On 03/30/20 23:29, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> I think using the QEMU feature to map a path from the host as 
> a FAT formatted driver has value to provision QEMU with a set
> of target tests to run.  I agree there is no persistent storage
> of writes to this type of drive.  I believe writes are supported
> if file state is needed during the single boot of QEMU.
> 
> In order to get test results out of QEMU back to the host,
> I was considering the use of consoles mapped as named pipes
> and send all test results out the console device and the
> host can parse the logs.

Yes, this should be viable.

> I have not had good experiences trying to build virtual
> disks, provision them, map them into QEMU, and remount
> from host to collect test results.  Slow operations and
> cumbersome to mount/unmount from the host (at least from
> Windows environments).

I must agree. It is slow and somewhat cumbersome with "guestfish" too,
on Linux.

Guestfish, on the plus side, does not mount the disk with the host
kernel, and so it doesn't need root rights. Guestfish launches a
separate QEMU instance on top of the virtual disk, booting a dedicated
kernel and agent (the "libguestfs appliance") in the guest. The guest
appliance communicates with the "guestfish" host side command shell over
virtio-serial.

So the host kernel never trusts (or even sees) the filesystem structures
of the guest disk image; files are exchanged between the "libguestfs
appliance" (= guest kernel + agent) and the host-side command shell
using a dedicated wire protocol.

But, I agree it is somewhat cumbersome and slow, and I have no idea
whether it works on Windows hosts.

Thanks,
Laszlo


> 
> Mike
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On
>> Behalf Of Laszlo Ersek
>> Sent: Monday, March 30, 2020 2:11 PM
>> To: devel@edk2.groups.io; ard.biesheu...@linaro.org; Sean
>> Brogan 
>> Subject: Re: [edk2-devel] [PATCH] .azurepipelines: Enable
>> CI for OvmfPkg and EmulatorPkg
>>
>> On 03/30/20 08:07, Ard Biesheuvel wrote:
>>> On Mon, 30 Mar 2020 at 01:16, Sean via Groups.Io
>>>  wrote:

 Ard/Laszlo or anyone familiar with QEMU.

 I read up on the ovmf readme and the qemu wiki but
>> still have a few issues i am hoping for quick/easy
>> answers.

 1. How do i programmatically exit the emulator.
>> Seems like uefi shell > reset just reboots.  Other ideas?
>>>
>>> 'reset' is supposed to do that. Use 'reset -s' to kill
>> the VM
>>>
 2. Is there an easy way to map a local file system so
>> that i can setup a startup.nsh?

>>>
>>> As Andrew points out, use '-hda fat:' and it will
>> be exposed to
>>> the VM as a FAT formatted block device.
>>
>> Note that it will not offer (functional, or any) write
>> support.
>>
>> I prefer "mtools" or "guestfish" for formatting and
>> populating a virtual
>> disk image.
>>
>> None of those are easy ways, admittedly -- I'm not
>> offering some
>> examples right now exactly because I have to sit down and
>> think about
>> them every time I need them. :) If there's interest in
>> such commands, I
>> could hack up something, but then please give me some
>> specs, and I'll
>> have to work on these things in the morning (while my
>> brain is still not
>> mush).
>>
>> Thanks
>> Laszlo
>>
>>
>> 
> 


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

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

Re: [edk2-devel] [PATCH v2] NetworkPkg/Ip6Dxe: Improve Neightbor Discovery message validation.

[Maciej Rabeda]

Hi Laszlo,

Thanks for trying this out!

The condition in the ASSERTs is reversed, consequently for the ASSERTs 
added in this function.
I have added them to fire up when Ip6IsNDOptionValid() fails to properly 
react to invalid packet (return with an error and do not proceed with 
processing options).
In this case, fire assert when current position within the packet 
(Offset) moved past the size of the option (Option.Length) * 8 (it's in 
8-byte units) is outside the packet (further than first byte outside the 
packet).
Offset one byte past the packet == last option ended at the end of the 
packet (packet size is Head->PayloadLength).


Can you try swapping the >= to <= and rerun your test?
This should apply to lines 2114, 2167 and 2337.

I will submit a patch for that.

Thanks,
Maciej

On 31-Mar-20 02:35, Laszlo Ersek wrote:

Hi Maciej,

On 03/30/20 14:23, Maciej Rabeda wrote:

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

Problem has been identified with Ip6ProcessRouterAdvertise() when
Router Advertise packet contains options with malicious/invalid
'Length' field. This can lead to platform entering infinite loop
when processing options from that packet.

Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Signed-off-by: Maciej Rabeda 
Reviewed-by: Siyuan Fu 
---
  NetworkPkg/Ip6Dxe/Ip6Nd.c | 44 +--
  NetworkPkg/Ip6Dxe/Ip6Nd.h | 13 +
  NetworkPkg/Ip6Dxe/Ip6Option.c | 57 +++-
  3 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.c b/NetworkPkg/Ip6Dxe/Ip6Nd.c
index 4288ef02dd46..fd7f60b2f92c 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Nd.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c
@@ -1927,7 +1927,7 @@ Ip6ProcessRouterAdvertise (
UINT32ReachableTime;
UINT32RetransTimer;
UINT16RouterLifetime;
-  UINT16Offset;
+  UINT32Offset;
UINT8 Type;
UINT8 Length;
IP6_ETHER_ADDR_OPTION LinkLayerOption;
@@ -2094,10 +2094,11 @@ Ip6ProcessRouterAdvertise (
//
// The only defined options that may appear are the Source
// Link-Layer Address, Prefix information and MTU options.
-  // All included options have a length that is greater than zero.
+  // All included options have a length that is greater than zero and
+  // fit within the input packet.
//
Offset = 16;
-  while (Offset < Head->PayloadLength) {
+  while (Offset < (UINT32) Head->PayloadLength) {
  NetbufCopy (Packet, Offset, sizeof (UINT8), );
  switch (Type) {
  case Ip6OptionEtherSource:
@@ -2105,9 +2106,12 @@ Ip6ProcessRouterAdvertise (
// Update the neighbor cache
//
NetbufCopy (Packet, Offset, sizeof (IP6_ETHER_ADDR_OPTION), (UINT8 *) 
);
-  if (LinkLayerOption.Length <= 0) {
-goto Exit;
-  }
+
+  //
+  // Option size validity ensured by Ip6IsNDOptionValid().
+  //
+  ASSERT (LinkLayerOption.Length != 0);
+  ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 >= (UINT32) 
Head->PayloadLength);
  
ZeroMem (, sizeof (EFI_MAC_ADDRESS));

CopyMem (, LinkLayerOption.EtherAddr, 6);
@@ -2151,13 +2155,17 @@ Ip6ProcessRouterAdvertise (
  }
}
  
-  Offset = (UINT16) (Offset + (UINT16) LinkLayerOption.Length * 8);

+  Offset += (UINT32) LinkLayerOption.Length * 8;
break;
  case Ip6OptionPrefixInfo:
NetbufCopy (Packet, Offset, sizeof (IP6_PREFIX_INFO_OPTION), (UINT8 *) 
);
-  if (PrefixOption.Length != 4) {
-goto Exit;
-  }
+
+  //
+  // Option size validity ensured by Ip6IsNDOptionValid().
+  //
+  ASSERT (PrefixOption.Length == 4);
+  ASSERT (Offset + (UINT32) PrefixOption.Length * 8 >= (UINT32) 
Head->PayloadLength);

I've hit this ASSERT():

   ASSERT NetworkPkg/Ip6Dxe/Ip6Nd.c(2167): Offset + (UINT32) PrefixOption.Length * 8 
>= (UINT32) Head->PayloadLength

Here's the packet that triggers it:

02:30:25.350096 52:54:00:18:64:e7 > 33:33:00:00:00:01, ethertype IPv6 (0x86dd), 
length 142: (class 0xc0, hlim 255, next-header ICMPv6 (58) payload length: 88) 
fe80::5054:ff:fe18:64e7 > ff02::1: [icmp6 sum ok] ICMP6, router advertisement, 
length 88
 hop limit 64, Flags [none], pref medium, router lifetime 1800s, 
reachable time 0ms, retrans time 0ms
   prefix info option (3), length 32 (4): fd33:eb1b:9b36::/64, Flags 
[onlink, auto], valid time 3600s, pref. time 3600s
   mtu option (5), length 8 (1):  1500
   source link-address option (1), length 8 (1): 52:54:00:18:64:e7
   rdnss option (25), length 24 (3):  lifetime 3600s, addr: 
fe80::5054:ff:fe18:64e7
 0x:  6c00  0058 3aff fe80     lX:.
 0x0010:  5054 00ff fe18 64e7 ff02     PTd.
 0x0020:     0001 8600 0008 4000 0708  @...
 0x0030:      0304 40c0  0e10  

Re: [edk2-devel] [PATCH v4 1/1] BaseTools: Enable file dependencies in .inf files

[PierreGondois]

This patch is following the v3 named "BaseTools: Build ASL files before C files 
", available at https://edk2.groups.io/g/devel/message/53735 
Sorry for messing with the names.

Regards,
Pierre

-Original Message-
From: PierreGondois  
Sent: Monday, March 30, 2020 5:09 PM
To: devel@edk2.groups.io
Cc: Pierre Gondois ; bob.c.f...@intel.com; 
liming@intel.com; Sami Mujawar ; nd 
Subject: [PATCH v4 1/1] BaseTools: Enable file dependencies in .inf files

From: Pierre Gondois 

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

The dependencies for C files are satisfied by the build system.
However, there are use cases where source files with different languages are 
inter-dependent. The EDKII build framework currently doesn't have options to 
specify such dependencies.
E.g. It may be necessary to specify the build order between
 ASL files and C files. The use case being that the AML
 blob generated by compiling the ASL files is loaded and
 parsed by some C code.

This patch allows to describe dependencies between files listed in the 
'[Sources]' section of '.inf' files. The list of source files to build prior 
starts with a colon (':'), e.g.:
[Sources]
  FileName1.X
  FileName2.Y : FileName1.X
  FileName3.Z : FileName1.X FileName2.Y

In the example above:
  * FileName1.X will be built prior to FileName2.Y.
  * FileName1.X and FileName2.Y will be built prior to
FileName3.Z.

This does not affect the file specific build options, which are pipe separated, 
e.g.:
  FileName2.Y | GCC : FileName1.X

The list of colon separated files must be part of the module, i.e. they must be 
present in the [Sources] section.

Their file extension must be described in the 
BaseTools/Conf/build_rule.template file, and generate an output file, i.e. have 
a non-empty '' section.

Declaring a dependency in the [Sources] sections leads to the creation of 
makefile rules between these files in the build. The makefile rule is between 
the generated files.
E.g.:
  FileName1.X generates FileName1.objx
  FileName2.Y generates FileName2.objy
  Then
FileName2.Y : FileName1.X
  will create the following makefile rule:
FileName2.objy : FileName1.objx

INF Specification updates:
s3.2.1, "Common Definitions":
   ::= ":"
 ::=   

s2.5, "[Sources] Section":
  To describe a build order dependency between source
  files, specify the source files to build prior by
  listing them, colon separated.
   ::=  [] []
   ::=  [FileNameDependency]*

Signed-off-by: Pierre Gondois 
---

The changes can be seen at 
https://github.com/PierreARM/edk2/commits/676_build_asl_first_v4

Notes:
v4:
 - Correct typo in commit message, requested from [Bob]
 - Create a Bugzilla to update the Inf specification,
   and submit a patch to update the Inf specification,
   available at:
   https://bugzilla.tianocore.org/show_bug.cgi?id=2646

 BaseTools/Source/Python/AutoGen/BuildEngine.py  |  5 
 BaseTools/Source/Python/AutoGen/GenMake.py  |  5 
 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py| 16 ++
 BaseTools/Source/Python/Common/DataType.py  |  3 +-
 BaseTools/Source/Python/Common/Misc.py  |  2 ++
 BaseTools/Source/Python/Workspace/InfBuildData.py   | 31 ++--
 BaseTools/Source/Python/Workspace/MetaFileParser.py | 17 +--
 7 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/BuildEngine.py 
b/BaseTools/Source/Python/AutoGen/BuildEngine.py
index 
d602414ca41f37155c9c6d00eec54ea3918840c3..59cc0f8a765835ee459880f66453063af5f5779d
 100644
--- a/BaseTools/Source/Python/AutoGen/BuildEngine.py
+++ b/BaseTools/Source/Python/AutoGen/BuildEngine.py
@@ -2,6 +2,7 @@
 # The engine for building files
 #
 # Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2020, ARM Limited. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent  #
 
@@ -53,6 +54,7 @@ class TargetDescBlock(object):
 self.Outputs = Outputs
 self.Commands = Commands
 self.Dependencies = Dependencies
+self.SourceFileDependencies = None
 if self.Outputs:
 self.Target = self.Outputs[0]
 else:
@@ -277,6 +279,9 @@ class FileBuildRule:
 TargetDesc.GenListFile = self.GenListFile
 TargetDesc.GenIncListFile = self.GenIncListFile
 self.BuildTargets[DstFile[0]] = TargetDesc
+
+TargetDesc.SourceFileDependencies = 
+ SourceFile.SourceFileDependencies
+
 return TargetDesc
 
 def _BuildCommand(self, Macros):
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 
bbb3c29446f53fa7f2cb61a216a5b119f72c3fbc..e257fd2c2eefee2782bc52606e537a2147f6857b
 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1013,6 +1013,11 @@ cleanlib:
 
 Deps = []
 

Re: [edk2-devel] [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

[Laszlo Ersek]

On 03/30/20 23:22, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 22:56, Laszlo Ersek  wrote:
>>
>> On 03/30/20 19:44, Ard Biesheuvel wrote:
>>> On Mon, 30 Mar 2020 at 19:11, Sean via Groups.Io
>>>  wrote:

 On Mon, Mar 30, 2020 at 10:04 AM, Ard Biesheuvel wrote:

 Is there any way I could contribute ArmVirtQemu to this? Or would it
 be easier if I provided comments/instructions?

 Either way.
 Any instructions you provide would be great.  I was going to hack 
 something up for feedback but happy for someone else to do it.   Let me 
 know.
>>>
>>> OK, so the typical invocation would be
>>>
>>> qemu-system-aarch64 -M virt -cpu cortex-a57 -m 1024 -net none
>>> -nographic -bios .../path/to/QEMU_EFI.fd -hda
>>> fat:rw:.../path/to/startup.nsh
>>>
>>> The only complication compared to OVMF is that there is no separate
>>> serial port for debug output vs console output, so everything is going
>>> to come out of the same pipe, and grep'ing the console output for
>>> meaningful strings may easily result in false positives. (-pflash
>>> could be used as well, but doesn't really add anything in this case,
>>> and QEMU for ARM has a quirk where pflash images must be exactly 64 MB
>>> in size)
>>
>> I'm begging you not to introduce instances of "-bios" anywhere near
>> edk2. Please? :) We need to educate QEMU users, and we need to keep our
>> sanity when facing bug reports. Let's not set bad examples anywhere, if
>> we can manage.
>>
>> I think truncate(1) can do what we need for padding, without actually
>> allocating those MBs.
>>
> 
> truncate should work on Linux, but I have no idea how to pad an image
> to a certain size on Windows, and I think the idea was to enable both?
> 
> In any case, I don't feel as strongly about this as Laszlo does: even
> though -bios really shouldn't be used when you are actually installing
> an OS into the VM, I don't think it is inappropriate for booting into
> a shell and nothing else. Perhaps we could annotate the scripts in a
> way that discourages people from adopting it?
> 
> Or if there is an easy way to make this work on both Windows and Linux
> using -pflash, I obviously wouldn't mind. I just don't feel it is
> essential.

OK, thank you.

Meta-comment: while I have strong opinions about these matters, I'm 100%
aware that I cannot put in basically any actual work. So I'm trying to
police myself in that I make suggestions politely and really only as
moderate suggestions, not as "demands" or knee-jerk reactions.
Unfortunately, I sometimes slip up, and my thoughts hit the list in less
than optimal form. I'm really sorry about that!

It's a fine line -- I don't want to ignore this topic (esp. when asked
for an opinion), but then again I shouldn't present unreasonable
"requirements", without the capacity to contribute code. Please bear
with me. :)

Thanks!
Laszlo


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

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

Re: [edk2-devel] API breakages and their implications. Was: [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

[Ni, Ray]

Leif,
Thanks for introducing such an interesting tool.
I see this too is very useful for code refactoring.
It's a game changing tool

To help me understand, do you suggest MAYBE when incompatible changes like this
happen, the change owners propose the semantic patches for all platforms?


Thanks,
Ray


> -Original Message-
> From: Leif Lindholm 
> Sent: Tuesday, March 31, 2020 5:22 PM
> To: Ni, Ray 
> Cc: Ard Biesheuvel ; Jiang, Guomin 
> ; devel@edk2.groups.io; Andrew
> Fish ; Laszlo Ersek ; Kinney, Michael D 
> 
> Subject: Re: [edk2-devel] API breakages and their implications. Was: [PATCH 
> 1/1] MdeModulePkg: UART Dynamic clock freq
> Support
> 
> On Tue, Mar 31, 2020 at 01:53:21 +, Ni, Ray wrote:
> > Leif,
> > Please understand that the concern of this change is all the platforms that 
> > uses
> > this serial port lib must be changed otherwise build breaks.
> 
> Yes. This is the nature of collaborative development.
> This is something we on the ARM side have lived with since we got
> involved with EDK2, being the less-deployed user of the codebase, and
> that is fine.
> 
> TianoCore specifically produced the UDK to make life easier for those
> who are unable to track upstream, and we have followed that up with
> stable tags. I would highly recommend that any team that feels that
> this change would be too much effort to move to edk2-stable202002. Of
> course, they would then need to manually cherry-pick any improvements
> and security fixes from master. That is their choice.
> 
> Nevertheless, breaking existing platforms is a side effect, not a
> goal. So if an alternative solution can be found (which is not a hack
> that is going to affect the codebase negatively over time and simply
> need to be fixed properly at a later date), then clearly that is
> preferable.
> 
> "This breaks many platforms" is a good argument for seeing if a
> solution can be found that does not break (as) many platforms. It is
> not an argument for duplicating drivers when the change needed for
> those platforms is trivial.
> 
> These days, Linux tends to deal with API breakages (and other things)
> using a semantic patch tool called Coccinelle[1]. It would not be
> unreasonable, and indeed it would be helpful to us on the non-x86 side
> if something similar was adopted (and semantic patches mandated) for
> API breakages in EDK2.
> 
> [1] http://coccinelle.lip6.fr/sp.php
> 
> Regards,
> 
> Leif
> 
> > Ard,
> > Using Guided HOB sounds a good idea to me: )
> > The benefits of using HOB is:
> >   Length field in the HOB header can be used for extension if more 
> > parameters are needed.
> >   DXE can have the HOB access as well.
> >
> > EFI_SEC_HOB_DATA_PPI can be used to return the new Guided HOB from SEC 
> > phase if needed.
> >
> > Thanks,
> > Ray
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel 
> > > Sent: Monday, March 30, 2020 3:45 PM
> > > To: Leif Lindholm 
> > > Cc: Jiang, Guomin ; devel@edk2.groups.io; 
> > > pankaj.ban...@nxp.com; Ni, Ray
> ;
> > > Wang, Jian J ; Wu, Hao A ; Ma, 
> > > Maurice ; Dong,
> > > Guo ; You, Benjamin ; 
> > > Meenakshi Aggarwal
> > > ; Varun Sethi ; Samer 
> > > El-Haj-Mahmoud  > > mahm...@arm.com>
> > > Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: UART Dynamic clock 
> > > freq Support
> > >
> > > On Mon, 30 Mar 2020 at 09:35, Leif Lindholm  wrote:
> > > >
> > > > Hi Jiang,
> > > >
> > > > It is not a question of effort of copying a driver, it is a question
> > > > that copying drivers is something that should be avoided wherever
> > > > practically possible. I did not think this topic was still under
> > > > debate.
> > > >
> > > > If the existing 16550 SerialPortLib is overspecialised to the point
> > > > where it only works on a subset of 16550 implementations, then it
> > > > should change. There are going to be more non-PC systems turning up
> > > > with 16550 UARTs - should they each copy/modify their drivers?
> > > >
> > > > If there are better ways of solving that problem, please suggest.
> > > > But more duplicated drivers is not the answer.
> > > >
> > >
> > > Could we use a GUIDed HOB? If it exists, we use its contents, and if
> > > it doesn't, we use the default set by the FixedPCD.

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

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

Re: [edk2-devel] [PATCH] NetworkPkg/UefiPxeBcDxe: handle competing DHCP servers (more) gracefully

[Siyuan, Fu]

Reviewed-by: Siyuan Fu 

> -Original Message-
> From: Laszlo Ersek 
> Sent: 2020年3月31日 8:48
> To: edk2-devel-groups-io 
> Cc: Wu, Jiaxin ; Maciej Rabeda
> ; Philippe Mathieu-Daudé
> ; Fu, Siyuan 
> Subject: [PATCH] NetworkPkg/UefiPxeBcDxe: handle competing DHCP servers
> (more) gracefully
> 
> When DHCP is misconfigured on a network segment, such that two DHCP
> servers attempt to reply to requests (and therefore race with each other),
> the edk2 PXE client can confuse itself.
> 
> In PxeBcDhcp4BootInfo() / PxeBcDhcp6BootInfo(), the client may refer to a
> DHCP reply packet as an "earlier" packet from the "same" DHCP server, when
> in reality both packets are unrelated, and arrive from different DHCP
> servers.
> 
> While the edk2 PXE client can do nothing to fix this, it should at least
> not ASSERT() -- ASSERT() is for catching programming errors (violations of
> invariants that are under the control of the programmer). ASSERT()s should
> in particular not refer to external data (such as network packets). What's
> more, in RELEASE builds, we get NULL pointer references.
> 
> Check the problem conditions with actual "if"s, and return
> EFI_PROTOCOL_ERROR. This will trickle out to PxeBcLoadBootFile(), and be
> reported as "PXE-E99: Unexpected network error".
> 
> Cc: Jiaxin Wu 
> Cc: Maciej Rabeda 
> Cc: Philippe Mathieu-Daudé 
> Cc: Siyuan Fu 
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> Repo:   https://pagure.io/lersek/edk2.git
> Branch: dhcp_assert
> 
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 30 ++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> index 10bbb06f7593..d062a526077b 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> @@ -482,7 +482,20 @@ PxeBcDhcp4BootInfo (
>  Cache4 = >DhcpAck.Dhcp4;
>}
> 
> -  ASSERT (Cache4->OptList[PXEBC_DHCP4_TAG_INDEX_BOOTFILE] != NULL);
> +  if (Cache4->OptList[PXEBC_DHCP4_TAG_INDEX_BOOTFILE] == NULL) {
> +//
> +// This should never happen in a correctly configured DHCP / PXE
> +// environment. One misconfiguration that can cause it is two DHCP
> servers
> +// mistakenly running on the same network segment at the same time,
> and
> +// racing each other in answering DHCP requests. Thus, the DHCP packets
> +// that the edk2 PXE client considers "belonging together" may actually
> be
> +// entirely independent, coming from two (competing) DHCP servers.
> +//
> +// Try to deal with this gracefully. Note that this check is not
> +// comprehensive, as we don't try to identify all such errors.
> +//
> +return EFI_PROTOCOL_ERROR;
> +  }
> 
>//
>// Parse the boot server address.
> @@ -612,7 +625,20 @@ PxeBcDhcp6BootInfo (
>  Cache6 = >DhcpAck.Dhcp6;
>}
> 
> -  ASSERT (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] != NULL);
> +  if (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] == NULL) {
> +//
> +// This should never happen in a correctly configured DHCP / PXE
> +// environment. One misconfiguration that can cause it is two DHCP
> servers
> +// mistakenly running on the same network segment at the same time,
> and
> +// racing each other in answering DHCP requests. Thus, the DHCP packets
> +// that the edk2 PXE client considers "belonging together" may actually
> be
> +// entirely independent, coming from two (competing) DHCP servers.
> +//
> +// Try to deal with this gracefully. Note that this check is not
> +// comprehensive, as we don't try to identify all such errors.
> +//
> +return EFI_PROTOCOL_ERROR;
> +  }
> 
>//
>// Set the station address to IP layer.
> --
> 2.19.1.3.g30247aa5d201


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

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

Re: [edk2-devel] [PATCH v2 03/28] Silicon/NXP/I2cDxe: Fix I2c Timeout with RTC

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:18 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> With latest edk2 codebase, sometimes i2c timeout is observed when
> Network devices are being probed.
> This is happening when gRT->GetTime request is ongoing.
> gRT->GetTime triggers a read request to Real Time Clock which is
> connected to I2c bus.
> In between read request, if an event occurs, which also triggers
> gRT->GetTime (i.e. RTC read), the I2c bus goes into unrecovered state.
> 
> This state is not even recovered, when rebooting the board.
> We need to power off the board completely to recover i2c bus.
> 
> To prevent this, TPL level of I2c read is being raised to high, so that
> no other event can preempt this. with this solution no timeout has been
> observed so far.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.c   | 12 
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf |  3 ++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c 
> b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
> index 848e707c1673..a5aba47b3ed4 100644
> --- a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
> +++ b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "I2cDxe.h"
>  
> @@ -88,6 +89,13 @@ StartRequest (
>NXP_I2C_MASTER   *I2c;
>UINTNI2cBase;
>EFI_STATUS   Status;
> +  EFI_TPL  Tpl;
> +  BOOLEAN  AtRuntime;
> +
> +  AtRuntime = EfiAtRuntime ();
> +  if (!AtRuntime) {
> +Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> +  }
>  
>I2c = NXP_I2C_FROM_THIS (This);
>  
> @@ -95,6 +103,10 @@ StartRequest (
>  
>Status = I2cBusXfer (I2cBase, SlaveAddress, RequestPacket);
>  
> +  if (!AtRuntime) {
> +gBS->RestoreTPL (Tpl);
> +  }
> +
>return Status;
>  }
>  
> diff --git a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf 
> b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> index 84adb837c249..867376044656 100644
> --- a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> +++ b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> @@ -13,7 +13,7 @@ [Defines]
>INF_VERSION= 0x0001001A
>BASE_NAME  = I2cDxe
>FILE_GUID  = 5f2927ba-1b04-4d5f-8bef-2b50c635d1e7
> -  MODULE_TYPE= DXE_DRIVER
> +  MODULE_TYPE= DXE_RUNTIME_DRIVER
>VERSION_STRING = 1.0
>ENTRY_POINT= I2cDxeEntryPoint
>UNLOAD = I2cDxeUnload
> @@ -36,6 +36,7 @@ [LibraryClasses]
>UefiBootServicesTableLib
>UefiDriverEntryPoint
>UefiLib
> +  UefiRuntimeLib
>  
>  [Guids]
>gNxpNonDiscoverableI2cMasterGuid
> -- 
> 2.17.1
> 

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

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

Re: [edk2-devel] [PATCH v2 2/2] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval"

[Siyuan, Fu]

Reviewed-by: Siyuan Fu 

> -Original Message-
> From: michael.kuba...@outlook.com 
> Sent: 2020年3月25日 11:00
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek ; Fu, Siyuan ;
> Maciej Rabeda ; Wu, Jiaxin
> 
> Subject: [PATCH v2 2/2] Revert "NetworkPkg/TlsAuthConfigDxe: fix
> TlsCaCertificate attributes retrieval"
> 
> From: Michael Kubacki 
> 
> This reverts commit 6896efdec2709e530b23c688cf0f31706709a0c5.
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> 
> GetVariable() now returns attributes when it fails with
> EFI_BUFFER_TOO_SMALL. Therefore, commit 6896efdec270 is
> reverted since it is no longer relevant.
> 
> Cc: Laszlo Ersek 
> Cc: Siyuan Fu 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Signed-off-by: Michael Kubacki 
> ---
>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 +---
>  1 file changed, 1 insertion(+), 26 deletions(-)
> 
> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> index 715bc3a0a941..2481d1098fa3 100644
> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> @@ -657,7 +657,6 @@ EnrollX509toVariable (
>EFI_SIGNATURE_LIST*CACert;
>EFI_SIGNATURE_DATA*CACertData;
>VOID  *Data;
> -  VOID  *CurrentData;
>UINTN DataSize;
>UINTN SigDataSize;
>UINT32Attr;
> @@ -669,7 +668,6 @@ EnrollX509toVariable (
>CACert= NULL;
>CACertData= NULL;
>Data  = NULL;
> -  CurrentData   = NULL;
>Attr  = 0;
> 
>Status = ReadFileContent (
> @@ -712,30 +710,11 @@ EnrollX509toVariable (
>Status = gRT->GetVariable(
>VariableName,
>,
> -  NULL,
> +  ,
>,
>NULL
>);
>if (Status == EFI_BUFFER_TOO_SMALL) {
> -//
> -// Per spec, we have to fetch the variable's contents, even though we're
> -// only interested in the variable's attributes.
> -//
> -CurrentData = AllocatePool (DataSize);
> -if (CurrentData == NULL) {
> -  Status = EFI_OUT_OF_RESOURCES;
> -  goto ON_EXIT;
> -}
> -Status = gRT->GetVariable(
> -VariableName,
> -,
> -,
> -,
> -CurrentData
> -);
> -if (EFI_ERROR (Status)) {
> -  goto ON_EXIT;
> -}
>  Attr |= EFI_VARIABLE_APPEND_WRITE;
>} else if (Status == EFI_NOT_FOUND) {
>  Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
> @@ -766,10 +745,6 @@ ON_EXIT:
>  FreePool (Data);
>}
> 
> -  if (CurrentData != NULL) {
> -FreePool (CurrentData);
> -  }
> -
>if (X509Data != NULL) {
>  FreePool (X509Data);
>}
> --
> 2.16.3.windows.1


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

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

Re: [edk2-devel] [PATCH v2 01/28] Silicon/NXP: Add I2c lib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:16 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> I2c lib is going to be used in PrePeiCore sec module to get the
> System clock information from devices connected to i2c (like fpga
> or clock generator)
> 
> since we don't have support of DXE modules this early in boot stage,
> move the i2c controller functionality in library.

This isn't moving the functionality to a library though - it is moving
the functionality to a library *and* adding new features. These are
two separate changes that should be two separate patches.

The content in this patch is mostly fine as the end result (but some
comments below).

I suggest this patch is reordered with 2/28 and all of the splitting
out part takes place in that patch. This patch can then be reduced to
... the bits that are currently impossible to see are changed (at
least the glitch fixing).

> Signed-off-by: Pankaj Bansal 
> ---
>  Platform/NXP/NxpQoriqLs.dsc.inc |   4 +-
>  Silicon/NXP/Include/Library/I2cLib.h| 120 
>  Silicon/NXP/Library/I2cLib/I2cLib.c | 576 
>  Silicon/NXP/Library/I2cLib/I2cLib.inf   |  31 ++
>  Silicon/NXP/Library/I2cLib/I2cLibInternal.h | 105 
>  Silicon/NXP/NxpQoriqLs.dec  |  10 +-
>  6 files changed, 844 insertions(+), 2 deletions(-)
>  create mode 100644 Silicon/NXP/Include/Library/I2cLib.h
>  create mode 100644 Silicon/NXP/Library/I2cLib/I2cLib.c
>  create mode 100644 Silicon/NXP/Library/I2cLib/I2cLib.inf
>  create mode 100644 Silicon/NXP/Library/I2cLib/I2cLibInternal.h
> 
> diff --git a/Platform/NXP/NxpQoriqLs.dsc.inc b/Platform/NXP/NxpQoriqLs.dsc.inc
> index fa5f30dd3909..b28e0615f7ca 100644
> --- a/Platform/NXP/NxpQoriqLs.dsc.inc
> +++ b/Platform/NXP/NxpQoriqLs.dsc.inc
> @@ -1,6 +1,6 @@
>  #  @file
>  #
> -#  Copyright 2017-2019 NXP.
> +#  Copyright 2017-2020 NXP.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -94,6 +94,8 @@ [LibraryClasses.common]
>
> NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
>
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>  
> +  I2cLib|Silicon/NXP/Library/I2cLib/I2cLib.inf
> +

I think the changes to this file belong in 2/28.

>  [LibraryClasses.common.SEC]
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>
> UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> diff --git a/Silicon/NXP/Include/Library/I2cLib.h 
> b/Silicon/NXP/Include/Library/I2cLib.h
> new file mode 100644
> index ..e39237abd3ee
> --- /dev/null
> +++ b/Silicon/NXP/Include/Library/I2cLib.h
> @@ -0,0 +1,120 @@
> +/** @file
> +  I2c Lib to control I2c controller.
> +
> +  Copyright 2020 NXP
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef I2C_LIB_H__
> +#define I2C_LIB_H__
> +
> +#include 
> +#include 
> +
> +/**
> +  software reset of the entire I2C module.
> +  The module is reset and disabled.
> +  Status register fields (IBSR) are cleared.
> +
> +  @param[in] Base   Base Address of I2c controller's registers
> +
> +  @return  EFI_SUCCESS  successfuly reset the I2c module
> +**/
> +EFI_STATUS
> +I2cReset (
> +  IN UINTN  Base
> +  );
> +
> +/**
> +  Early init I2C for reading the sysclk from I2c slave device.
> +  I2c bus clock is determined from the clock input to I2c controller.
> +  The clock input to I2c controller is derived from the sysclk.
> +  sysclk is determined by clock generator, which is controller by i2c.
> +
> +  So, it's a chicken-egg problem to read the sysclk from clock generator.
> +  To break this cycle (i.e. to read the sysclk), we setup the i2c bus clock 
> to
> +  lowest value, in the hope that it won't be out of clock generator's 
> supported
> +  i2c clock frequency. Once we have the correct sysclk, we can setup the
> +  correct i2c bus clock.
> +
> +  @param[in] Base   Base Address of I2c controller's registers
> +
> +  @return  EFI_SUCCESS  successfuly setup the i2c bus for reading sysclk
> +**/
> +EFI_STATUS
> +I2cEarlyInitialize (
> +  IN UINTN  Base
> +  );
> +
> +/**
> +  Configure I2c bus to operate at a given speed
> +
> +  @param[in] Base Base Address of I2c controller's registers
> +  @param[in] I2cBusClock  Input clock to I2c controller
> +  @param[in] Speedspeed to be configured for I2c bus
> +
> +  @return  EFI_SUCCESS  successfuly setup the i2c bus
> +**/
> +EFI_STATUS
> +I2cInitialize (
> +  IN UINTN  Base,
> +  IN UINT64 I2cBusClock,
> +  IN UINT64 Speed
> +  );
> +
> +/**
> +  Transfer data to/from I2c slave device
> +
> +  @param[in] Base   Base Address of I2c controller's registers
> +  @param[in] SlaveAddress   Slave Address from which data is to be read
> +  @param[in] RequestPacket  Pointer to an EFI_I2C_REQUEST_PACKET structure
> +describing the I2C transaction
> +
> +  

[edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Liran Alon]

Previous to this change, PvScsiFreeRings() was not undoing all
operations that was done by PvScsiInitRings().
This is because PvScsiInitRings() was both preparing rings (Allocate
memory and map it for device DMA) and setup the rings against device by
issueing a device command. While PvScsiFreeRings() only unmaps the rings
and free their memory.

Driver do not have a functional error as it makes sure to reset device
before every call site to PvScsiFreeRings(). However, this is not
intuitive.

Therefore, prefer to refactor the setup of the ring against device to a
separate function than PvScsiInitRings().

Signed-off-by: Liran Alon 
---
 OvmfPkg/PvScsiDxe/PvScsi.c | 150 +++--
 1 file changed, 76 insertions(+), 74 deletions(-)

diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index 1ca50390c0e5..5b7fdcbda10b 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.c
+++ b/OvmfPkg/PvScsiDxe/PvScsi.c
@@ -991,13 +991,6 @@ PvScsiInitRings (
   )
 {
   EFI_STATUS Status;
-  union {
-PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
-UINT32  Uint32;
-  } AlignedCmd;
-  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
-
-  Cmd = 
 
   Status = PvScsiAllocateSharedPages (
  Dev,
@@ -1032,6 +1025,69 @@ PvScsiInitRings (
   }
   ZeroMem (Dev->RingDesc.RingCmps, EFI_PAGE_SIZE);
 
+  return EFI_SUCCESS;
+
+FreeRingReqs:
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingReqs,
+>RingDesc.RingReqsDmaDesc
+);
+
+FreeRingState:
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingState,
+>RingDesc.RingStateDmaDesc
+);
+
+  return Status;
+}
+
+STATIC
+VOID
+PvScsiFreeRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingCmps,
+>RingDesc.RingCmpsDmaDesc
+);
+
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingReqs,
+>RingDesc.RingReqsDmaDesc
+);
+
+  PvScsiFreeSharedPages (
+Dev,
+1,
+Dev->RingDesc.RingState,
+>RingDesc.RingStateDmaDesc
+);
+}
+
+STATIC
+EFI_STATUS
+PvScsiSetupRings (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  union {
+PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
+UINT32  Uint32;
+  } AlignedCmd;
+  PVSCSI_CMD_DESC_SETUP_RINGS *Cmd;
+
+  Cmd = 
+
   ZeroMem (Cmd, sizeof (*Cmd));
   Cmd->ReqRingNumPages = 1;
   Cmd->CmpRingNumPages = 1;
@@ -1052,71 +1108,12 @@ PvScsiInitRings (
 sizeof (*Cmd) % sizeof (UINT32) == 0,
 "Cmd must be multiple of 32-bit words"
 );
-  Status = PvScsiWriteCmdDesc (
- Dev,
- PvScsiCmdSetupRings,
- (UINT32 *)Cmd,
- sizeof (*Cmd) / sizeof (UINT32)
- );
-  if (EFI_ERROR (Status)) {
-goto FreeRingCmps;
-  }
-
-  return EFI_SUCCESS;
-
-FreeRingCmps:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingCmps,
->RingDesc.RingCmpsDmaDesc
-);
-
-FreeRingReqs:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingReqs,
->RingDesc.RingReqsDmaDesc
-);
-
-FreeRingState:
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingState,
->RingDesc.RingStateDmaDesc
-);
-
-  return Status;
-}
-
-STATIC
-VOID
-PvScsiFreeRings (
-  IN OUT PVSCSI_DEV *Dev
-  )
-{
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingCmps,
->RingDesc.RingCmpsDmaDesc
-);
-
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingReqs,
->RingDesc.RingReqsDmaDesc
-);
-
-  PvScsiFreeSharedPages (
-Dev,
-1,
-Dev->RingDesc.RingState,
->RingDesc.RingStateDmaDesc
-);
+  return PvScsiWriteCmdDesc (
+   Dev,
+   PvScsiCmdSetupRings,
+   (UINT32 *)Cmd,
+   sizeof (*Cmd) / sizeof (UINT32)
+   );
 }
 
 STATIC
@@ -1157,6 +1154,10 @@ PvScsiInit (
   if (EFI_ERROR (Status)) {
 goto RestorePciAttributes;
   }
+  Status = PvScsiSetupRings (Dev);
+  if (EFI_ERROR (Status)) {
+goto FreeRings;
+  }
 
   //
   // Allocate DMA communication buffer
@@ -1168,7 +1169,7 @@ PvScsiInit (
  >DmaBufDmaDesc
  );
   if (EFI_ERROR (Status)) {
-goto FreeRings;
+goto UnsetupRings;
   }
 
   //
@@ -1202,13 +1203,14 @@ PvScsiInit (
 
   return EFI_SUCCESS;
 
-FreeRings:
+UnsetupRings:
   //
   // Reset device to stop device usage of the rings.
   // This is required to safely free the rings.
   //
   PvScsiResetAdapter (Dev);
 
+FreeRings:
   PvScsiFreeRings (Dev);
 
 RestorePciAttributes:
-- 
2.20.1


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

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

[edk2-devel] [PATCH edk2-platforms 1/1] Maintainers: switch to my @arm.com email address

[Ard Biesheuvel]

I no longer work for Linaro so switch to my ARM email address.

Signed-off-by: Ard Biesheuvel 
---
 Maintainers.txt | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 475f7d530b85..0e50b6fdf36a 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -80,7 +80,7 @@ EDK II Platforms Packages:
 
 96Boards
 F: Platform/96Boards/
-M: Ard Biesheuvel 
+M: Ard Biesheuvel 
 M: Leif Lindholm 
 
 AMD Seattle
@@ -88,24 +88,24 @@ F: Platform/AMD/OverdriveBoard/
 F: Platform/LeMaker/CelloBoard/
 F: Platform/SoftIron/
 F: Silicon/AMD/Styx/
-M: Ard Biesheuvel 
+M: Ard Biesheuvel 
 M: Leif Lindholm 
 
 ARM
 F: Platform/ARM/
-R: Ard Biesheuvel 
+R: Ard Biesheuvel 
 R: Thomas Abraham 
 M: Leif Lindholm 
 
 BeagleBoard:
 F: Platform/BeagleBoard/
 F: Silicon/TexasInstruments/
-R: Ard Biesheuvel 
+R: Ard Biesheuvel 
 M: Leif Lindholm 
 
 Comcast
 F: Platform/Comcast/
-M: Ard Biesheuvel 
+M: Ard Biesheuvel 
 M: Leif Lindholm 
 
 OptionRomPkg
@@ -116,14 +116,14 @@ M: Ray Ni 
 DisplayLink
 F: Drivers/DisplayLink/
 M: Leif Lindholm 
-M: Ard Biesheuvel 
+M: Ard Biesheuvel 
 R: Andy Hayes 
 
 HiSilicon
 F: Platform/Hisilicon/
 F: Silicon/Hisilicon/
 M: Leif Lindholm 
-R: Ard Biesheuvel 
+R: Ard Biesheuvel 
 
 Features/Intel
 F: Features/Intel/
@@ -241,7 +241,7 @@ Miscellaneous drivers
 F: Silicon/Atmel/
 F: Silicon/Openmoko/
 F: Silicon/Synopsys/DesignWare/
-R: Ard Biesheuvel 
+R: Ard Biesheuvel 
 M: Leif Lindholm 
 
 NXP platforms and silicon
@@ -253,7 +253,7 @@ R: Meenakshi Aggarwal 
 Raspberry Pi platforms and silicon
 F: Platform/RaspberryPi/
 F: Silicon/Broadcom/
-M: Ard Biesheuvel 
+M: Ard Biesheuvel 
 M: Leif Lindholm 
 R: Pete Batard 
 
@@ -261,5 +261,5 @@ Socionext platforms and silicon
 F: Platform/Socionext/
 F: Silicon/NXP/Library/Pcf8563RealTimeClockLib/
 F: Silicon/Socionext/
-M: Ard Biesheuvel 
+M: Ard Biesheuvel 
 M: Leif Lindholm 
-- 
2.17.1


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

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

[edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

[Liran Alon]

Sean reported that VS2019 build produce the following build error:
INFO - PvScsi.c
INFO - Generating code
INFO - d:\a\1\s\OvmfPkg\PvScsiDxe\PvScsi.c(459): error C2220: the following 
warning is treated as an error
INFO - d:\a\1\s\OvmfPkg\PvScsiDxe\PvScsi.c(459): warning C4244: '=': conversion 
from 'const UINT16' to 'UINT8', possible loss of data

This result from an implicit cast from PVSCSI Response->ScsiStatus
(Which is UINT16) to Packet->TargetResponse (Which is UINT8).

Fix this issue by adding an appropriate explicit cast and verify with
assert that this truncation do not result in loss of data.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2651
Reported-by: Sean Brogan 
Signed-off-by: Liran Alon 
---
 OvmfPkg/PvScsiDxe/PvScsi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index 0a66c98421a9..1ca50390c0e5 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.c
+++ b/OvmfPkg/PvScsiDxe/PvScsi.c
@@ -455,8 +455,12 @@ HandleResponse (
 
   //
   // Report target status
+  // (Strangely, PVSCSI interface defines Response->ScsiStatus as UINT16.
+  // But it should de-facto always have a value that fits UINT8. To avoid
+  // unexpected behavior, verify value is in UINT8 bounds before casting)
   //
-  Packet->TargetStatus = Response->ScsiStatus;
+  ASSERT (Response->ScsiStatus <= MAX_UINT8);
+  Packet->TargetStatus = (UINT8)Response->ScsiStatus;
 
   //
   // Host adapter status and function return value depend on
-- 
2.20.1


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

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

[edk2-devel] [PATCH] Maintainers.txt: Add Liran and Nikita as OvmfPkg/PvScsiDxe reviewers

[Liran Alon]

Laszlo suggested that as I have contributed the OvmfPkg PVSCSI driver, I
will also register myself as a reviewer in Maintainers.txt.

In addition, as Nikita have assisted the development of the PVSCSI
driver and have developed another similar OvmfPkg SCSI driver, add him
as a reviewer to PVSCSI driver as-well.

Cc: Nikita Leshenko 
Suggested-by: Laszlo Ersek 
Signed-off-by: Liran Alon 
---
 Maintainers.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 342bb8d0850c..de443ba7ba1f 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -435,6 +435,11 @@ OvmfPkg: CSM modules
 F: OvmfPkg/Csm/
 R: David Woodhouse 
 
+OvmfPkg: PVSCSI driver
+F: OvmfPkg/PvScsiDxe
+R: Liran Alon 
+R: Nikita Leshenko 
+
 PcAtChipsetPkg
 F: PcAtChipsetPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/PcAtChipsetPkg
-- 
2.20.1


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

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

Re: [edk2-devel] [PATCH v2 28/28] Platform/NXP/LS1043aRdbPkg: Add PEI Phase

[Leif Lindholm]

On Tue, Mar 31, 2020 at 10:23:48 +, Pankaj Bansal (OSS) wrote:
> > > -/*++
> > > -
> > > -Routine Description:
> > > -
> > > -
> > > -
> > > -Arguments:
> > > -
> > > -  FileHandle  - Handle of the file being invoked.
> > > -  PeiServices - Describes the list of possible PEI Services.
> > > -
> > > -Returns:
> > > -
> > > -  Status -  EFI_SUCCESS if the boot mode could be set
> > > -
> > > ---*/
> > 
> > The above line caused me an unexpected level of excitement this
> > morning, as my "put back the CRs SMTP strips out" script treated the
> > --- as a diff separator.
> > 
> > Now, I *have* seen the use of /*++ --*/ elsewhere in the tree, but
> > this syntax is *not* described in the coding style and should not be
> > used. While this is a delete statement, there is an addition below
> > using the same format. The doxygen tags to use are /** and **/.
> > 
> > Fortunately, I can't spot any of these in the rest of the set.
> > 
> > Please send an updated version of this patch - alone if it's the only
> > patch that needs changes, or with a v4 if such is required.
> 
> I have not received any comments on other patches so far.
> Does that mean all patches are OK (except above)?
> If that is the case, then I can send only this patch after update.
> If some rework is needed for other patches as well, I will send this patch 
> along with other reworked patches in v3.

No, sorry, going through them now.
Had a lot of random interruptions.

I commented on this first because it caused an error when I was
importing the set.

/
Leif

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

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

Re: [edk2-devel] [edk2-platforms][PATCH v3 9/9] Maintainers.txt: Update Arm platform

[Ard Biesheuvel]

On Wed, 25 Mar 2020 at 11:53, Aditya Angadi  wrote:
>
> From: Thomas Abraham 
>
> Update the reviewers list for Arm platforms.
>
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Sami Mujawar 
> Signed-off-by: Aditya Angadi 

Thanks - I'll take this as a separate patch and apply it right away.


> ---
>  Maintainers.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index a19a64fc42d0..475f7d530b85 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -94,6 +94,7 @@ M: Leif Lindholm 
>  ARM
>  F: Platform/ARM/
>  R: Ard Biesheuvel 
> +R: Thomas Abraham 
>  M: Leif Lindholm 
>
>  BeagleBoard:
> --
> 2.17.1
>

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

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

Re: [edk2-devel] [edk2-platforms][PATCH v3 8/9] Platform/ARM/Sgi: add initial support

[Ard Biesheuvel]

Again, please don't use duplicate patch titles in the same series.


On Wed, 25 Mar 2020 at 11:53, Aditya Angadi  wrote:
>
> For RD-Daniel Config-XLR, use multichip mode information from the SGI
> platform descriptor HOB to pick the correct ACPI table to be installed.
>
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Signed-off-by: Aditya Angadi 
> ---
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c   | 5 +
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf | 1 +
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h   | 1 +
>  3 files changed, 7 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index 7e0de765f753..b1f5714b934d 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -51,6 +51,11 @@ STATIC SGI_PLATFORM_ACPI_TABLE_GUID_LOOKUP 
> AcpiTableGuidLookup[] = {
>RD_DANIEL_CFGM_CONF_ID,
>MULTI_CHIP_MODE_DISABLED,
>),
> +  ACPI_GUID_LOOKUP (
> +  RD_DANIEL_PART_NUM,
> +  RD_DANIEL_CFGXLR_CONF_ID,
> +  MULTI_CHIP_MODE_ENABLED,
> +  ),
>  };
>
>  VOID
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf 
> b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index 82569820b78c..00cbe608c219 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -35,6 +35,7 @@ [Guids]
>gRdN1EdgeX2AcpiTablesFileGuid
>gRdE1EdgeAcpiTablesFileGuid
>gRdDanielCfgMAcpiTablesFileGuid
> +  gRdDanielCfgXlrAcpiTablesFileGuid
>
>  [FeaturePcd]
>gArmSgiTokenSpaceGuid.PcdVirtioBlkSupported
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h 
> b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> index b6a427b8b657..9822858f6ea0 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -73,6 +73,7 @@
>  //RDDANIEL Platform Identification values
>  #define RD_DANIEL_PART_NUM0x78A
>  #define RD_DANIEL_CFGM_CONF_ID0x1
> +#define RD_DANIEL_CFGXLR_CONF_ID  0x2
>
>  #define SGI_CONFIG_MASK   0x0F
>  #define SGI_CONFIG_SHIFT  0x1C
> --
> 2.17.1
>

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

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

Re: [edk2-devel] [edk2-platforms][PATCH v3 7/9] Platform/ARM/SgiPkg: add ACPI tables

[Ard Biesheuvel]

Please don't use the exact same subject for different patches in the
same series.

On Wed, 25 Mar 2020 at 11:53, Aditya Angadi  wrote:
>
> RD-Daniel Config-XLR is a platform in which four identical chips are
> connected via a high speed CCIX link. Add Madt and Dsdt tables for the
> same.
>
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Signed-off-by: Aditya Angadi 
> ---
>  Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Dsdt.asl  | 125 
> 
>  Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Madt.aslc | 150 
> 
>  Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlrAcpiTables.inf |  63 
>  Platform/ARM/SgiPkg/SgiPlatform.dec |   1 +
>  Platform/ARM/SgiPkg/SgiPlatform.dsc |   1 +
>  Platform/ARM/SgiPkg/SgiPlatform.fdf |   1 +
>  6 files changed, 341 insertions(+)
>

Please include the .DSC for this platform as well.

> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Dsdt.asl 
> b/Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Dsdt.asl
> new file mode 100644
> index ..23ada55ec4a1
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Dsdt.asl
> @@ -0,0 +1,125 @@
> +/** @file
> +*  Differentiated System Description Table Fields (DSDT)
> +*
> +*  Copyright (c) 2020, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include "SgiPlatform.h"
> +#include "SgiAcpiHeader.h"
> +
> +DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "ARMLTD", "ARMSGI",
> + EFI_ACPI_ARM_OEM_REVISION) {
> +  Scope (_SB) {
> +
> +Device (CP00) { // Zeus core 0
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 0)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP01) { // Zeus core 1
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 1)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP02) { // Zeus core 2
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 2)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP03) { // Zeus core 3
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 3)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP04) { // Zeus core 4
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 4)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP05) { // Zeus core 5
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 5)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP06) { // Zeus core 6
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 6)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP07) { // Zeus core 7
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 7)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP08) { // Zeus core 8
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 8)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP09) { // Zeus core 9
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 9)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP10) { // Zeus core 10
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 10)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP11) { // Zeus core 11
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 11)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP12) { // Zeus core 12
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 12)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP13) { // Zeus core 13
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 13)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP14) { // Zeus core 14
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 14)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP15) { // Zeus core 15
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 15)
> +  Name (_STA, 0xF)
> +}
> +
> +Device (CP16) { // Zeus core 16
> +  Name (_HID, "ACPI0007")
> +  Name (_UID, 16)
> +  Name (_STA, 0xF)
> +}
> +  } // Scope(_SB)
> +}
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Madt.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Madt.aslc
> new file mode 100644
> index ..e3784b55f2e5
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdDanielCfgXlr/Madt.aslc
> @@ -0,0 +1,150 @@
> +/** @file
> +*  Multiple APIC Description Table (MADT)
> +*
> +*  Copyright (c) 2020, ARM Limited. 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

Re: [edk2-devel] [edk2-platforms][PATCH v3 4/9] Platform/ARM/SgiPkg: remove

[Ard Biesheuvel]

On Wed, 25 Mar 2020 at 11:53, Aditya Angadi  wrote:
>
> The number of CPUs depend on the SGI/RD platform. So instead of

depends

> defining a Fixed PCD to specify the value of core and cluster count,
> let each platform define these values.
>
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Signed-off-by: Aditya Angadi 

Reviewed-by: Ard Biesheuvel 

> ---
>  Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc   |  7 ++-
>  Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf   |  2 --
>  Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Madt.aslc   |  7 ---
>  Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf   |  2 --
>  Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Madt.aslc | 12 ++--
>  Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf |  2 --
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Madt.aslc |  9 +
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf |  4 +---
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf |  3 ---
>  Platform/ARM/SgiPkg/SgiPlatform.dsc |  4 
>  10 files changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc
> index 987254928535..a9540f2e0374 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc
> @@ -14,12 +14,17 @@
>  #include 
>  #include 
>
> +#define CLUSTER_COUNT 2
> +#define CORES_PER_CLUSTER 8
> +#define THREADS_PER_CORE  2
> +#define CORE_COUNT(CLUSTER_COUNT * CORES_PER_CLUSTER * 
> THREADS_PER_CORE)
> +
>  // Multiple APIC Description Table
>  #pragma pack (1)
>
>  typedef struct {
>EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
> -  EFI_ACPI_6_2_GIC_STRUCTUREGicInterfaces[32];
> +  EFI_ACPI_6_2_GIC_STRUCTURE
> GicInterfaces[CORE_COUNT];
>EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTUREGicDistributor;
>EFI_ACPI_6_2_GICR_STRUCTURE   GicRedistributor;
>EFI_ACPI_6_2_GIC_ITS_STRUCTUREGicIts;
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf 
> b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> index b08d7c2df5c7..742ca9e68335 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
> @@ -34,8 +34,6 @@ [Packages]
>Platform/ARM/SgiPkg/SgiPlatform.dec
>
>  [FixedPcd]
> -  gArmPlatformTokenSpaceGuid.PcdCoreCount
> -  gArmPlatformTokenSpaceGuid.PcdClusterCount
>gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Madt.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Madt.aslc
> index 05eb78c5616a..d8ec0ce421dc 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Madt.aslc
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Madt.aslc
> @@ -14,15 +14,16 @@
>  #include 
>  #include 
>
> -#define CORE_CNT   (FixedPcdGet32 (PcdClusterCount) * \
> -FixedPcdGet32 (PcdCoreCount))
> +#define CLUSTER_COUNT 2
> +#define CORES_PER_CLUSTER 4
> +#define CORE_COUNT(CLUSTER_COUNT * CORES_PER_CLUSTER)
>
>  // Multiple APIC Description Table
>  #pragma pack (1)
>
>  typedef struct {
>EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
> -  EFI_ACPI_6_2_GIC_STRUCTURE
> GicInterfaces[CORE_CNT];
> +  EFI_ACPI_6_2_GIC_STRUCTURE
> GicInterfaces[CORE_COUNT];
>EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTUREGicDistributor;
>EFI_ACPI_6_2_GICR_STRUCTURE   GicRedistributor;
>EFI_ACPI_6_2_GIC_ITS_STRUCTUREGicIts;
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf 
> b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
> index 61b07bffccf3..206479f9428e 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
> @@ -34,8 +34,6 @@ [Packages]
>Platform/ARM/SgiPkg/SgiPlatform.dec
>
>  [FixedPcd]
> -  gArmPlatformTokenSpaceGuid.PcdCoreCount
> -  gArmPlatformTokenSpaceGuid.PcdClusterCount
>gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
>gArmPlatformTokenSpaceGuid.PL011UartInterrupt
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Madt.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Madt.aslc
> index 47368931e367..add972437496 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Madt.aslc
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Madt.aslc
> @@ -14,19 +14,19 @@
>  #include 
>  #include 
>
> -#define CORE_CNT   (FixedPcdGet32 (PcdClusterCount) * \
> -FixedPcdGet32 (PcdCoreCount))
> -
> -#define CHIP_CNT   

Re: [edk2-devel] [edk2-platforms][PATCH v3 2/9] Platform/ARM/SgiPkg: move the GIC

[Ard Biesheuvel]

On Wed, 25 Mar 2020 at 11:53, Aditya Angadi  wrote:
>
> Move the ACPI helper macros defines related to GIC structure,
> distributor, redistributor and ITS to SgiAcpiHeader.h as these are
> common across ARM SGI/RD platforms.
>
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Signed-off-by: Aditya Angadi 

Reviewed-by: Ard Biesheuvel 

> ---
>  Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc   | 68 +--
>  Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Madt.aslc   | 68 +--
>  Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Madt.aslc | 57 +---
>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h | 70 +++-
>  4 files changed, 72 insertions(+), 191 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc 
> b/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc
> index 48e7a61478e8..987254928535 100644
> --- a/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc
> +++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1Edge/Madt.aslc
> @@ -1,7 +1,7 @@
>  /** @file
>  *  Multiple APIC Description Table (MADT)
>  *
> -*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -14,72 +14,6 @@
>  #include 
>  #include 
>
> -// EFI_ACPI_6_2_GIC_STRUCTURE
> -#define EFI_ACPI_6_2_GICC_STRUCTURE_INIT(GicId, AcpiCpuUid, Mpidr, Flags,
>   \
> -  PmuIrq, GicBase, GicVBase, GicHBase, GsivId, GicRBase, Efficiency) 
>   \
> -  {  
>   \
> -EFI_ACPI_6_2_GIC, /* Type */ 
>   \
> -sizeof (EFI_ACPI_6_2_GIC_STRUCTURE),  /* Length */   
>   \
> -EFI_ACPI_RESERVED_WORD,   /* Reserved */ 
>   \
> -GicId,/* CPUInterfaceNumber */   
>   \
> -AcpiCpuUid,   /* AcpiProcessorUid */ 
>   \
> -Flags,/* Flags */
>   \
> -0,/* ParkingProtocolVersion */   
>   \
> -PmuIrq,   /* PerformanceInterruptGsiv */ 
>   \
> -0,/* ParkedAddress */
>   \
> -GicBase,  /* PhysicalBaseAddress */  
>   \
> -GicVBase, /* GICV */ 
>   \
> -GicHBase, /* GICH */ 
>   \
> -GsivId,   /* VGICMaintenanceInterrupt */ 
>   \
> -GicRBase, /* GICRBaseAddress */  
>   \
> -Mpidr,/* MPIDR */
>   \
> -Efficiency,   /* ProcessorPowerEfficiencyClass 
> */  \
> -{
>   \
> -  EFI_ACPI_RESERVED_BYTE, /* Reserved2[0] */ 
>   \
> -  EFI_ACPI_RESERVED_BYTE, /* Reserved2[1] */ 
>   \
> -  EFI_ACPI_RESERVED_BYTE  /* Reserved2[2] */ 
>   \
> -}
>   \
> -  }
> -
> -// EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE
> -#define EFI_ACPI_6_2_GIC_DISTRIBUTOR_INIT(GicDistHwId, GicDistBase,  
>   \
> -  GicDistVector, GicVersion) 
>   \
> -  {  
>   \
> -EFI_ACPI_6_2_GICD,/* Type */ 
>   \
> -sizeof (EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE), 
>   \
> -EFI_ACPI_RESERVED_WORD,   /* Reserved1 */
>   \
> -GicDistHwId,  /* GicId */
>   \
> -GicDistBase,  /* PhysicalBaseAddress */  
>   \
> -GicDistVector,/* SystemVectorBase */ 
>   \
> -GicVersion,   /* GicVersion */   
>   \
> -{
>   \
> -  EFI_ACPI_RESERVED_BYTE, /* Reserved2[0] */ 
>   \
> -  EFI_ACPI_RESERVED_BYTE, /* Reserved2[1] */ 
>   \
> -  EFI_ACPI_RESERVED_BYTE  /* Reserved2[2] */ 
>   \
> -}
>   \
> -  }
> -
> -// EFI_ACPI_6_2_GICR_STRUCTURE
> -#define EFI_ACPI_6_2_GIC_REDISTRIBUTOR_INIT(RedisRegionAddr, 
> RedisDiscLength)  \
> -  {   

Re: [edk2-devel] [PATCH v2 28/28] Platform/NXP/LS1043aRdbPkg: Add PEI Phase

[Pankaj Bansal]

> -Original Message-
> From: Leif Lindholm 
> Sent: Monday, March 30, 2020 5:49 PM
> To: Pankaj Bansal (OSS) 
> Cc: Meenakshi Aggarwal ; Michael D Kinney
> ; devel@edk2.groups.io; Varun Sethi
> ; Samer El-Haj-Mahmoud  mahm...@arm.com>; Jon Nettleton 
> Subject: Re: [PATCH v2 28/28] Platform/NXP/LS1043aRdbPkg: Add PEI Phase
> 
> On Fri, Mar 20, 2020 at 20:05:43 +0530, Pankaj Bansal wrote:
> > From: Pankaj Bansal 
> >
> > Add PEI phase to LS1043aRdb. This is needed becuase we need to have
> > dynamic PCDs support to be able to reserve memory before reporting
> > memory to UEFI fimrware.
> >
> > Signed-off-by: Pankaj Bansal 
> > ---
> >  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc  |  9 ---
> >  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf  | 18 +++--
> >  .../MemoryInitPeiLib/MemoryInitPeiLib.c   | 77 ++-
> >  .../MemoryInitPeiLib/MemoryInitPeiLib.inf |  3 +-
> >  Silicon/NXP/NxpQoriqLs.dsc.inc| 59 ++
> >  5 files changed, 99 insertions(+), 67 deletions(-)
> >
> > diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > index d486c9b36fab..d45fd67c03b5 100644
> > --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> > @@ -30,15 +30,6 @@ [LibraryClasses.common]
> >RealTimeClockLib|Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf
> >
> >  [PcdsFixedAtBuild.common]
> > -
> > -  #
> > -  # LS1043a board Specific PCDs
> > -  # XX (DRAM - Region 1 2GB)
> > -  # (NOR - IFC Region 1 512MB)
> > -  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x8000
> > -  gArmTokenSpaceGuid.PcdSystemMemorySize|0x7BE0
> > -
> gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0200
> > -
> >#
> ># RTC Pcds
> >#
> > diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> > index 99fbc87e1200..931d0bb14f9b 100644
> > --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> > +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> > @@ -24,10 +24,10 @@
> >
> >  [FD.LS1043ARDB_EFI]
> >  BaseAddress   = 0x8200|gArmTokenSpaceGuid.PcdFdBaseAddress  #The
> base address of the FLASH Device.
> > -Size  = 0x000ED000|gArmTokenSpaceGuid.PcdFdSize #The size 
> > in
> bytes of the FLASH Device
> > +Size  = 0x0014|gArmTokenSpaceGuid.PcdFdSize #The size 
> > in
> bytes of the FLASH Device
> >  ErasePolarity = 1
> > -BlockSize = 0x1
> > -NumBlocks = 0xED000
> > +BlockSize = 0x4
> > +NumBlocks = 0x5
> >
> >
> #
> ###
> >  #
> > @@ -44,7 +44,7 @@ [FD.LS1043ARDB_EFI]
> >  # RegionType 
> >  #
> >
> #
> ###
> > -0x|0x000ED000
> > +0x|0x0014
> >  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> >  FV = FVMAIN_COMPACT
> >
> > @@ -159,7 +159,15 @@ [FV.FVMAIN_COMPACT]
> >  READ_LOCK_CAP  = TRUE
> >  READ_LOCK_STATUS   = TRUE
> >
> > -  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
> > +  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> > +  INF MdeModulePkg/Core/Pei/PeiMain.inf
> > +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> > +  INF
> MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> > +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> > +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> > +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
> > +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> > +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> >
> >FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> >  SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF
> PROCESSING_REQUIRED = TRUE {
> > diff --git a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> > index 54d026ef1270..7fdf9cb77a6e 100644
> > --- a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> > +++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> > @@ -46,30 +46,12 @@ InitMmu (
> >}
> >  }
> >
> > -/*++
> > -
> > -Routine Description:
> > -
> > -
> > -
> > -Arguments:
> > -
> > -  FileHandle  - Handle of the file being invoked.
> > -  PeiServices - Describes the list of possible PEI Services.
> > -
> > -Returns:
> > -
> > -  Status -  EFI_SUCCESS if the boot mode could be set
> > -
> > ---*/
> 
> The above line caused me an unexpected level of excitement this
> morning, as my "put back the CRs SMTP strips out" script treated the
> --- as a diff separator.
> 
> Now, I *have* seen the use of /*++ --*/ elsewhere in the tree, but
> this syntax is *not* described in the coding style and should not be
> used. While this is a delete statement, there is an addition below
> using the same format. The doxygen tags to use are /** and **/.
> 
> Fortunately, I can't spot any of these 

Re: [edk2-devel] [PATCH] Maintainers: switch to my Arm email address

[Ard Biesheuvel]

On Thu, 5 Mar 2020 at 15:33, Laszlo Ersek  wrote:
>
> On 03/05/20 13:30, Leif Lindholm wrote:
> > On Thu, Mar 05, 2020 at 12:23:46 +0100, Ard Biesheuvel wrote:
> >> I no longer work for Linaro (and haven't for a while) so in anticipation
> >> of losing access to my @linaro.org mailbox, let's switch to the ARM one
> >> for my Tiancore contributions and maintainerships. Update the .mailmap
> >> at the same time so the tooling can rewrite history where needed.
> >
> > I have no issue either way, but I also don't think a proper decision
> > was made last time around the .mailmap topic came up as to whether it
> > was intended to span organisation changes.
>
> We said that stewards wouldn't have the power to approve such, but the
> subject contributors would.
>
> > At present it does not in this project, and I would prefer for such a
> > change to happen after an explicit policy decision rather than by
> > accident.
> >
> > In the meantime, for the Maintainers.txt changes:
> > Reviewed-by: Leif Lindholm 
>
> I'm fine with both keeping and dropping the .mailmap hunk, as it is
> being proposed by Ard.
>
> So either way:
>
> Reviewed-by: Laszlo Ersek 
>

Thanks all

Given the fact that the mailmap is only used when explicitly asked for
it, adding the entry is not going to make it more likely that
unsuspecting contributors will end up using my @arm.com email address
after looking at my contributions in the Git history. So I dropped the
mailmap hunk and pushed this to edk2 master.

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

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

[edk2-devel] How to change RAID Card's configuration without going into its setup ui

[Tiger Liu(BJ-RD)]

Hi, Experts:
I am studying how to remote control some hardware's configuration.

Such as changing RAID Card's configuration without going into its setup ui.

So, one question confused me.
If RAID Card's firmware is written with UEFI driver model, so I can use some 
standard UEFI protocol to pass some configurations to RAID card's firmware?
Or must use RAID Card vendor's private protocol to transfer new configurations?

Such as:
EFI Adapter Information Protocol, SetInformation function.

Thanks


保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.

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

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

Re: [edk2-devel] API breakages and their implications. Was: [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

[Leif Lindholm]

On Tue, Mar 31, 2020 at 01:53:21 +, Ni, Ray wrote:
> Leif,
> Please understand that the concern of this change is all the platforms that 
> uses
> this serial port lib must be changed otherwise build breaks.

Yes. This is the nature of collaborative development.
This is something we on the ARM side have lived with since we got
involved with EDK2, being the less-deployed user of the codebase, and
that is fine.

TianoCore specifically produced the UDK to make life easier for those
who are unable to track upstream, and we have followed that up with
stable tags. I would highly recommend that any team that feels that
this change would be too much effort to move to edk2-stable202002. Of
course, they would then need to manually cherry-pick any improvements
and security fixes from master. That is their choice.

Nevertheless, breaking existing platforms is a side effect, not a
goal. So if an alternative solution can be found (which is not a hack
that is going to affect the codebase negatively over time and simply
need to be fixed properly at a later date), then clearly that is
preferable.

"This breaks many platforms" is a good argument for seeing if a
solution can be found that does not break (as) many platforms. It is
not an argument for duplicating drivers when the change needed for
those platforms is trivial.

These days, Linux tends to deal with API breakages (and other things)
using a semantic patch tool called Coccinelle[1]. It would not be
unreasonable, and indeed it would be helpful to us on the non-x86 side
if something similar was adopted (and semantic patches mandated) for
API breakages in EDK2.

[1] http://coccinelle.lip6.fr/sp.php

Regards,

Leif

> Ard,
> Using Guided HOB sounds a good idea to me: )
> The benefits of using HOB is:
>   Length field in the HOB header can be used for extension if more parameters 
> are needed.
>   DXE can have the HOB access as well.
> 
> EFI_SEC_HOB_DATA_PPI can be used to return the new Guided HOB from SEC phase 
> if needed.
> 
> Thanks,
> Ray
> 
> 
> > -Original Message-
> > From: Ard Biesheuvel 
> > Sent: Monday, March 30, 2020 3:45 PM
> > To: Leif Lindholm 
> > Cc: Jiang, Guomin ; devel@edk2.groups.io; 
> > pankaj.ban...@nxp.com; Ni, Ray ;
> > Wang, Jian J ; Wu, Hao A ; Ma, 
> > Maurice ; Dong,
> > Guo ; You, Benjamin ; Meenakshi 
> > Aggarwal
> > ; Varun Sethi ; Samer 
> > El-Haj-Mahmoud  > mahm...@arm.com>
> > Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq 
> > Support
> > 
> > On Mon, 30 Mar 2020 at 09:35, Leif Lindholm  wrote:
> > >
> > > Hi Jiang,
> > >
> > > It is not a question of effort of copying a driver, it is a question
> > > that copying drivers is something that should be avoided wherever
> > > practically possible. I did not think this topic was still under
> > > debate.
> > >
> > > If the existing 16550 SerialPortLib is overspecialised to the point
> > > where it only works on a subset of 16550 implementations, then it
> > > should change. There are going to be more non-PC systems turning up
> > > with 16550 UARTs - should they each copy/modify their drivers?
> > >
> > > If there are better ways of solving that problem, please suggest.
> > > But more duplicated drivers is not the answer.
> > >
> > 
> > Could we use a GUIDed HOB? If it exists, we use its contents, and if
> > it doesn't, we use the default set by the FixedPCD.

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

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

Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow.

[Maciej Rabeda]

Siyuan/Jiaxin,

Can we get this patch reviewed? I cannot give myself green light on this 
patch :)


On 26-Mar-20 04:16, Gao, Zhichao wrote:

The ping command implementation would go into the ip4/ip6 protocol. But I am 
not familiar with the network part.

Jiaxin/Siyuan,

Can you help to review this patch?

Thanks,
Zhichao


-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Wednesday, March 25, 2020 7:34 PM
To: Ni, Ray ; Gao, Zhichao 
Cc: devel@edk2.groups.io; maciej.rab...@linux.intel.com
Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4
receive flow.

Ray, Zhichao,

On 02/27/20 12:02, Maciej Rabeda wrote:

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

'ping' command's receive flow utilizes a single Rx token which it
attempts to reuse before recycling the previously received packet.
This causes a situation where under ICMP traffic,
Ping6OnEchoReplyReceived() function will receive an already recycled
packet with EFI_SUCCESS token status and finally dereference invalid
pointers from RxData structure.

Cc: Ray Ni 
Cc: Zhichao Gao 
Signed-off-by: Maciej Rabeda 
---
  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

can you please review this ShellPkg patch? It's been on the list for almost a
month now.

Thanks
Laszlo


diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index 23567fa2c1bb..a3fa32515192 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived (

  ON_EXIT:

+  //
+  // Recycle the packet before reusing RxToken  //  gBS->SignalEvent
+ (Private->IpChoice ==
+ PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
RxToken.Packet.R
+ xData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private-
RxToken.Packe
+ t.RxData)->RecycleSignal);
+
if (Private->RxCount < Private->SendNum) {
  //
  // Continue to receive icmp echo reply packets.
@@ -632,10 +637,6 @@ ON_EXIT:
  //
  Private->Status = EFI_SUCCESS;
}
-  //
-  // Singal to recycle the each rxdata here, not at the end of process.
-  //
-  gBS->SignalEvent (Private->IpChoice ==
PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
RxToken.Packet.RxD
ata)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private-
RxToken.Packet.Rx
Data)->RecycleSignal);
  }

  /**







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

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

Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

[Guomin Jiang]

There is a spell error in the comments of VariableServiceGetVariable() in 
Variable.c.
- @return EFI_BUFFER_TO_SMALL   DataSize is too small for the result.
+ @return EFI_BUFFER_TOO_SMALLDataSize is too small for the result.

Need create new bugs for it or fix in this comment directly?

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wang, Jian J
> Sent: Monday, March 30, 2020 12:16 PM
> To: michael.kuba...@outlook.com; devel@edk2.groups.io
> Cc: Bret Barkelew ; Gao, Liming
> ; Kinney, Michael D ;
> Wu, Hao A 
> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
> GetVariable() attr if EFI_BUFFER_TOO_SMALL
> 
> 
> Reviewed-by: Jian J Wang 
> 
> Regards,
> Jian
> 
> > -Original Message-
> > From: michael.kuba...@outlook.com 
> > Sent: Saturday, March 28, 2020 5:56 AM
> > To: devel@edk2.groups.io
> > Cc: Bret Barkelew ; Gao, Liming
> > ; Kinney, Michael D
> > ; Wang, Jian J ;
> > Wu, Hao A 
> > Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
> > attr if EFI_BUFFER_TOO_SMALL
> >
> > From: Michael Kubacki 
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >
> > The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> > "Attributes" parameter description states:
> >
> > "If not NULL, a pointer to the memory location to return the
> > attributes bitmask for the variable. See 'Related Definitions.'
> >  If not NULL, then Attributes is set on output both when  EFI_SUCCESS
> > and when EFI_BUFFER_TOO_SMALL is returned."
> >
> > The attributes were previously only returned from the implementation
> > in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
> > EFI_BUFFER_TOO_SMALL according to spec.
> >
> > Cc: Bret Barkelew 
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Jian J Wang 
> > Cc: Hao A Wu 
> > Signed-off-by: Michael Kubacki 
> > ---
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c  | 10
> > +++---
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c |
> > 10 ++
> >  2 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > index d23aea4bc712..1e71fc642c76 100644
> > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > @@ -18,6 +18,8 @@
> >
> >  Copyright (c) 2006 - 2020, Intel Corporation. All rights
> > reserved.
> >  (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP
> > +Copyright (c) Microsoft Corporation.
> > +
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
> >  }
> >
> >  CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
> > mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
> > -if (Attributes != NULL) {
> > -  *Attributes = Variable.CurrPtr->Attributes;
> > -}
> >
> >  *DataSize = VarDataSize;
> >  UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile,
> > TRUE, FALSE, FALSE, FALSE, ); @@ -2395,6 +2394,11 @@
> > VariableServiceGetVariable (
> >}
> >
> >  Done:
> > +  if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> > +if (Attributes != NULL && Variable.CurrPtr != NULL) {
> > +  *Attributes = Variable.CurrPtr->Attributes;
> > +}
> > +  }
> >ReleaseLockOnlyAtBootTime (
> > >VariableGlobal.VariableServicesLock);
> >return Status;
> >  }
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > index 2cf0ed32ae55..ca833fb0244d 100644
> > ---
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > @@ -14,6 +14,7 @@
> >InitCommunicateBuffer() is really function to check the variable data 
> > size.
> >
> >  Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) Microsoft Corporation.
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
> >  }
> >
> >  CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
> > mVariableAuthFormat), TempDataSize);
> > -if (Attributes != NULL) {
> > -  *Attributes = RtPtrTrack.CurrPtr->Attributes;
> > -}
> > -
> >  *DataSize = TempDataSize;
> >
> >  UpdateVariableInfo (VariableName, VendorGuid,
> > RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, ); @@
> > -661,6 +658,11 @@ FindVariableInRuntimeCache (
> >}
> >
> >  Done:
> > +  if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> > +if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
> > +  *Attributes = 

Re: [edk2-devel] [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

[Guomin Jiang]

Reviewed-by: Guomin Jiang 

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wang, Jian J
> Sent: Monday, March 30, 2020 12:19 PM
> To: michael.kuba...@outlook.com; devel@edk2.groups.io
> Cc: Bret Barkelew ; Gao, Liming
> ; Kinney, Michael D ;
> Wu, Hao A 
> Subject: Re: [edk2-devel] [PATCH v3 2/3] MdeModulePkg VariablePei:
> Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
> 
> Reviewed-by: Jian J Wang 
> 
> Regards,
> Jian
> 
> > -Original Message-
> > From: michael.kuba...@outlook.com 
> > Sent: Saturday, March 28, 2020 5:56 AM
> > To: devel@edk2.groups.io
> > Cc: Bret Barkelew ; Gao, Liming
> > ; Kinney, Michael D
> > ; Wang, Jian J ;
> > Wu, Hao A 
> > Subject: [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable()
> > attr if EFI_BUFFER_TOO_SMALL
> >
> > From: Michael Kubacki 
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >
> > This commit makes the behavior for PeiGetVariable() match the
> > following specification-defined behavior. It is now consistent with
> > the DXE/SMM variable driver implementation.
> >
> > The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> > "Attributes" parameter description states:
> >
> > "If not NULL, a pointer to the memory location to return the
> > attributes bitmask for the variable. See 'Related Definitions.'
> >  If not NULL, then Attributes is set on output both when  EFI_SUCCESS
> > and when EFI_BUFFER_TOO_SMALL is returned."
> >
> > The attributes were previously only returned from the implementation
> > in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
> > EFI_BUFFER_TOO_SMALL according to spec.
> >
> > Cc: Bret Barkelew 
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Jian J Wang 
> > Cc: Hao A Wu 
> > Signed-off-by: Michael Kubacki 
> > ---
> >  MdeModulePkg/Universal/Variable/Pei/Variable.c | 19
> > ++-
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > index f61465fc3045..f420b58165b7 100644
> > --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > @@ -3,6 +3,7 @@
> >PEI ReadOnly Varaiable2 PPI. These services operates the non
> > volatile storage space.
> >
> >  Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) Microsoft Corporation.
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -1047,17 +1048,17 @@ PeiGetVariable (
> >  }
> >
> >  GetVariableNameOrData (, GetVariableDataPtr
> > (Variable.CurrPtr, VariableHeader, StoreInfo.AuthFlag), VarDataSize,
> > Data);
> > -
> > -if (Attributes != NULL) {
> > -  *Attributes = VariableHeader->Attributes;
> > -}
> > -
> > -*DataSize = VarDataSize;
> > -return EFI_SUCCESS;
> > +Status = EFI_SUCCESS;
> >} else {
> > -*DataSize = VarDataSize;
> > -return EFI_BUFFER_TOO_SMALL;
> > +Status = EFI_BUFFER_TOO_SMALL;
> >}
> > +
> > +  if (Attributes != NULL) {
> > +*Attributes = VariableHeader->Attributes;  }  *DataSize =
> > + VarDataSize;
> > +
> > +  return Status;
> >  }
> >
> >  /**
> > --
> > 2.16.3.windows.1
> 
> 
> 


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

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

Re: [edk2-devel] [PATCH v2 1/2] DynamicTablesPkg: SRAT: Fix entry points

[Ard Biesheuvel]

On Tue, 31 Mar 2020 at 09:26, Sami Mujawar  wrote:
>
> VS2017 reports 'warning C4028: formal parameter 2 different
> from declaration' for the library constructor and destructor
> interfaces for the SRAT Generator modules.
>
> Remove the CONST qualifier for the ImageHandle and the
> SystemTable pointer in the library constructor and destructor
> to make it compatible with the formal declaration.
>
> Signed-off-by: Sami Mujawar 

Reviewed-by: Ard Biesheuvel 

> ---
>
> The changes can be seen at:
> https://github.com/samimujawar/edk2/tree/702_srat_vs2017_compile_warning_v2
>
> Notes:
> V2:
> - Update commit message to reflect the update to the CONST  [SAMI]
>   qualifier at 2 places in the constructor & destructor.
>
>  DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c 
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
> index 
> 5d56af66608d862e6eca81da812d719f110867d2..74cb7d92a5d8cddd3df8334f3ab55e6fa3e7267a
>  100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
> @@ -800,8 +800,8 @@ ACPI_TABLE_GENERATOR SratGenerator = {
>  EFI_STATUS
>  EFIAPI
>  AcpiSratLibConstructor (
> -  IN CONST EFI_HANDLEImageHandle,
> -  IN   EFI_SYSTEM_TABLE  * CONST SystemTable
> +  IN  EFI_HANDLE   ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *  SystemTable
>)
>  {
>EFI_STATUS  Status;
> @@ -823,8 +823,8 @@ AcpiSratLibConstructor (
>  EFI_STATUS
>  EFIAPI
>  AcpiSratLibDestructor (
> -  IN CONST EFI_HANDLEImageHandle,
> -  IN   EFI_SYSTEM_TABLE  * CONST SystemTable
> +  IN  EFI_HANDLE   ImageHandle,
> +  IN  EFI_SYSTEM_TABLE  *  SystemTable
>)
>  {
>EFI_STATUS  Status;
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>

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

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

[edk2-devel] [PATCH v2 1/2] DynamicTablesPkg: SRAT: Fix entry points

[Sami Mujawar]

VS2017 reports 'warning C4028: formal parameter 2 different
from declaration' for the library constructor and destructor
interfaces for the SRAT Generator modules.

Remove the CONST qualifier for the ImageHandle and the
SystemTable pointer in the library constructor and destructor
to make it compatible with the formal declaration.

Signed-off-by: Sami Mujawar 
---

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/702_srat_vs2017_compile_warning_v2

Notes:
V2:
- Update commit message to reflect the update to the CONST  [SAMI]
  qualifier at 2 places in the constructor & destructor.

 DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c 
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
index 
5d56af66608d862e6eca81da812d719f110867d2..74cb7d92a5d8cddd3df8334f3ab55e6fa3e7267a
 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
@@ -800,8 +800,8 @@ ACPI_TABLE_GENERATOR SratGenerator = {
 EFI_STATUS
 EFIAPI
 AcpiSratLibConstructor (
-  IN CONST EFI_HANDLEImageHandle,
-  IN   EFI_SYSTEM_TABLE  * CONST SystemTable
+  IN  EFI_HANDLE   ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *  SystemTable
   )
 {
   EFI_STATUS  Status;
@@ -823,8 +823,8 @@ AcpiSratLibConstructor (
 EFI_STATUS
 EFIAPI
 AcpiSratLibDestructor (
-  IN CONST EFI_HANDLEImageHandle,
-  IN   EFI_SYSTEM_TABLE  * CONST SystemTable
+  IN  EFI_HANDLE   ImageHandle,
+  IN  EFI_SYSTEM_TABLE  *  SystemTable
   )
 {
   EFI_STATUS  Status;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

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

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Ard Biesheuvel]

On Tue, 31 Mar 2020 at 03:40, Jiang, Guomin  wrote:
>
> Hi Laszlo,
>
> Thanks for you spending time review the changes.
>
> And I just want to present how to reproduce the build error.
>
> When build OvmfPkgX64, you can encounter this issue with your local change. 
> The error as below:
> TcgPei.lib(AutoGen.obj) : error LNK2001: unresolved external symbol _fltused
> c:\sourcecodes\tiano\Build\OvmfX64\DEBUG_VS2019\X64\SecurityPkg\Tcg\TcgPei\TcgPei\DEBUG\TcgPei.dll
>  : fatal error LNK1120: 1 unresolved externals
>
> The build command: build -p OvmfPkg\OvmfPkgX64.dsc -b DEBUG -t VS2019 -a X64 
> -D TPM_ENABLE
> The code changes for reproducing this symptom:
> -  int  GLOBAL_USED _fltused = 1;
> + //int  GLOBAL_USED _fltused = 1;
> The machine: WIN10
> The branch: edk2 master
>

Doesn't the build error go away as well if you simply add FloatUsed.c
to all BaseCryptLib flavours if Visual Studio is being used?

Something like

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 1bbe4f435aec..f40bf18e7f5d 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -27,6 +27,7 @@ [Defines]
 #

 [Sources]
+  FloatUsed.c | MSFT
   InternalCryptLib.h
   Hash/CryptMd4.c
   Hash/CryptMd5.c

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

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

[edk2-devel] [PATCH] UnitTestFrameworkPkg/UnitTestLib: Correct dereferred pointer.

[Guomin Jiang]

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

The copied pointer (SavedState) will be updated by LoadUnitTestCache
call. But the change of SavedState will not update source pointer, which
is NewFramework->SavedState in this case.

Cc: Michael D Kinney 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Signed-off-by: Guomin Jiang 
---
 UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c 
b/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c
index b136992d99..71050b5618 100644
--- a/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c
+++ b/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c
@@ -209,7 +209,7 @@ InitUnitTestFramework (
   EFI_STATUS  Status;
   UNIT_TEST_FRAMEWORK_HANDLE  NewFrameworkHandle;
   UNIT_TEST_FRAMEWORK *NewFramework;
-  UNIT_TEST_SAVE_HEADER   *SavedState;
+  UNIT_TEST_SAVE_HEADER   **SavedState;
 
   Status   = EFI_SUCCESS;
   NewFramework = NULL;
@@ -264,8 +264,8 @@ InitUnitTestFramework (
   // If there is a persisted context, load it now.
   //
   if (DoesCacheExist (NewFrameworkHandle)) {
-SavedState = (UNIT_TEST_SAVE_HEADER *)NewFramework->SavedState;
-Status = LoadUnitTestCache (NewFrameworkHandle, );
+SavedState = (UNIT_TEST_SAVE_HEADER **)(>SavedState);
+Status = LoadUnitTestCache (NewFrameworkHandle, SavedState);
 if (EFI_ERROR (Status)) {
   //
   // Don't actually report it as an error, but emit a warning.
-- 
2.25.1.windows.1


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

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

Re: [edk2-devel] [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

[Ard Biesheuvel]

On Tue, 31 Mar 2020 at 08:31, Sean via Groups.Io
 wrote:
>
> @Ard -
> pflash change: https://github.com/spbrogan/edk2/pull/12
>
> Logging change - I actually switched OVMF to use stdio since the log is 
> captured either way and now it shows up in the web log output.  
> https://github.com/spbrogan/edk2/pull/13
>

Even better.

> Do you have instructions for the cmdline for Qemu for Arm?  Is it something 
> you would like to run?
>

Not sure I follow. Which command line are we talking about?

> All the builds can be seen here or clicking the icon in github:
> https://dev.azure.com/tianocore/edk2-ci-play/_build?view=folders=XEFybVZpcnRQa2ckXEVtdWxhdG9yUGtnJFxPVk1G

Thanks again for the effort. This is going to help tremendously.

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

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

Re: [edk2-devel] [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

[Sean via Groups.Io]

@Ard -
pflash change: https://github.com/spbrogan/edk2/pull/12

Logging change - I actually switched OVMF to use stdio since the log is 
captured either way and now it shows up in the web log output. 
https://github.com/spbrogan/edk2/pull/13

Do you have instructions for the cmdline for Qemu for Arm?  Is it something you 
would like to run?

All the builds can be seen here or clicking the icon in github:
https://dev.azure.com/tianocore/edk2-ci-play/_build?view=folders=XEFybVZpcnRQa2ckXEVtdWxhdG9yUGtnJFxPVk1G

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

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