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

[Nate DeSimone]

Erik,

Please remember to Cc: the reviewers on the cover letter as well next time.

For the series...
Reviewed-by: Nate DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 0/7] Adding support for 
archiving branch combos

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 (#56884): https://edk2.groups.io/g/devel/message/56884
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] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Improve state tracking when checking out pin files

[Nate DeSimone]

Reviewed-by: Nate DeSimone 

-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 (#56883): https://edk2.groups.io/g/devel/message/56883
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] Query about VS2012

[Zhang, Shenglei]

Hi All,

We have a build failure with 
VS2012(https://bugzilla.tianocore.org/show_bug.cgi?id=2631).
If no body uses VS2012, I would like to ignore this issue. I send this mail to 
collect information whether you are using VS2012.

Thanks,
Shenglei

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

View/Reply Online (#56882): https://edk2.groups.io/g/devel/message/56882
Mute This Topic: https://groups.io/mt/72716358/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/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()

[Siyuan, Fu]

With Laszlo's comments.
Reviewed-by: Siyuan Fu 

> -Original Message-
> From: Laszlo Ersek 
> Sent: 2020年4月1日 20:02
> To: Maciej Rabeda ; devel@edk2.groups.io
> Cc: Wu, Jiaxin ; Fu, Siyuan 
> Subject: Re: [PATCH] NetworkPkg/Ip6Dxe: Fix ASSERT logic in
> Ip6ProcessRouterAdvertise()
> 
> Hi Maciej,
> 
> On 04/01/20 11:53, Maciej Rabeda wrote:
> > This patch fixes reversed logic of recently added ASSERTs which should
> > ensure that Ip6IsNDOptionValid() implementation properly reacts to
> invalid
> > packets.
> >
> > Cc: Jiaxin Wu 
> > Cc: Siyuan Fu 
> > Signed-off-by: Maciej Rabeda 
> > Tested-by: Laszlo Ersek 
> > ---
> >  NetworkPkg/Ip6Dxe/Ip6Nd.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Thanks for the patch. Two meta-suggestions:
> 
> (1) we should do one of the following:
> 
> (1a) Create a new BZ for this issue, cross-link it -- via the See Also
> field -- with TianoCore#2174, and reference the new BZ in this commit
> message. If we file this new BZ, it should get the Regression keyword.
> 
> (1b) Or else we should reopen TianoCore#2174, and reference *that* BZ in
> this commit message.
> 
> (2) Independently of (1), the commit message should carry the following tag:
> 
> Fixes: 9c20342eed70ec99ec50cd73cb81804299f05403
> 
> Regarding this patch, the above updates only affect the commit message,
> so I'm not asking for a v2 -- you can implement the commit message
> changes right before pushing.
> 
> Thanks!
> Laszlo
> 
> 
> 
> > 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
> >


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

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

Re: [edk2-devel] [PATCH v6 00/42] SEV-ES guest support

[Dong, Eric]

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Lendacky, Thomas
> Sent: Thursday, April 2, 2020 4:42 AM
> To: Dong, Eric ; devel@edk2.groups.io
> Cc: Justen, Jordan L ; Laszlo Ersek
> ; Ard Biesheuvel ; Kinney,
> Michael D ; Gao, Liming
> ; Ni, Ray ; Brijesh Singh
> ; You, Benjamin ; Bi,
> Dandan ; Dong, Guo ; Wu,
> Hao A ; Wang, Jian J ; Ma,
> Maurice 
> Subject: Re: [edk2-devel] [PATCH v6 00/42] SEV-ES guest support
> 
> On 3/30/20 7:47 PM, Dong, Eric wrote:
> > Hi Tom,
> >
> > Sorry for late response. It’s a huge patch, please give me two more
> > weeks to detail review them.
> >
> > I have rough go through these patches and have some basic comments for
> > them now:
> >
> > 1.It’s better to spit patch if changes files not in same package. Like
> > patch 1/42.
> 
> Ok, will do.
> 
> >
> > 2.All functions need to have comments for them. Miss comments in patch
> > 10/42 and others.
> 
> Just external functions or both external and internal (STATIC) functions, too?

All the functions.

Thanks,
Eric

> 
> Thanks,
> Tom
> 
> >
> > Please update patches to fix above basic checks first.
> >
> > Thanks,
> >
> > Eric
> >
> > *From:*devel@edk2.groups.io [mailto:devel@edk2.groups.io] *On Behalf
> > Of *Lendacky, Thomas
> > *Sent:* Tuesday, March 31, 2020 12:54 AM
> > *To:* devel@edk2.groups.io
> > *Cc:* Justen, Jordan L ; Laszlo Ersek
> > ; Ard Biesheuvel ;
> > Kinney, Michael D ; Gao, Liming
> > ; Dong, Eric ; Ni, Ray
> > ; Brijesh Singh ; You,
> > Benjamin ; Bi, Dandan ;
> > Dong, Guo ; Wu, Hao A ;
> Wang,
> > Jian J ; Ma, Maurice 
> > *Subject:* Re: [edk2-devel] [PATCH v6 00/42] SEV-ES guest support
> >
> > I've gotten some nice feedback from Laszlo, especially on the OvmfPkg
> > side of this patchset, but haven't seen much response from the other
> > maintainers. Is there any feedback on the MdePkg, MdeModulePkg and
> > UefiCpuPkg changes that needs to be addressed in order to merge this?
> >
> > I do have some minor changes on ensuring the per-CPU variable page
> > stays encrypted, but not much beyond that. Those changes can be
> > submitted afterwards or as a new version before inclusion.
> >
> > Thanks,
> > Tom
> >
> > On 3/24/20 12:40 PM, Tom Lendacky wrote:
> >> This patch series provides support for running EDK2/OVMF under SEV-ES.
> >>
> >> Secure Encrypted Virtualization - Encrypted State (SEV-ES) expands on
> >> the SEV support to protect the guest register state from the
> >> hypervisor. See
> >> "AMD64 Architecture Programmer's Manual Volume 2: System
> >> Programming", section "15.35 Encrypted State (SEV-ES)" [1].
> >>
> >> In order to allow a hypervisor to perform functions on behalf of a
> >> guest, there is architectural support for notifying a guest's
> >> operating system when certain types of VMEXITs are about to occur.
> >> This allows the guest to selectively share information with the
> >> hypervisor to satisfy the requested function. The notification is
> >> performed using a new exception, the VMM Communication exception
> >> (#VC). The information is shared through the Guest-Hypervisor
> Communication Block (GHCB) using the VMGEXIT instruction.
> >> The GHCB format and the protocol for using it is documented in
> >> "SEV-ES Guest-Hypervisor Communication Block Standardization" [2].
> >>
> >> The main areas of the EDK2 code that are updated to support SEV-ES
> >> are around the exception handling support and the AP boot support.
> >>
> >> Exception support is required starting in Sec, continuing through Pei
> >> and into Dxe in order to handle #VC exceptions that are generated.
> >> Each AP requires it's own GHCB page as well as a page to hold values
> >> specific to that AP.
> >>
> >> AP booting poses some interesting challenges. The INIT-SIPI-SIPI
> >> sequence is typically used to boot the APs. However, the hypervisor
> >> is not allowed to update the guest registers. The GHCB document [2]
> >> talks about how SMP booting under SEV-ES is performed.
> >>
> >> Since the GHCB page must be a shared (unencrypted) page, the
> >> processor must be running in long mode in order for the guest and
> >> hypervisor to communicate with each other. As a result, SEV-ES is
> >> only supported under the X64 architecture.
> >>
> >> [1] https://www.amd.com/system/files/TechDocs/24593.pdf
> >
>  ww
> > .amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf=02%7C01%
> 7Cthomas
> > .lendacky%40amd.com%7C2ee33c1d932a4906558f08d7d50d1ca2%7C3dd89
> 61fe4884
> >
> e608e11a82d994e183d%7C0%7C0%7C637212125835211690=Q%2BIjeq
> %2FRDgi
> > ovKtPeA4TGDVorCK07jQVNZ7N9kvD%2BuE%3D=0>
> >> [2] https://developer.amd.com/wp-content/resources/56421.pdf
> >
>  v
> > eloper.amd.com%2Fwp-
> content%2Fresources%2F56421.pdf=02%7C01%7Ctho
> >
> mas.lendacky%40amd.com%7C2ee33c1d932a4906558f08d7d50d1ca2%7C3dd
> 8961fe4
> >
> 

[edk2-devel] [edk2-staging/EdkRepo] [PATCH V1 0/3] EdkRepo: Command completion in bash/zsh

[Nate DeSimone]

This patch series adds command completions (also referred to as
[TAB] completions) to EdkRepo. It also adds the currently
checked out branch combination to the command prompt. This is a
significant quality of life improvement for EdkRepo's users.

The bash and zsh shells are supported. The reason why bash and
zsh were choosen is based on their popularity in the UNIX world.
zsh is the default shell on macOS and most contemporary Linux
distributions use bash as the default shell. Git for Windows
also uses bash.

Performance was a strong consideration in the design of this
feature. Since EdkRepo has become a fairly large amount of
Python code with lots of dependencies, the "boot time" for
the Python interperter to load and execute the EdkRepo
entrypoint is approximately 1-2 seconds. A 2 second delay
on TAB completion would not be a good user experience.
To mitigate this, instead of enumerating and instantiating
all the command classes to generate the list of EdkRepo
sub-commands every time the user presses the TAB
key, this is done once and stored in a auto-generated shell
script at installation time. This way, the shell has all the
information needed for static completions without having
to invoke the Python interperter at all.

There are cases where TAB completions need some dynamic data.
For example, completing the 3rd parameter after
"edkrepo checkout " requires the shell to know the list of
branch combinations, this requires parsing the manifest XML.
command_completion_edkrepo.py provides a means for the shell
to get that type of data. A new command_completion_edkrepo.py
script has been added for this purpose.
command_completion_edkrepo.py is callable by the shell when
dynamic completion data is needed.
command_completion_edkrepo.py has been tuned to limit imports
as much as possible so that it executes quickly.

command_completion_edkrepo.py is also used to retrieve the
currently checked out branch combination so that it can be
added to the prompt. For performance considerations,
this operation is only done iff the present working directory
was changed by the last command the shell executed.

Cc: Ashley DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Prince Agyeman 
Cc: Bret Barkelew 
Cc: Philippe Mathieu-Daude 
Signed-off-by: Nate DeSimone 

Nate DeSimone (3):
  EdkRepo: Generate command completion scripts
  EdkRepo: Add command completion setup to install.py
  EdkRepo: Add command completion setup to Windows installer

 edkrepo/command_completion_edkrepo.py |  86 ++
 edkrepo/edkrepo_cli.py|  61 +++-
 .../EdkRepoInstaller/InstallWorker.cs | 212 +++--
 .../EdkRepoInstaller/InstallerStrings.cs  |  44 ++-
 .../Vendor/win_edkrepo_prompt.sh  |  60 
 edkrepo_installer/linux-scripts/install.py| 285 +-
 setup.py  |   8 +-
 7 files changed, 729 insertions(+), 27 deletions(-)
 create mode 100644 edkrepo/command_completion_edkrepo.py
 create mode 100644 edkrepo_installer/Vendor/win_edkrepo_prompt.sh

-- 
2.24.0.windows.2


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

View/Reply Online (#56876): https://edk2.groups.io/g/devel/message/56876
Mute This Topic: https://groups.io/mt/72713332/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/3] EdkRepo: Generate command completion scripts

[Nate DeSimone]

Adds code to edkrepo_cli.py to generate a bash/zsh
compatible command completion script.

Add a new command_completion_edkrepo.py script which
is callable by the shell when needed for dynamic
completion data. For example, providing completion for
"edkrepo checkout" requires the shell to know the list
of possible branch combinations, this requires parsing
the manifest XML. command_completion_edkrepo.py provides
a means for the shell to get that type of data.

Cc: Ashley DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Prince Agyeman 
Cc: Bret Barkelew 
Cc: Philippe Mathieu-Daude 
Signed-off-by: Nate DeSimone 
---
 edkrepo/command_completion_edkrepo.py | 86 +++
 edkrepo/edkrepo_cli.py| 61 ++-
 setup.py  |  8 +--
 3 files changed, 150 insertions(+), 5 deletions(-)
 create mode 100644 edkrepo/command_completion_edkrepo.py

diff --git a/edkrepo/command_completion_edkrepo.py 
b/edkrepo/command_completion_edkrepo.py
new file mode 100644
index 000..05de9ca
--- /dev/null
+++ b/edkrepo/command_completion_edkrepo.py
@@ -0,0 +1,86 @@
+#!/usr/bin/env python3
+#
+## @file
+# command_completion_edkrepo.py
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+import argparse
+import io
+import os
+import sys
+import traceback
+
+from edkrepo_manifest_parser.edk_manifest import ManifestXml
+from edkrepo.config import config_factory
+from edkrepo.config.config_factory import get_workspace_manifest
+
+def checkout(parsed_args, config):
+manifest = get_workspace_manifest()
+print(' '.join([c.name for c in manifest.combinations]))
+
+def current_combo(parsed_args, config):
+manifest = get_workspace_manifest()
+print(" [{}]".format(manifest.general_config.current_combo))
+
+def checkout_pin(parsed_args, config):
+pins = []
+manifest_directory = config['cfg_file'].manifest_repo_abs_local_path
+manifest = get_workspace_manifest()
+pin_folder = os.path.normpath(os.path.join(manifest_directory, 
manifest.general_config.pin_path))
+for dirpath, _, filenames in os.walk(pin_folder):
+for file in filenames:
+pin_file = os.path.join(dirpath, file)
+# Capture error output from manifest parser stdout so it is hidden 
unless verbose is enabled
+stdout = sys.stdout
+sys.stdout = io.StringIO()
+pin = ManifestXml(pin_file)
+parse_output = sys.stdout.getvalue()
+sys.stdout = stdout
+if parsed_args.verbose and parse_output.strip() != '':
+print('Pin {} Parsing Errors: {}\n'.format(file, 
parse_output.strip()))
+if pin.project_info.codename == manifest.project_info.codename:
+pins.append(file)
+print(' '.join(pins))
+
+# To add command completions for a new command, add an entry to this 
dictionary.
+command_completions = {
+'current-combo': current_combo,
+'checkout': checkout,
+'checkout-pin': checkout_pin,
+'chp': checkout_pin
+}
+
+def main():
+parser = argparse.ArgumentParser()
+parser.add_argument("-v", "--verbose", action="store_true", 
help='Increases command verbosity')
+subparsers = parser.add_subparsers(dest='subparser_name')
+for command_completion in command_completions:
+subparsers.add_parser(command_completion, 
formatter_class=argparse.RawTextHelpFormatter)
+if len(sys.argv) <= 1:
+return 0
+parsed_args = parser.parse_args()
+try:
+command_name = parsed_args.subparser_name
+config = {}
+config["cfg_file"] = config_factory.GlobalConfig()
+config["user_cfg_file"] = config_factory.GlobalUserConfig()
+if command_name not in command_completions:
+return 1
+command_completions[command_name](parsed_args, config)
+return 0
+except Exception as e:
+if parsed_args.verbose:
+traceback.print_exc()
+print("Error: {}".format(str(e)))
+return 1
+return 0
+
+if __name__ == "__main__":
+try:
+sys.exit(main())
+except Exception as e:
+traceback.print_exc()
+sys.exit(1)
diff --git a/edkrepo/edkrepo_cli.py b/edkrepo/edkrepo_cli.py
index 0b69860..03061c9 100644
--- a/edkrepo/edkrepo_cli.py
+++ b/edkrepo/edkrepo_cli.py
@@ -3,7 +3,7 @@
 ## @file
 # edkrepo_cli.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
 #
 
@@ -29,6 +29,7 @@ from edkrepo.common.edkrepo_exception import 
EdkrepoException, EdkrepoGlobalConf
 from edkrepo.common.edkrepo_exception import EdkrepoWarningException
 from edkrepo.common.edkrepo_exception import EdkrepoConfigFileInvalidException
 from edkrepo.common.humble import KEYBOARD_INTERRUPT, GIT_CMD_ERROR
+from edkrepo.common.pathfix import 

[edk2-devel] [edk2-staging/EdkRepo] [PATCH V1 3/3] EdkRepo: Add command completion setup to Windows installer

[Nate DeSimone]

Add configuration of command completion scripts
to the Windows installer. This enables edkrepo command
completions to work "out of box" on Git for Windows by
adding the edkrepo command completions scripts to
the Git for Windows /etc/profile.d directory

Cc: Ashley DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Prince Agyeman 
Cc: Bret Barkelew 
Cc: Philippe Mathieu-Daude 
Signed-off-by: Nate DeSimone 
---
 .../EdkRepoInstaller/InstallWorker.cs | 212 --
 .../EdkRepoInstaller/InstallerStrings.cs  |  44 +++-
 .../Vendor/win_edkrepo_prompt.sh  |  60 +
 3 files changed, 296 insertions(+), 20 deletions(-)
 create mode 100644 edkrepo_installer/Vendor/win_edkrepo_prompt.sh

diff --git a/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs 
b/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs
index 472a6c8..679b4f4 100644
--- a/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs
+++ b/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs
@@ -19,6 +19,7 @@ using System.Security.AccessControl;
 using System.Security.Cryptography;
 using System.Security.Principal;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading;
 
 namespace TianoCore.EdkRepoInstaller
@@ -611,7 +612,7 @@ namespace TianoCore.EdkRepoInstaller
 SilentProcess process = 
SilentProcess.StartConsoleProcessSilently(GitPath, "--version", 
dataCapture.DataReceivedHandler);
 process.WaitForExit();
 PythonVersion gitVersion = new 
PythonVersion(dataCapture.GetData().Trim());
-if (gitVersion < new PythonVersion(2,13,0))
+if (gitVersion < new PythonVersion(2, 13, 0))
 {
 InstallLogger.Log(string.Format("Git Version 2.13 or later is 
required. You have version {0}, please upgrade to a newer version of Git.", 
gitVersion));
 ReportComplete(false, false);
@@ -624,7 +625,7 @@ namespace TianoCore.EdkRepoInstaller
 List> ExclusivePackages = new 
List>();
 foreach (PythonInstance PyInstance in PythonWheelsToInstall)
 {
-foreach(PythonWheel Wheel in PyInstance.Wheels)
+foreach (PythonWheel Wheel in PyInstance.Wheels)
 {
 if (Wheel.UninstallAllOtherCopies)
 {
@@ -668,13 +669,13 @@ namespace TianoCore.EdkRepoInstaller
 //
 foreach (PythonVersion Obsolete in ObsoletedPythonVersions)
 {
-if(ExistingEdkRepoPaths.Select(p => 
p.Item2).Contains(Obsolete))
+if (ExistingEdkRepoPaths.Select(p => 
p.Item2).Contains(Obsolete))
 {
 foreach (string ExistingEdkrepoPythonPath in 
ExistingEdkRepoPaths.Where(p => p.Item2 == Obsolete).Select(p => p.Item1))
 {
 string UninstallerPath = 
Path.Combine(Path.GetDirectoryName(ExistingEdkrepoPythonPath), "Lib", 
"site-packages");
 UninstallerPath = Path.Combine(UninstallerPath, 
Path.GetFileName(WindowsHelpers.GetApplicationPath()));
-if(File.Exists(UninstallerPath))
+if (File.Exists(UninstallerPath))
 {
 InstallLogger.Log(string.Format("Uninstalling 
{0}...", UninstallerPath));
 string UninstallString = string.Format("\"{0}\" 
/Uninstall /Passive", UninstallerPath);
@@ -788,14 +789,15 @@ namespace TianoCore.EdkRepoInstaller
 //
 // Step 10 - Setup symlink to edkrepo and bash script to launch 
edkrepo from git bash
 //
+string EdkrepoSymlinkPath = null;
 if (!string.IsNullOrEmpty(EdkrepoPythonPath))
 {
 string EdkrepoScriptPath = 
Path.Combine(Path.GetDirectoryName(EdkrepoPythonPath), "Scripts", 
InstallerStrings.EdkrepoCliExecutable);
-string EdkrepoSymlinkPath = 
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Windows), 
InstallerStrings.EdkrepoCliExecutable);
+EdkrepoSymlinkPath = 
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Windows), 
InstallerStrings.EdkrepoCliExecutable);
 if (File.Exists(EdkrepoScriptPath))
 {
 bool CreateSymlink = true;
-if(File.Exists(EdkrepoSymlinkPath))
+if (File.Exists(EdkrepoSymlinkPath))
 {
 try
 {
@@ -805,22 +807,22 @@ namespace TianoCore.EdkRepoInstaller
 }
 }
 catch (NotASymlinkException) { }
-if(CreateSymlink)
+if (CreateSymlink)
 {
 File.Delete(EdkrepoSymlinkPath);
 }
 }
-   

[edk2-devel] [edk2-staging/EdkRepo] [PATCH V1 2/3] EdkRepo: Add command completion setup to install.py

[Nate DeSimone]

Add configuration of command completion scripts
to install.py This enables edkrepo command completions
to work "out of box" on most Linux systems by appending
to the user's .bashrc and .zshrc startup scripts
inclusion of the EdkRepo command completion scripts.

Cc: Ashley DeSimone 
Cc: Puja Pandya 
Cc: Erik Bjorge 
Cc: Prince Agyeman 
Cc: Bret Barkelew 
Cc: Philippe Mathieu-Daude 
Signed-off-by: Nate DeSimone 
---
 edkrepo_installer/linux-scripts/install.py | 285 -
 1 file changed, 283 insertions(+), 2 deletions(-)

diff --git a/edkrepo_installer/linux-scripts/install.py 
b/edkrepo_installer/linux-scripts/install.py
index b2cdfed..52f0c52 100755
--- a/edkrepo_installer/linux-scripts/install.py
+++ b/edkrepo_installer/linux-scripts/install.py
@@ -15,6 +15,7 @@ import importlib.util
 import logging
 import os
 import platform
+import re
 import stat
 import shutil
 import subprocess
@@ -22,7 +23,7 @@ import sys
 import traceback
 import xml.etree.ElementTree as et
 
-tool_sign_on = 'Installer for edkrepo version {}\nCopyright(c) Intel 
Corporation, 2019'
+tool_sign_on = 'Installer for edkrepo version {}\nCopyright(c) Intel 
Corporation, 2020'
 
 # Data here should be maintained in a configuration file
 cfg_dir = '.edkrepo'
@@ -31,6 +32,21 @@ cfg_src_dir = os.path.abspath('config')
 whl_src_dir = os.path.abspath('wheels')
 def_python = 'python3'
 
+# ZSH Configuration options
+prompt_regex = 
re.compile(r"#\s+[Aa][Dd][Dd]\s+[Ee][Dd][Kk][Rr][Ee][Pp][Oo]\s+&\s+[Gg][Ii][Tt]\s+[Tt][Oo]\s+[Tt][Hh][Ee]\s+[Pp][Rr][Oo][Mm][Pp][Tt]")
+zsh_autoload_compinit_regex = re.compile(r"autoload\s+-U\s+compinit")
+zsh_autoload_bashcompinit_regex = re.compile(r"autoload\s+-U\s+bashcompinit")
+zsh_autoload_colors_regex = re.compile(r"autoload\s+-U\s+colors")
+zsh_colors_regex = re.compile(r"\n\s*colors\n")
+zsh_compinit_regex = re.compile(r"compinit\s+-u")
+zsh_bashcompinit_regex = re.compile(r"\n\s*bashcompinit\n")
+zsh_autoload_compinit = 'autoload -U compinit'
+zsh_autoload_bashcompinit = 'autoload -U bashcompinit'
+zsh_autoload_colors = 'autoload -U colors'
+zsh_colors = 'colors'
+zsh_compinit = 'compinit -u'
+zsh_bashcompinit = 'bashcompinit'
+
 def default_run(cmd):
 return subprocess.run(cmd, universal_newlines=True, 
stdout=subprocess.PIPE, stderr=subprocess.STDOUT, check=True)
 
@@ -51,8 +67,85 @@ def get_args():
 parser.add_argument('-p', '--py', action='store', default=None, 
help='Specify the python command to use when installing')
 parser.add_argument('-u', '--user', action='store', default=None, 
help='Specify user account to install edkrepo support on')
 parser.add_argument('-v', '--verbose', action='store_true', default=False, 
help='Enables verbose output')
+parser.add_argument('--no-prompt', action='store_true', default=False, 
help='Do NOT add EdkRepo combo and git branch to the shell prompt')
+parser.add_argument('--prompt', action='store_true', default=False, 
help='Add EdkRepo combo and git branch to the shell prompt')
 return parser.parse_args()
 
+def is_prompt_customization_installed(user_home_dir):
+script_files = [os.path.join(user_home_dir, '.bashrc'), 
os.path.join(user_home_dir, '.zshrc')]
+customization_installed = True
+for script_file in script_files:
+if os.path.isfile(script_file):
+with open(script_file, 'r') as f:
+script = f.read().strip()
+data = prompt_regex.search(script)
+if not data:
+customization_installed = False
+break
+return customization_installed
+
+__add_prompt_customization = None
+def get_add_prompt_customization(args, user_home_dir):
+global __add_prompt_customization
+if __add_prompt_customization is not None:
+return __add_prompt_customization
+if args.no_prompt:
+__add_prompt_customization = False
+return False
+elif args.prompt:
+__add_prompt_customization = True
+return True
+#Check if the prompt customization has already been installed
+if is_prompt_customization_installed(user_home_dir):
+__add_prompt_customization = False
+return False
+#If the prompt has not been installed and EdkRepo >= 2.0.0 is installed, 
then don't install the prompt customization
+if shutil.which('edkrepo') is not None:
+res = default_run(['edkrepo', '--version'])
+if _check_version(res.stdout.replace('edkrepo ', '').strip(), '2.0.0') 
>= 0:
+__add_prompt_customization = False
+return False
+#Show the user an advertisement to see if they want the prompt 
customization
+from select import select
+import termios
+import tty
+def get_key(timeout=-1):
+key = None
+old_settings = termios.tcgetattr(sys.stdin.fileno())
+try:
+tty.setraw(sys.stdin.fileno())
+if timeout != -1:
+rlist, _, _ = select([sys.stdin], [], [], timeout)
+if rlist:
+   

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 7/7] EdkRepo: Update 
List Repos for archived combos

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 

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 6/7] EdkRepo: Update 
clone for archived combos

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 (#56874): https://edk2.groups.io/g/devel/message/56874
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 5/7] EdkRepo: Update 
Checkout Pin for archived combos

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 (#56873): https://edk2.groups.io/g/devel/message/56873
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 4/7] EdkRepo: Update 
Sync for archived combos

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 (#56872): https://edk2.groups.io/g/devel/message/56872
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 3/7] EdkRepo: Update 
Checkout for archived combos

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 (#56871): https://edk2.groups.io/g/devel/message/56871
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 2/7] EdkRepo: Added 
ability to display archived combinations

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 (#56870): https://edk2.groups.io/g/devel/message/56870
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Desimone, Ashley E]

Reviewed-by: Ashley DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bjorge, Erik C
Sent: Tuesday, March 31, 2020 3:42 PM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Pandya, Puja 
; Bret Barkelew ; Agyeman, 
Prince 
Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH v1 1/7] EdkRepo: Adding 
support for archiving combos

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 (#56869): https://edk2.groups.io/g/devel/message/56869
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 00/28] Add PEI phase to LS1043ARDB Platform

[Leif Lindholm]

Hi Pankaj,

I have now finished my review of individual patches.

Beyond that, the build fails at the patch
10/28 "Silicon/NXP: remove print information from Soc lib" with the error
edk2-platforms/Silicon/NXP/Library/SocLib/Chassis.c:209:1: error:
‘CpuMaskNext’ defined but not used [-Werror=unused-function]

The function is deleted in the subsequent patch,
11/28 "Silicon/NXP: remove not needed components".
Please move that deletion to the now failing patch.

Regards,

Leif

On Fri, Mar 20, 2020 at 20:05:15 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> This patch series adds PEI phase to NXP LS1043ARDB Platform.
> The previous attempt at this feature can be referred here:
> https://edk2.groups.io/g/devel/message/54006
> 
> I have taken care of the review comments received on v1 and have
> broken down the patches further to make review easier.
> 
> That is why the number of patches have increased from 19 in v1 to
> 28 in v2.
> 
> As such the v1 and v2 patches have diverged, which is why i am not
> putting version specific changes in each indivisual patch.
> 
> i have created v2 series in a way that the changes feel more organic
> and not abrupt.
> Only the patch "12/28 remove not needed components" would seem too
> invasive. But, as i have noted in patch description, i am not removing
> anything which is needed for booting LS1043ARDB as of now. i have done
> this to keep the code simple and introduce the components as and when
> needed for new features. This makes code review simpler too.
> 
> Pankaj Bansal (28):
>   Silicon/NXP: Add I2c lib
>   Silicon/NXP: changes to use I2clib in i2cdxe
>   Silicon/NXP/I2cDxe: Fix I2c Timeout with RTC
>   Silicon/Maxim: Fix bug in RtcWrite in Ds1307RtcLib
>   Silicon/Maxim: Add comments in Ds1307RtcLib
>   NXP/LS1043aRdb: Move Soc specific components to soc files
>   Silicon/NXP: Implement SerialUartClockLib
>   Silicon/NXP/LS1043A: Use BaseSerialPortLib16550 as SerialPortLib
>   Silicon/NXP: Drop DUartPortLib
>   Silicon/NXP: remove print information from Soc lib
>   Silicon/NXP: remove not needed components
>   Silicon/NXP: Remove unnecessary PCDs
>   Silicon/NXP: Move dsc file
>   Platform/NXP: rename the ArmPlatformLib as per ArmPlatformPkg
>   Silicon/NXP: Move RAM retrieval from SocLib
>   Platform/NXP/LS1043aRdbPkg: Add Clock retrieval APIs
>   Silicon/NXP: Use Clock retrieval PPI in modules
>   Silicon/NXP: Add Chassis2 Package
>   Silicon/NXP/LS1043A: Use ChassisLib from Chassis2 Pkg
>   Silicon/NXP/LS1043A: Move SocLib to Soc Package
>   Slicon/NXP: Add PlatformPei Lib
>   NXP/LS1043aRdbPkg/ArmPlatformLib: Use default ArmPlatformHelper.S
>   NXP/LS1043aRdbPkg/ArmPlatformLib: Use Allocate pool
>   NXP/LS1043aRdbPkg/ArmPlatformLib: Remove extern SocInit
>   Platform/NXP: Modify FV rules
>   Platform/NXP/LS1043aRdbPkg: Add VarStore
>   Silicon/NXP: move MemoryInitPeiLib as per PEIM structures
>   Platform/NXP/LS1043aRdbPkg: Add PEI Phase
> 
>  Platform/NXP/FVRules.fdf.inc  |  59 +-
>  .../Drivers/PlatformDxe/PlatformDxe.c |  15 +-
>  .../Drivers/PlatformDxe/PlatformDxe.inf   |  11 +-
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc  |  26 +-
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf  |  21 +-
>  .../AArch64/ArmPlatformHelper.S   |  45 ++
>  .../ArmPlatformLib.c  |  61 +-
>  .../Library/ArmPlatformLib/ArmPlatformLib.inf |  42 ++
>  .../ArmPlatformLibMem.c}  |  84 ++-
>  .../Library/PlatformLib/ArmPlatformLib.inf|  55 --
>  .../Library/PlatformLib/NxpQoriqLsHelper.S|  31 -
>  Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc   |  91 +++
>  .../Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c |  23 +-
>  Silicon/NXP/Chassis2/Chassis2.dec |  23 +
>  Silicon/NXP/Chassis2/Chassis2.dsc.inc |  10 +
>  Silicon/NXP/Chassis2/Include/Chassis.h|  34 ++
>  .../Chassis2/Library/ChassisLib/ChassisLib.c  |  97 +++
>  .../Library/ChassisLib/ChassisLib.inf |  34 ++
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.c   | 533 +---
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.h   |  50 +-
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf |  14 +-
>  Silicon/NXP/Include/Chassis2/LsSerDes.h   |  62 --
>  Silicon/NXP/Include/Chassis2/NxpSoc.h | 361 ---
>  Silicon/NXP/Include/DramInfo.h|  38 --
>  Silicon/NXP/Include/Library/ChassisLib.h  |  51 ++
>  Silicon/NXP/Include/Library/I2cLib.h  | 120 
>  Silicon/NXP/Include/Library/SocLib.h  |  52 ++
>  Silicon/NXP/Include/Ppi/NxpPlatformGetClock.h |  53 ++
>  Silicon/NXP/LS1043A/Include/Soc.h |  55 ++
>  Silicon/NXP/LS1043A/Include/SocSerDes.h   |  51 --
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc   |  51 +-
>  Silicon/NXP/LS1043A/Library/SocLib/SocLib.c   |  77 +++
>  Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf |  27 +
>  Silicon/NXP/Library/DUartPortLib/DUart.h  | 122 
>  .../NXP/Library/DUartPortLib/DUartPortLib.c   | 364 

Re: [edk2-devel] [PATCH v6 00/42] SEV-ES guest support

[Lendacky, Thomas]

On 3/30/20 7:47 PM, Dong, Eric wrote:

Hi Tom,

Sorry for late response. It’s a huge patch, please give me two more weeks 
to detail review them.


I have rough go through these patches and have some basic comments for 
them now:


1.It’s better to spit patch if changes files not in same package. Like 
patch 1/42.


Ok, will do.



2.All functions need to have comments for them. Miss comments in patch 
10/42 and others.


Just external functions or both external and internal (STATIC) functions, too?

Thanks,
Tom



Please update patches to fix above basic checks first.

Thanks,

Eric

*From:*devel@edk2.groups.io [mailto:devel@edk2.groups.io] *On Behalf Of 
*Lendacky, Thomas

*Sent:* Tuesday, March 31, 2020 12:54 AM
*To:* devel@edk2.groups.io
*Cc:* Justen, Jordan L ; Laszlo Ersek 
; Ard Biesheuvel ; Kinney, 
Michael D ; Gao, Liming 
; Dong, Eric ; Ni, Ray 
; Brijesh Singh ; You, Benjamin 
; Bi, Dandan ; Dong, Guo 
; Wu, Hao A ; Wang, Jian J 
; Ma, Maurice 

*Subject:* Re: [edk2-devel] [PATCH v6 00/42] SEV-ES guest support

I've gotten some nice feedback from Laszlo, especially on the OvmfPkg side
of this patchset, but haven't seen much response from the other
maintainers. Is there any feedback on the MdePkg, MdeModulePkg and
UefiCpuPkg changes that needs to be addressed in order to merge this?

I do have some minor changes on ensuring the per-CPU variable page stays
encrypted, but not much beyond that. Those changes can be submitted
afterwards or as a new version before inclusion.

Thanks,
Tom

On 3/24/20 12:40 PM, Tom Lendacky wrote:

 This patch series provides support for running EDK2/OVMF under SEV-ES.

 Secure Encrypted Virtualization - Encrypted State (SEV-ES) expands on the
 SEV support to protect the guest register state from the hypervisor. See
 "AMD64 Architecture Programmer's Manual Volume 2: System Programming",
 section "15.35 Encrypted State (SEV-ES)" [1].

 In order to allow a hypervisor to perform functions on behalf of a guest,
 there is architectural support for notifying a guest's operating system
 when certain types of VMEXITs are about to occur. This allows the guest to
 selectively share information with the hypervisor to satisfy the requested
 function. The notification is performed using a new exception, the VMM
 Communication exception (#VC). The information is shared through the
 Guest-Hypervisor Communication Block (GHCB) using the VMGEXIT instruction.
 The GHCB format and the protocol for using it is documented in "SEV-ES
 Guest-Hypervisor Communication Block Standardization" [2].

 The main areas of the EDK2 code that are updated to support SEV-ES are
 around the exception handling support and the AP boot support.

 Exception support is required starting in Sec, continuing through Pei
 and into Dxe in order to handle #VC exceptions that are generated.  Each
 AP requires it's own GHCB page as well as a page to hold values specific
 to that AP.

 AP booting poses some interesting challenges. The INIT-SIPI-SIPI sequence
 is typically used to boot the APs. However, the hypervisor is not allowed
 to update the guest registers. The GHCB document [2] talks about how SMP
 booting under SEV-ES is performed.

 Since the GHCB page must be a shared (unencrypted) page, the processor
 must be running in long mode in order for the guest and hypervisor to
 communicate with each other. As a result, SEV-ES is only supported under
 the X64 architecture.

 [1] https://www.amd.com/system/files/TechDocs/24593.pdf 


 [2] https://developer.amd.com/wp-content/resources/56421.pdf 




 ---

 These patches are based on commit:
 2f524a745e23 ("BaseTools:Fix build tools print traceback info issue")

 Proper execution of SEV-ES relies on Bugzilla 2340 being fixed.

 A version of the tree (with an extra patch to workaround Bugzilla 2340) can
 be found at:
https://github.com/AMDESE/ovmf/tree/sev-es-v13 




 Cc: Ard Biesheuvel mailto:ard.biesheu...@linaro.org>>
 Cc: Benjamin You mailto:benjamin@intel.com>>
 Cc: Dandan Bi mailto:dandan...@intel.com>>
 Cc: Eric Dong mailto:eric.d...@intel.com>>
 Cc: Guo 

Re: [edk2-devel] [PATCH v2 27/28] Silicon/NXP: move MemoryInitPeiLib as per PEIM structures

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:42 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> MemoryInitPeiLib would be linked to MemoryInitPeim, when we implement
> PEI phase. therefore, move the library to directory of same name.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  .../{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.c | 0
>  .../{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.h | 0
>  .../{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.inf   | 0
>  Silicon/NXP/NxpQoriqLs.dsc.inc | 3 +--
>  4 files changed, 1 insertion(+), 2 deletions(-)
>  rename Silicon/NXP/Library/{MemoryInitPei => 
> MemoryInitPeiLib}/MemoryInitPeiLib.c (100%)
>  rename Silicon/NXP/Library/{MemoryInitPei => 
> MemoryInitPeiLib}/MemoryInitPeiLib.h (100%)
>  rename Silicon/NXP/Library/{MemoryInitPei => 
> MemoryInitPeiLib}/MemoryInitPeiLib.inf (100%)
> 
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c 
> b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> similarity index 100%
> rename from Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> rename to Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h 
> b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.h
> similarity index 100%
> rename from Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> rename to Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.h
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf 
> b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> similarity index 100%
> rename from Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> rename to Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
> index 5f77f47f0399..b2b10ce28a93 100644
> --- a/Silicon/NXP/NxpQoriqLs.dsc.inc
> +++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
> @@ -102,6 +102,7 @@ [LibraryClasses.common]
>PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
>  
>PlatformPeiLib|Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> +  MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
>  
>  [LibraryClasses.common.SEC]
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> @@ -113,7 +114,6 @@ [LibraryClasses.common.SEC]
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>
> MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> -  MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
>  
># 1/123 faster than Stm or Vstm version
>BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> @@ -143,7 +143,6 @@ [LibraryClasses.common.DXE_DRIVER]
>DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> -  MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -- 
> 2.17.1
> 

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

View/Reply Online (#56866): https://edk2.groups.io/g/devel/message/56866
Mute This Topic: https://groups.io/mt/72077461/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 26/28] Platform/NXP/LS1043aRdbPkg: Add VarStore

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:41 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Add VarStore Fd. This Fd is used to store non volatile variables in
> flash.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf |  3 +-
>  Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc  | 91 
>  2 files changed, 93 insertions(+), 1 deletion(-)
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf 
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> index 8d66f36d7407..99fbc87e1200 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> @@ -3,7 +3,7 @@
>  #  FLASH layout file for LS1043a board.
>  #
>  #  Copyright (c) 2016, Freescale Ltd. All rights reserved.
> -#  Copyright 2017-2019 NXP
> +#  Copyright 2017-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -49,6 +49,7 @@ [FD.LS1043ARDB_EFI]
>  FV = FVMAIN_COMPACT
>  
>  !include Platform/NXP/FVRules.fdf.inc
> +!include VarStore.fdf.inc
>  
> 
>  #
>  # FV Section
> diff --git a/Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc 
> b/Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc
> new file mode 100644
> index ..391e4ae5eaf8
> --- /dev/null
> +++ b/Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc
> @@ -0,0 +1,91 @@
> +## @file
> +#  FDF include file with FD definition that defines an empty variable store.
> +#
> +#  Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
> +#  Copyright (C) 2014, Red Hat, Inc.
> +#  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> +#  Copyright (c) 2016, Freescale Semiconductor. All rights reserved.
> +#  Copyright 2017-2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[FD.LS1043aRdbNv_EFI]
> +BaseAddress   = 0x6050  #The base address of the FLASH device
> +Size  = 0x000C  #The size in bytes of the FLASH device
> +ErasePolarity = 1
> +BlockSize = 0x4
> +NumBlocks = 0x3
> +
> +#
> +# Place NV Storage just above Platform Data Base
> +#
> +DEFINE NVRAM_AREA_VARIABLE_BASE= 0x
> +DEFINE NVRAM_AREA_VARIABLE_SIZE= 0x0004
> +DEFINE FTW_WORKING_BASE= $(NVRAM_AREA_VARIABLE_BASE) 
> + $(NVRAM_AREA_VARIABLE_SIZE)
> +DEFINE FTW_WORKING_SIZE= 0x0004
> +DEFINE FTW_SPARE_BASE  = $(FTW_WORKING_BASE) + 
> $(FTW_WORKING_SIZE)
> +DEFINE FTW_SPARE_SIZE  = 0x0004
> +
> +#
> +# LS1043ARDB NVRAM Area
> +# LS1043ARDB NVRAM Area contains: Variable + FTW Working + FTW Spare
> +#
> +
> +
> +$(NVRAM_AREA_VARIABLE_BASE)|$(NVRAM_AREA_VARIABLE_SIZE)
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +#NV_VARIABLE_STORE
> +DATA = {
> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
> +  # ZeroVector []
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # FileSystemGuid: gEfiSystemNvDataFvGuid =
> +  #   { 0xFFF12B8D, 0x7696, 0x4C8B,
> +  # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
> +  # FvLength: 0xC
> +  0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # Signature "_FVH"   # Attributes
> +  0x5f, 0x46, 0x56, 0x48, 0x36, 0x0E, 0x00, 0x00,
> +  # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
> +  0x48, 0x00, 0xC2, 0xF9, 0x00, 0x00, 0x00, 0x02,
> +  # Blockmap[0]: 0x3 Blocks * 0x4 Bytes / Block
> +  0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
> +  # Blockmap[1]: End
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  ## This is the VARIABLE_STORE_HEADER
> +  # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
> +  # Signature: gEfiVariableGuid =
> +  #   { 0xddcf3616, 0x3275, 0x4164,
> +  # { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}
> +  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
> +  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
> +  # Size: 0x4 
> (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
> +  # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
> +  # This can speed up the Variable Dispatch a bit.
> +  0xB8, 0xFF, 0x03, 0x00,
> +  # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +$(FTW_WORKING_BASE)|$(FTW_WORKING_SIZE)
> 

Re: [edk2-devel] [PATCH v2 25/28] Platform/NXP: Modify FV rules

[Leif Lindholm]

Please make the subject contain information.

/
Leif

On Fri, Mar 20, 2020 at 20:05:40 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Use same FV rules as ArmVirtPkg/ArmVirtRules.fdf.inc
> 
> Signed-off-by: Pankaj Bansal 
> ---
>  Platform/NXP/FVRules.fdf.inc | 59 +++-
>  1 file changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/Platform/NXP/FVRules.fdf.inc b/Platform/NXP/FVRules.fdf.inc
> index c9fba65dae85..63de26abe056 100644
> --- a/Platform/NXP/FVRules.fdf.inc
> +++ b/Platform/NXP/FVRules.fdf.inc
> @@ -1,8 +1,7 @@
> -#  FvRules.fdf.inc
>  #
> -#  Rules for creating FD.
> -#
> -#  Copyright 2017-2019 NXP
> +#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2014-2016, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -16,40 +15,49 @@
>  #
>  
> 
>  
> +
> +
> +# Example of a DXE_DRIVER FFS file with a Checksum encapsulation section   #
> +
> +#
> +#[Rule.Common.DXE_DRIVER]
> +#  FILE DRIVER = $(NAMED_GUID) {
> +#DXE_DEPEXDXE_DEPEX   Optional 
> $(INF_OUTPUT)/$(MODULE_NAME).depex
> +#COMPRESS PI_STD {
> +#  GUIDED {
> +#PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
> +#UI   STRING="$(MODULE_NAME)" Optional
> +#VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> +#  }
> +#}
> +#  }
> +#
> +
> +
>  [Rule.Common.SEC]
> -  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> -TE  TEAlign = 32$(INF_OUTPUT)/$(MODULE_NAME).efi
> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
> +TE  TE Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi
>}
>  
>  [Rule.Common.PEI_CORE]
> -  FILE PEI_CORE = $(NAMED_GUID) {
> -TE TE   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  FILE PEI_CORE = $(NAMED_GUID) FIXED {
> +TE TE Align = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
>  UI STRING ="$(MODULE_NAME)" Optional
>}
>  
>  [Rule.Common.PEIM]
> -  FILE PEIM = $(NAMED_GUID) {
> +  FILE PEIM = $(NAMED_GUID) FIXED {
>   PEI_DEPEX PEI_DEPEX Optional   $(INF_OUTPUT)/$(MODULE_NAME).depex
> - PE32  PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi
> + TE   TE Align = Auto   $(INF_OUTPUT)/$(MODULE_NAME).efi
>   UI   STRING="$(MODULE_NAME)" Optional
>}
>  
> -[Rule.Common.PEIM.TIANOCOMPRESSED]
> -  FILE PEIM = $(NAMED_GUID) DEBUG_MYTOOLS_IA32 {
> -PEI_DEPEX PEI_DEPEX Optional$(INF_OUTPUT)/$(MODULE_NAME).depex
> -GUIDED A31280AD-481E-41B6-95E8-127F4C984779 PROCESSING_REQUIRED = TRUE {
> -  PE32  PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
> -  UISTRING="$(MODULE_NAME)" Optional
> -}
> -  }
> -
>  [Rule.Common.DXE_CORE]
>FILE DXE_CORE = $(NAMED_GUID) {
>  PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
>  UI   STRING="$(MODULE_NAME)" Optional
>}
>  
> -
>  [Rule.Common.UEFI_DRIVER]
>FILE DRIVER = $(NAMED_GUID) {
>  DXE_DEPEXDXE_DEPEX  Optional 
> $(INF_OUTPUT)/$(MODULE_NAME).depex
> @@ -62,6 +70,8 @@ [Rule.Common.DXE_DRIVER]
>  DXE_DEPEXDXE_DEPEX  Optional 
> $(INF_OUTPUT)/$(MODULE_NAME).depex
>  PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
>  UI   STRING="$(MODULE_NAME)" Optional
> +RAW  ACPI  Optional   |.acpi
> +RAW  ASL   Optional   |.aml
>}
>  
>  [Rule.Common.DXE_RUNTIME_DRIVER]
> @@ -73,7 +83,7 @@ [Rule.Common.DXE_RUNTIME_DRIVER]
>  
>  [Rule.Common.UEFI_APPLICATION]
>FILE APPLICATION = $(NAMED_GUID) {
> -UI STRING ="$(MODULE_NAME)" Optional
> +UI STRING ="$(MODULE_NAME)" Optional
>  PE32   PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi
>}
>  
> @@ -91,3 +101,10 @@ [Rule.Common.UEFI_APPLICATION.BINARY]
>  UISTRING="$(MODULE_NAME)" Optional
>  VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
>}
> +
> +[Rule.Common.USER_DEFINED.ACPITABLE]
> +  FILE FREEFORM = $(NAMED_GUID) {
> +RAW   ACPI|.acpi
> +RAW   ASL |.aml
> +UISTRING="$(MODULE_NAME)" Optional
> +  }
> -- 
> 2.17.1
> 

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

View/Reply Online (#56864): https://edk2.groups.io/g/devel/message/56864
Mute This Topic: https://groups.io/mt/72077459/21656
Group Owner: 

Re: [edk2-devel] [PATCH v2 24/28] NXP/LS1043aRdbPkg/ArmPlatformLib: Remove extern SocInit

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:39 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> SocInit can be defined in SocLib.h
> No need to make it extern in ArmPlatformLib
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

But do please move it as early as possible in the series.

> ---
>  .../LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c | 2 --
>  Silicon/NXP/Include/Library/SocLib.h  | 8 
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index 1e2e85f87dfe..dc81e7ba3101 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -16,8 +16,6 @@
>  #include 
>  #include 
>  
> -extern VOID SocInit (VOID);
> -
>  /**
>Get the clocks supplied by Platform(Board) to NXP Layerscape SOC IPs
>  
> diff --git a/Silicon/NXP/Include/Library/SocLib.h 
> b/Silicon/NXP/Include/Library/SocLib.h
> index 749aa230dec5..0ca68602618d 100644
> --- a/Silicon/NXP/Include/Library/SocLib.h
> +++ b/Silicon/NXP/Include/Library/SocLib.h
> @@ -41,4 +41,12 @@ SocGetClock (
>IN  VA_LIST   Args
>);
>  
> +/**
> +  Function to initialize SoC specific constructs
> + **/
> +VOID
> +SocInit (
> +  VOID
> +  );
> +
>  #endif // SOC_LIB_H__
> -- 
> 2.17.1
> 

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

View/Reply Online (#56863): https://edk2.groups.io/g/devel/message/56863
Mute This Topic: https://groups.io/mt/72077456/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 23/28] NXP/LS1043aRdbPkg/ArmPlatformLib: Use Allocate pool

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:38 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Allocate Pages may allocate more memory than required for
> VirtualMemoryTable.
> There is no special requirement that VirtualMemoryTable size should be
> page size aligned.
> 
> Therefore, replace AllocatePages with AllocatePool.
> 
> Signed-off-by: Pankaj Bansal 

I don't object to this as such (although one comment), but what is the
purpose of this change?

My comment is that most other platforms use AllocatePages for this. So
this is diverging from the norm. Secondly, while I don't necessarily
*like* the current design (copied across most ARM platforms), it's
somewhat mitigated by the AllocatePages giving you a minimum of 128
entries before you start overwriting things. I don't know what you'll
overwrite if you do go too far, but you will overwrite it *before* the
ASSERT ever gets evaluated.

/
Leif

> ---
>  .../LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf  | 1 +
>  .../LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c | 5 +++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 1faf99b99c54..c64032f32772 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -25,6 +25,7 @@ [Packages]
>  
>  [LibraryClasses]
>ArmLib
> +  DebugLib
>SocLib
>  
>  [Sources.common]
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> index f5fa308551aa..f8dd642e3cff 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> @@ -43,10 +43,11 @@ ArmPlatformGetVirtualMemoryMap (
>  
>ASSERT (VirtualMemoryMap != NULL);
>  
> -  VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages (
> -  EFI_SIZE_TO_PAGES (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) * 
> MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
> +  VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> + MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
>  
>if (VirtualMemoryTable == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", 
> __FUNCTION__));
>  return;
>}
>  
> -- 
> 2.17.1
> 

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

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

Re: [edk2-devel] [PATCH 0/4] remove generation of EFI properties table

[Ard Biesheuvel]

On Mon, 30 Mar 2020 at 19:57, Ard Biesheuvel  wrote:
>
> (adding Jian and Hao)
>
> Thanks for the acks, and apologies for failing to cc the MdeModulePkg
> maintainers.
>
> Jian, Hao, do you have any opinion on this series?
>

Jian, Hao, Liming, Michael,

It is not always clear to me how you at Intel divide up the maintainer
duties between yourselves.

Liming and Jiewen have acked this entire series, but they are not the
MdeModulePkg maintainers, so I assume either Jian or Hao should
approve it as well before i can proceed with it.

If this is the case, could Jian and Hao please acknowledge that they
read this email, and perhaps indicate a timeframe within which they
will be able to give their verdict on these changes?

Thanks,
Ard.





>
>
> On Mon, 30 Mar 2020 at 15:42, Gao, Liming  wrote:
> >
> > Ack-by: Liming Gao 
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Yao, Jiewen
> > > Sent: Friday, March 27, 2020 1:01 PM
> > > To: Ard Biesheuvel ; devel@edk2.groups.io
> > > Cc: Laszlo Ersek ; Leif Lindholm ; 
> > > Kinney, Michael D ; Ni, Ray
> > > ; Bret Barkelew 
> > > Subject: Re: [edk2-devel] [PATCH 0/4] remove generation of EFI properties 
> > > table
> > >
> > > Acked-by: Jiewen Yao 
> > >
> > > I cannot remember if there is windows OS still using the properties table.
> > > Maybe Microsoft people can comment.
> > >
> > > If no, I agree we can remove the old code.
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Ard Biesheuvel 
> > > > Sent: Thursday, March 26, 2020 6:25 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Ard Biesheuvel ; Laszlo Ersek
> > > > ; Leif Lindholm ; Kinney, Michael 
> > > > D
> > > > ; Ni, Ray ; Yao, Jiewen
> > > > ; Bret Barkelew 
> > > > Subject: [PATCH 0/4] remove generation of EFI properties table
> > > >
> > > > The EFI properties table is broken by design, deprecated, and seems to 
> > > > be
> > > > causing confusion as it is unclear to some how it differs from the 
> > > > memory
> > > > attributes table (which supersedes it). So let's get rid of the code 
> > > > that
> > > > generates it entirely, along with the GUID definitions, PCDs etc.
> > > >
> > > > Due to how the two implementations are intertwined, patch #2 makes the
> > > > minimal changes required to stop producing the table (and to allow patch
> > > > #3 to remove the associated definitions from MdePkg). Patch #4 is 
> > > > optional
> > > > and merges the code together.
> > > >
> > > > Cc: Laszlo Ersek 
> > > > Cc: Leif Lindholm 
> > > > Cc: Michael D Kinney 
> > > > Cc: Ray Ni 
> > > > Cc: Jiewen Yao 
> > > > Cc: Bret Barkelew 
> > > >
> > > > Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2633
> > > >
> > > > Ard Biesheuvel (4):
> > > >   OvmfPkg: remove handling of properties table
> > > >   MdeModulePkg: disable properties table generation but retain the code
> > > >   MdePkg: remove PropertiesTable GUID
> > > >   MdeModulePkg/DxeCore: merge properties table routines into MAT
> > > > handling
> > > >
> > > >  MdeModulePkg/Core/Dxe/DxeMain.h   |9 -
> > > >  MdeModulePkg/Core/Dxe/DxeMain.inf |3 -
> > > >  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   |1 -
> > > >  .../Core/Dxe/Misc/MemoryAttributesTable.c | 1226 ++-
> > > >  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |1 -
> > > >  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  | 1373 -
> > > >  MdeModulePkg/MdeModulePkg.dec |   24 -
> > > >  MdeModulePkg/MdeModulePkg.uni |   21 -
> > > >  MdePkg/Include/Guid/PropertiesTable.h |   31 -
> > > >  MdePkg/MdePkg.dec |3 -
> > > >  OvmfPkg/OvmfPkgIa32.dsc   |1 -
> > > >  OvmfPkg/OvmfPkgIa32X64.dsc|1 -
> > > >  OvmfPkg/OvmfPkgX64.dsc|1 -
> > > >  OvmfPkg/OvmfXen.dsc   |1 -
> > > >  OvmfPkg/PlatformPei/Platform.c|1 -
> > > >  OvmfPkg/PlatformPei/PlatformPei.inf   |1 -
> > > >  16 files changed, 1222 insertions(+), 1476 deletions(-)
> > > >  delete mode 100644 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> > > >  delete mode 100644 MdePkg/Include/Guid/PropertiesTable.h
> > > >
> > > > --
> > > > 2.17.1
> > >
> > >
> > > 
> >

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

View/Reply Online (#56861): https://edk2.groups.io/g/devel/message/56861
Mute This Topic: https://groups.io/mt/72560881/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 Ard,

I think adding a dependency in the module entry point
libs is also good way to guarantee an intrinsic lib
is available.  I agree that it can provide module type
and arch specific mappings.  The NULL lib instance can
do the same if the NULL instance is listed in the 
module/arch specific library classes sections. a few
example section names.

[LibraryClasses.ARM.PEIM, LibraryClasses.AARCH64.PEIM]

[LibraryClasses.IA32.UEFI_DRIVER]

[LibraryClasses.X64.DXE_DRIVER]

So either way, the DSC files need to provide the
library mapping.  The only difference between these
2 approaches is that adding a dependency to the
module entry point libs uses a formally defined
library class name for the intrinsic functions vs.
the un-named NULL library instance.  A formally
defined library class name is better supported for
things like unit tests from the UnitTestFrameworkPkg.

The fltused issue would not go away of we removed the 
float usage for OpenSSL.  One or more other libs or the
module could use float/double types and this issue
would pop up again.  I do agree it would be cleaner
if we could use OpenSSL without floating point.

I think adding an intrinsic lib for IA32/X64 for VS20xx
generation of intrinsic functions would address fltused
and other common C implementation styles that generate
intrinsic functions (e.g. 64-bit int math on 32-bit,
structure variable assignments, and loops that fill a buffer
with a const value) by VS20xx compilers.  An intrinsic
lib for GCC would also help with these same common C
implementation styles that generate intrinsic functions.

Mike

> -Original Message-
> From: devel@edk2.groups.io  On
> Behalf Of Ard Biesheuvel
> Sent: Tuesday, March 31, 2020 11:43 PM
> To: Kinney, Michael D 
> Cc: devel@edk2.groups.io; ler...@redhat.com;
> mac...@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> Add FltUsedLib for float.
> 
> On Tue, 31 Mar 2020 at 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/CompilerIntrins
> icsLib.inf
> >
> > Can we use this same technique for IA32/X64 VS builds?
> >
> 
> In my opinion, having these intrinsics libraries added
> via NULL
> library class resolution was a mistake to begin with.
> 
> Every component that we build incorporates some kind of
> startup code
> library that defines the _ModuleEntryPoint symbol, and it
> would be
> much better to make those libraries include an
> IntrinsicsLibrary
> dependency that can be satisfied by arch specific
> versions that also
> encapsulate the toolchain dependencies (such as this
> _fltused symbol,
> or memcpy/memset on ARM/GCC).
> 
> On another note, it is still deeply disappointing that we
> need to jump
> through all of these hoops because the *only* single use
> of floating
> point in OpenSSL is the entropy estimate of an RNG, which
> is in the
> range of 0..1023 to begin with [IIRC). Remember that we
> also have a
> softfloat submodule for 32-bit ARM, for the same stupid
> reason. If we
> could stop pulling in that part of the code (or replace
> it with an
> UINT16 when building for the UEFI target), the whole
> floating point
> issue would mostly go away AFAICT.
> 
> 


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

View/Reply Online (#56860): https://edk2.groups.io/g/devel/message/56860
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 v1 1/1] INF Spec: Add file dependency to [Sources] syntax

[Michael D Kinney]

Hi Pierre,

We discussed this in the bug scrub yesterday.

We think there may be a simpler way to address this issue
without extending the INF syntax.

It is our understanding that the ASL files need to be
processed before C files when both are present in an INF.

This is similar to the requirement that UNI files be processed
before C files that is already supported.

Please continue the discussion in Bugzilla with Bob.

Thanks,

Mike

> -Original Message-
> From: devel@edk2.groups.io  On
> Behalf Of Bob Feng
> Sent: Wednesday, April 1, 2020 1:53 AM
> To: devel@edk2.groups.io; pierre.gond...@arm.com; Sami
> Mujawar ; Gao, Liming
> 
> Cc: nd 
> Subject: Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add
> file dependency to [Sources] syntax
> 
> Hi Pierre,
> 
> I will review the spec and code change in this week.
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
> On Behalf Of PierreGondois
> Sent: Monday, March 30, 2020 11:52 PM
> To: devel@edk2.groups.io; Sami Mujawar
> ; Feng, Bob C
> ; Gao, Liming
> 
> Cc: nd 
> Subject: Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add
> file dependency to [Sources] syntax
> 
> Hello Liming and Bob,
> I couldn't find the list of maintainers for the
> specification files, but it seems Liming is a maintainer.
> If a maintainer is missing, please feel free to cc him,
> 
> Regards,
> Pierre
> 
> -Original Message-
> From: PierreGondois 
> Sent: Monday, March 30, 2020 4:43 PM
> To: devel@edk2.groups.io
> Cc: Pierre Gondois ;
> liming@intel.com; sami.muja...@arm.org; nd
> 
> Subject: [PATCH v1 1/1] INF Spec: Add file dependency to
> [Sources] syntax
> 
> From: Pierre Gondois 
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2464
> 
> When building an edk2 module, a C file was including a
> .hex file generated by the compilation of an ASL file.
> To describe this dependency between an ASL file and a C
> file, the edk2 patch,
>  - named "BaseTools: Build ASL files before C files",
>  - discussed at:
> https://edk2.groups.io/g/devel/message/52550
> has been created.
> This patch allows to establish build dependencies in the
> [Sources] section, between files that are not of the same
> language.
> E.g.:
> [Sources]
>   FileName1.X
>   FileName2.Y : FileName1.X
>   FileName3.Z : FileName1.X FileName2.Y
> 
> Here:
>   * FileName1.X will be built prior to FileName2.Y.
>   * FileName1.X and FileName2.Y will be built prior to
> FileName3.Z.
> 
> This patch updates the Inf specification accordingly.
> 
> Signed-off-by: Pierre Gondois 
> ---
> 
> The changes can be seen at
> https://github.com/PierreARM/edk2-
> InfSpecification/tree/Bugzilla_2464_Enable_Build_Dependen
> cies_v1
> 
> Notes:
> v1:
>  - Enable build dependencies in the [Sources] section
> 
>  2_inf_overview/25_[sources]_section.md
> | 12 
>  3_edk_ii_inf_file_format/32_component_inf_definition.md
> |  3 +++
>  3_edk_ii_inf_file_format/39_[sources]_sections.md
> |  6 --
>  README.md
> |  1 +
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/2_inf_overview/25_[sources]_section.md
> b/2_inf_overview/25_[sources]_section.md
> index
> 54757e61e37268eed293a5288e607cf2c7cfacf6..5b9f0a8395ef2be
> 4497d99197dc695625d841830 100644
> --- a/2_inf_overview/25_[sources]_section.md
> +++ b/2_inf_overview/25_[sources]_section.md
> @@ -2,6 +2,7 @@
>2.5 [Sources] Section
> 
>Copyright (c) 2007-2019, Intel Corporation. All rights
> reserved.
> +  Copyright (c) 2020, ARM Limited. All rights
> reserved.
> 
>Redistribution and use in source (original document
> form) and 'compiled'
>forms (converted to PDF, epub, HTML and other formats)
> with or without @@ -94,6 +95,17 @@ The following is an
> example for sources sections.
> 
>  ```
> 
> +The following example depicts the syntax to establish
> dependencies
> +between files of different source types. As shown in the
> example below,
> +Dsdt.asl will be compiled before DadtHandler.c:
> +
> +```ini
> +[Sources.common]
> +  DsdtHandler.c : Dsdt.asl
> +  DsdtHandler.h
> +  Dsdt.asl
> +```
> +
>  All Unicode files must be listed in the source section.
> If a Unicode file,  `A.uni`, has the statement: `#include
> B.uni`, and `B.uni` has a statement:
>  `#include C.uni`, both `B.uni` and `C.uni` files must be
> listed in the INF diff --git
> a/3_edk_ii_inf_file_format/32_component_inf_definition.md
> b/3_edk_ii_inf_file_format/32_component_inf_definition.md
> index
> 164771cb4cfff6e81fbf762a67ff741c190cecde..d776714c24c0baf
> 2348f53dc2576c9feb6f3cb5e 100644
> ---
> a/3_edk_ii_inf_file_format/32_component_inf_definition.md
> +++
> b/3_edk_ii_inf_file_format/32_component_inf_definition.md
> @@ -2,6 +2,7 @@
>3.2 Component INF Definition
> 
>Copyright (c) 2007-2019, Intel Corporation. All rights
> reserved.
> +  Copyright (c) 2020, ARM Limited. All rights
> reserved.
> 
>Redistribution and use in source (original document
> form) and 'compiled'
>

Re: [edk2-devel] [PATCH v2 22/28] NXP/LS1043aRdbPkg/ArmPlatformLib: Use default ArmPlatformHelper.S

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:37 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Default ArmPlatformHelper.S from ArmPlatformPkg is suffecient for
> LS1043ARDB ArmPlatformLib. Use the same.

The commit message could be clarified to to explicitly state that
ArmPlatformHelper.S is being replaced by the ArmPlatformPkg version at
commit hash f4dfad05dda2c7b29e8105605621f2b413f0af2b.

/
Leif

> Signed-off-by: Pankaj Bansal 
> ---
>  .../AArch64/ArmPlatformHelper.S   | 60 ---
>  .../Library/ArmPlatformLib/ArmPlatformLib.c   |  8 ---
>  .../Library/ArmPlatformLib/ArmPlatformLib.inf |  2 +
>  3 files changed, 39 insertions(+), 31 deletions(-)
> 
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
>  
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> index dfbf73675a2d..b7c6dbdc2e61 100644
> --- 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> +++ 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> @@ -1,31 +1,45 @@
> -#  @file
> -#
> -#  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> -#  Copyright 2017, 2020 NXP
> -#
> -#  SPDX-License-Identifier: BSD-2-Clause-Patent
> -#
> +//
> +//  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> +//
> +//  SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//
>  
>  #include 
> -#include 
> -
> -.text
> -.align 2
> -
> -GCC_ASM_IMPORT(ArmReadMpidr)
> -
> -ASM_FUNC(ArmPlatformIsPrimaryCore)
> -  tst x0, #3
> -  cset x0, eq
> -  ret
> +#include 
>  
>  ASM_FUNC(ArmPlatformPeiBootAction)
> -EL1_OR_EL2(x0)
> -1:
> -2:
>ret
>  
> +//UINTN
> +//ArmPlatformGetCorePosition (
> +//  IN UINTN MpId
> +//  );
> +// With this function: CorePos = (ClusterId * 4) + CoreId
> +ASM_FUNC(ArmPlatformGetCorePosition)
> +  and   x1, x0, #ARM_CORE_MASK
> +  and   x0, x0, #ARM_CLUSTER_MASK
> +  add   x0, x1, x0, LSR #6
> +  ret
> +
> +//UINTN
> +//ArmPlatformGetPrimaryCoreMpId (
> +//  VOID
> +//  );
>  ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> -  MOV32 (x0, FixedPcdGet32(PcdArmPrimaryCore))
> -  ldrh   w0, [x0]
> +  MOV32  (w0, FixedPcdGet32 (PcdArmPrimaryCore))
> +  ret
> +
> +//UINTN
> +//ArmPlatformIsPrimaryCore (
> +//  IN UINTN MpId
> +//  );
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32  (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
> +  and   x0, x0, x1
> +  MOV32  (w1, FixedPcdGet32 (PcdArmPrimaryCore))
> +  cmp   w0, w1
> +  mov   x0, #1
> +  mov   x1, #0
> +  csel  x0, x0, x1, eq
>ret
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index 7f5872a78cfc..1e2e85f87dfe 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -139,11 +139,3 @@ ArmPlatformGetPlatformPpiList (
>*PpiList = gPlatformPpiTable;
>  }
>  
> -
> -UINTN
> -ArmPlatformGetCorePosition (
> -  IN UINTN MpId
> -  )
> -{
> -  return 1;
> -}
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 07ca6b34445f..1faf99b99c54 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -37,3 +37,5 @@ [Ppis]
>  
>  [FixedPcd]
>gArmTokenSpaceGuid.PcdArmPrimaryCore
> +  gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
> +
> -- 
> 2.17.1
> 

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

View/Reply Online (#56858): https://edk2.groups.io/g/devel/message/56858
Mute This Topic: https://groups.io/mt/72077455/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 21/28] Slicon/NXP: Add PlatformPei Lib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:36 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> PlatformPeiLib is going to be linked to Platform PEIM.
> 
> Signed-off-by: Pankaj Bansal 
> ---
>  .../Library/PlatformPeiLib/PlatformPeiLib.c   | 30 ++
>  .../Library/PlatformPeiLib/PlatformPeiLib.inf | 41 +++
>  Silicon/NXP/NxpQoriqLs.dsc.inc|  3 +-
>  3 files changed, 73 insertions(+), 1 deletion(-)
>  create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
>  create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> 
> diff --git a/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c 
> b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
> new file mode 100644
> index ..f64e564469f8
> --- /dev/null
> +++ b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
> @@ -0,0 +1,30 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> +*  Copyright 2020 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 

Although this is based on an existing library, please sort includes
alphabetically here.

> +
> +#define XPRINT(x) PRINT(x)
> +#define PRINT(x) #x

This isn't a PRINT operation, this is a Stringize operation.

> +
> +EFI_STATUS
> +EFIAPI
> +PlatformPeim (
> +  VOID
> +  )
> +{
> +  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
> +  DEBUG ((DEBUG_INIT, "Edk2 version is %a\n", XPRINT 
> (WORKSPACE_GIT_VERSION)));
> +  DEBUG ((DEBUG_INIT, "Edk2 platforms version is %a\n", XPRINT 
> (PACKAGES_PATH_GIT_VERSION)));

The only benefit I can see from the macro as opposed to using '#'
directly is that it permits wrapping of too long lines, so please do
that.

> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf 
> b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> new file mode 100644
> index ..fb42693daa20
> --- /dev/null
> +++ b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> @@ -0,0 +1,41 @@
> +#/** @file
> +#
> +#  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION= 0x00010005

Update version.

> +  BASE_NAME  = ArmPlatformPeiLib
> +  FILE_GUID  = 49d37060-70b5-11e0-aa2d-0002a5d5c51b

Unless this is another magic GUID filename (like ACPI), please
generate a new GUID.

> +  MODULE_TYPE= PEIM
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = PlatformPeiLib
> +
> +[BuildOptions]
> +  GCC:*_*_*_CC_FLAGS = -DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"
> +  GCC:*_*_*_CC_FLAGS = 
> -DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"

Does this not require special magic build command line options to do
anything useful? This needs documenting.

/
Leif

> +
> +[Sources]
> +  PlatformPeiLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  HobLib
> +  PcdLib
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdFvSize
> +
> +[depex]
> +  TRUE
> diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
> index 234a5e2707cd..5f77f47f0399 100644
> --- a/Silicon/NXP/NxpQoriqLs.dsc.inc
> +++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
> @@ -101,6 +101,8 @@ [LibraryClasses.common]
>PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
>  
> +  PlatformPeiLib|Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> +
>  [LibraryClasses.common.SEC]
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>
> UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> @@ -111,7 +113,6 @@ [LibraryClasses.common.SEC]
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>
> MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
>  
># 1/123 faster than Stm or Vstm version
> -- 
> 2.17.1
> 

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

View/Reply Online (#56857): https://edk2.groups.io/g/devel/message/56857
Mute This Topic: https://groups.io/mt/72077454/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] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Laszlo Ersek]

On 04/01/20 00:56, 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 | 163 +++--
>  1 file changed, 85 insertions(+), 78 deletions(-)

Pushed as commit e210fc130e5c9738909dca432bbf8bf277ba6e37, via
.

Thanks!
Laszlo


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

View/Reply Online (#56856): https://edk2.groups.io/g/devel/message/56856
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

[Laszlo Ersek]

On 03/31/20 13:04, Liran Alon wrote:
> 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
> 

Pushed as commit 98936dc4f44b4ef47e7221d435de06a0813aa00a, via
.

Thanks,
Laszlo


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

View/Reply Online (#56855): https://edk2.groups.io/g/devel/message/56855
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

[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
> +R: Liran Alon 
> +R: Nikita Leshenko 
> +
>  PcAtChipsetPkg
>  F: PcAtChipsetPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/PcAtChipsetPkg
> 

Pushed as commit 335644f90f15fac8bfd5575f937fa65af4978a08, via
.

Thanks,
Laszlo


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

View/Reply Online (#56854): https://edk2.groups.io/g/devel/message/56854
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] NetworkPkg/UefiPxeBcDxe: handle competing DHCP servers (more) gracefully

[Laszlo Ersek]

On 03/31/20 19:49, Maciej Rabeda wrote:
> Always better than not detecting such stuff at all (or by ASSERT in
> debug builds). Thanks for the patch!
> 
> Reviewed-by: Maciej Rabeda 

Thanks All for the reviews; pushed as commit
3f55418d5396629c4458061f283068b6c46895fc, via
.

Laszlo

> 
> 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 (#56853): https://edk2.groups.io/g/devel/message/56853
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 1/1] OvmfPkg: Fix SMM/RT driver section alignment for XCODE5/CLANGPDB

[Laszlo Ersek]

On 03/30/20 13:37, Laszlo Ersek wrote:
> On 03/29/20 15:21, Vitaly Cheptsov wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2642
>>
>> This patch resolves the problem of using memory protection
>> attributes when OVMF firmware is compiled with XCODE5 and CLANGPDB.
>>
>> CC: Andrew Fish 
>> CC: Laszlo Ersek 
>> CC: Marvin Häuser 
>> Signed-off-by: Vitaly Cheptsov 
>> ---
>>  OvmfPkg/OvmfPkgIa32.dsc| 6 --
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 --
>>  OvmfPkg/OvmfPkgX64.dsc | 6 --
>>  OvmfPkg/OvmfXen.dsc| 8 ++--
>>  4 files changed, 18 insertions(+), 8 deletions(-)
> 
> This patch is an update on TianoCore#559 / commit 01e9597540fa
> ("OvmfPkg: Add XCODE5 statements to fix build break", 2017-05-19).
> 
> I'd like Mike and/or Liming to ACK this patch (I don't use either XCODE
> or CLANGPDB, and the patches related to those toolchains seem to have
> come from Liming and Mike mainly).
> 
> Afterwards, I'm happy to push this patch:
> 
> Acked-by: Laszlo Ersek 

Pushed as commit 4fb393aaa8bb029dc98a1330f40303bf16e2b092, via
.

(I didn't want to delay this patch -- it's potential to cause a
regression is limited, and if it happens, we still have plenty of time
to fix it for the stable tag.)

Thanks
Laszlo


>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 19728f20b3..0aba200c2d 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -80,14 +80,16 @@ [BuildOptions]
>>
>>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>>CLANGPDB:*_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>  # Force PE/COFF sections to be aligned at 4KB boundaries to support page 
>> level
>>  # protection of DXE_SMM_DRIVER/SMM_CORE modules
>>  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, 
>> BuildOptions.common.EDKII.SMM_CORE]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>>CLANGPDB:*_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>  
>> 
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 3c0c229e3a..eca70d64c3 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -85,14 +85,16 @@ [BuildOptions]
>>
>>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>>CLANGPDB:*_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>  # Force PE/COFF sections to be aligned at 4KB boundaries to support page 
>> level
>>  # protection of DXE_SMM_DRIVER/SMM_CORE modules
>>  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, 
>> BuildOptions.common.EDKII.SMM_CORE]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>>CLANGPDB:*_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>  
>> 
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index f6c1d8d228..676d0ed9a6 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -85,14 +85,16 @@ [BuildOptions]
>>
>>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>>CLANGPDB:*_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>  # Force PE/COFF sections to be aligned at 4KB boundaries to support page 
>> level
>>  # protection of DXE_SMM_DRIVER/SMM_CORE modules
>>  [BuildOptions.common.EDKII.DXE_SMM_DRIVER, 
>> BuildOptions.common.EDKII.SMM_CORE]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>>CLANGPDB:*_*_*_DLINK_FLAGS = /ALIGN:4096
>>
>>  
>> 
>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>> index 5751ff1f03..783bac3e8b 100644
>> --- a/OvmfPkg/OvmfXen.dsc
>> +++ b/OvmfPkg/OvmfXen.dsc
>> @@ -82,13 +82,17 @@ [BuildOptions]
>>
>>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> -  XCODE:*_*_*_DLINK_FLAGS =
>> +  XCODE:*_*_*_DLINK_FLAGS = -seg1addr 0x1000 -segalign 0x1000
>> +  XCODE:*_*_*_MTOC_FLAGS = -align 0x1000
>> +  

Re: [edk2-devel] [PATCH v2 20/28] Silicon/NXP/LS1043A: Move SocLib to Soc Package

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:35 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> The SocLib contains code specific to an Soc. it should be part of
> SOC package.
> Therefore, move the SocLib to Soc Package.
> Since we are moving the files to Soc Package, no need to mention the
> Soc name in file names. Their location is enough to indicate for which
> Soc the files are.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc | 2 +-
>  .../SocLib/Chassis2/Soc.c => LS1043A/Library/SocLib/SocLib.c}   | 0
>  .../LS1043aSocLib.inf => LS1043A/Library/SocLib/SocLib.inf} | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename Silicon/NXP/{Library/SocLib/Chassis2/Soc.c => 
> LS1043A/Library/SocLib/SocLib.c} (100%)
>  rename Silicon/NXP/{Library/SocLib/LS1043aSocLib.inf => 
> LS1043A/Library/SocLib/SocLib.inf} (92%)
> 
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc 
> b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> index 7e75d5b7cba9..a5942c0f2c9c 100644
> --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> @@ -11,7 +11,7 @@
>  
>  [LibraryClasses.common]
>
> PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
> -  SocLib|Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> +  SocLib|Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
>
> SerialUartClockLib|Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
>
> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>  
> diff --git a/Silicon/NXP/Library/SocLib/Chassis2/Soc.c 
> b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.c
> similarity index 100%
> rename from Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> rename to Silicon/NXP/LS1043A/Library/SocLib/SocLib.c
> diff --git a/Silicon/NXP/Library/SocLib/LS1043aSocLib.inf 
> b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> similarity index 92%
> rename from Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> rename to Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> index 1d042bbfc4e4..3d0f988e1c67 100644
> --- a/Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> +++ b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf
> @@ -24,4 +24,4 @@ [LibraryClasses]
>DebugLib
>  
>  [Sources.common]
> -  Chassis2/Soc.c
> +  SocLib.c
> -- 
> 2.17.1
> 

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

View/Reply Online (#56851): https://edk2.groups.io/g/devel/message/56851
Mute This Topic: https://groups.io/mt/72077453/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 19/28] Silicon/NXP/LS1043A: Use ChassisLib from Chassis2 Pkg

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:34 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Now the we have added Chassis Package, move the chassis specific common
> code for all SOCs belonging to same chassis to ChassisLib.
> 
> Use ChassisLib APIs in SocLib.
> 
> Signed-off-by: Pankaj Bansal 

There will be minor API changes required to this file based on
addressing other feedback, but unless any substantial rewrites are
needed:
Reviewed-by: Leif Lindholm 

> ---
>  .../Drivers/PlatformDxe/PlatformDxe.inf   |  1 +
>  .../Library/ArmPlatformLib/ArmPlatformLib.inf |  1 +
>  Silicon/NXP/Include/Chassis2/NxpSoc.h | 44 -
>  Silicon/NXP/LS1043A/Include/Soc.h |  6 +-
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc   |  9 ++-
>  Silicon/NXP/Library/SocLib/Chassis.c  | 61 ---
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c | 19 +-
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf  | 15 +
>  Silicon/NXP/Library/SocLib/NxpChassis.h   | 22 ---
>  Silicon/NXP/NxpQoriqLs.dec|  6 --
>  10 files changed, 14 insertions(+), 170 deletions(-)
>  delete mode 100644 Silicon/NXP/Include/Chassis2/NxpSoc.h
>  delete mode 100644 Silicon/NXP/Library/SocLib/Chassis.c
>  delete mode 100644 Silicon/NXP/Library/SocLib/NxpChassis.h
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf 
> b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index 038d48949a39..e522db81e5c0 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -24,6 +24,7 @@ [Packages]
>MdeModulePkg/MdeModulePkg.dec
>MdePkg/MdePkg.dec
>Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.dec
> +  Silicon/NXP/Chassis2/Chassis2.dec
>Silicon/NXP/LS1043A/LS1043A.dec
>Silicon/NXP/NxpQoriqLs.dec
>  
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 7a43ad86d183..07ca6b34445f 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -19,6 +19,7 @@ [Packages]
>ArmPlatformPkg/ArmPlatformPkg.dec
>EmbeddedPkg/EmbeddedPkg.dec
>MdePkg/MdePkg.dec
> +  Silicon/NXP/Chassis2/Chassis2.dec
>Silicon/NXP/LS1043A/LS1043A.dec
>Silicon/NXP/NxpQoriqLs.dec
>  
> diff --git a/Silicon/NXP/Include/Chassis2/NxpSoc.h 
> b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> deleted file mode 100644
> index 3f00a2614131..
> --- a/Silicon/NXP/Include/Chassis2/NxpSoc.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -/** Soc.h
> -*  Header defining the Base addresses, sizes, flags etc for chassis 1
> -*
> -*  Copyright 2017-2020 NXP
> -*
> -*  SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#ifndef NXP_SOC_H_
> -#define NXP_SOC_H_
> -
> -#define CLK_FREQ   1
> -
> -#define CHASSIS2_DCFG_ADDRESS  0x1EE
> -
> -/* SMMU Defintions */
> -#define SMMU_BASE_ADDR 0x0900
> -#define SMMU_REG_SCR0  (SMMU_BASE_ADDR + 0x0)
> -#define SMMU_REG_SACR  (SMMU_BASE_ADDR + 0x10)
> -#define SMMU_REG_IDR1  (SMMU_BASE_ADDR + 0x24)
> -#define SMMU_REG_NSCR0 (SMMU_BASE_ADDR + 0x400)
> -#define SMMU_REG_NSACR (SMMU_BASE_ADDR + 0x410)
> -
> -#define SCR0_USFCFG_MASK   0x0400
> -#define SCR0_CLIENTPD_MASK 0x0001
> -#define SACR_PAGESIZE_MASK 0x0001
> -#define IDR1_PAGESIZE_MASK 0x8000
> -
> -/* Device Configuration and Pin Control */
> -typedef struct {
> -  UINT8Res0[0x100-0x00];
> -  UINT32   RcwSr[16];  /* Reset control word status */
> -#define CHASSIS2_RCWSR0_SYS_PLL_RAT_SHIFT  25
> -#define CHASSIS2_RCWSR0_SYS_PLL_RAT_MASK  0x1f
> -} CCSR_GUR;
> -
> -UINT32
> -EFIAPI
> -GurRead (
> -  IN  UINTN Address
> -  );
> -
> -#endif /* NXP_SOC_H_ */
> diff --git a/Silicon/NXP/LS1043A/Include/Soc.h 
> b/Silicon/NXP/LS1043A/Include/Soc.h
> index e62de570da8a..97a77d3f5da6 100644
> --- a/Silicon/NXP/LS1043A/Include/Soc.h
> +++ b/Silicon/NXP/LS1043A/Include/Soc.h
> @@ -8,7 +8,7 @@
>  #ifndef SOC_H__
>  #define SOC_H__
>  
> -#include 
> +#include 
>  
>  /**
>Soc Memory Map
> @@ -43,13 +43,13 @@
>  #define LS1043A_I2C_SIZE 0x1
>  #define LS1043A_I2C_NUM_CONTROLLERS  4
>  
> -#define LS1043A_DCFG_ADDRESS CHASSIS2_DCFG_ADDRESS
> +#define LS1043A_DCFG_ADDRESS NXP_LAYERSCAPE_CHASSIS2_DCFG_ADDRESS
>  
>  /**
>Reset Control Word (RCW) Bits
>  **/
>  #define SYS_PLL_RAT(x)  (((x) & 0x7c) >> 2) // Bits 2-6
>  
> -typedef CCSR_GUR LS1043A_DEVICE_CONFIG;
> +typedef NXP_LAYERSCAPE_CHASSIS2_DEVICE_CONFIG LS1043A_DEVICE_CONFIG;
>  
>  #endif // SOC_H__
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc 
> b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> index 

Re: [edk2-devel] [PATCH v2 18/28] Silicon/NXP: Add Chassis2 Package

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:33 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> A Chassis is a base framework used for building SoCs.
> We can think of Chassis/Soc/Platform(a.k.a Borad) in Oops terms.

 Board?
"Oops" is not a term I am familiar with other than as an exclamation -
can you elaborate please?

> Chassis is base. Soc is based on some Chassis.
> Platform is based on some Soc.
> 
> SOCs that are designed around same chassis, reuse most of the components.
> 
> Therefore, add the package for Chassis2. LS1043A and LS1046A SOCs belong
> to Chassis2.
> 
> Signed-off-by: Pankaj Bansal 
> ---
>  Silicon/NXP/Chassis2/Chassis2.dec | 23 +
>  Silicon/NXP/Chassis2/Chassis2.dsc.inc | 10 ++
>  Silicon/NXP/Chassis2/Include/Chassis.h| 34 +++
>  .../Chassis2/Library/ChassisLib/ChassisLib.c  | 97 +++
>  .../Library/ChassisLib/ChassisLib.inf | 34 +++
>  Silicon/NXP/Include/Library/ChassisLib.h  | 51 ++
>  Silicon/NXP/NxpQoriqLs.dec|  4 +
>  7 files changed, 253 insertions(+)
>  create mode 100644 Silicon/NXP/Chassis2/Chassis2.dec
>  create mode 100644 Silicon/NXP/Chassis2/Chassis2.dsc.inc
>  create mode 100644 Silicon/NXP/Chassis2/Include/Chassis.h
>  create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
>  create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
>  create mode 100644 Silicon/NXP/Include/Library/ChassisLib.h
> 
> diff --git a/Silicon/NXP/Chassis2/Chassis2.dec 
> b/Silicon/NXP/Chassis2/Chassis2.dec
> new file mode 100644
> index ..a0048bd784ea
> --- /dev/null
> +++ b/Silicon/NXP/Chassis2/Chassis2.dec
> @@ -0,0 +1,23 @@
> +# @file
> +# NXP Layerscape processor package.
> +#
> +# Copyright 2020 NXP
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION  = 1.27
> +  PACKAGE_VERSION= 0.1
> +
> +
> +#
> +# Include Section - list of Include Paths that are provided by this package.
> +#   Comments are used for Keywords and Module Types.
> +#
> +#
> +
> +[Includes.common]
> +  Include# Root include for the package
> +
> diff --git a/Silicon/NXP/Chassis2/Chassis2.dsc.inc 
> b/Silicon/NXP/Chassis2/Chassis2.dsc.inc
> new file mode 100644
> index ..db8e5a92eacb
> --- /dev/null
> +++ b/Silicon/NXP/Chassis2/Chassis2.dsc.inc
> @@ -0,0 +1,10 @@
> +#  @file
> +#
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +[LibraryClasses.common]
> +  ChassisLib|Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf
> diff --git a/Silicon/NXP/Chassis2/Include/Chassis.h 
> b/Silicon/NXP/Chassis2/Include/Chassis.h
> new file mode 100644
> index ..72bd97efd004
> --- /dev/null
> +++ b/Silicon/NXP/Chassis2/Include/Chassis.h
> @@ -0,0 +1,34 @@
> +/** @file
> +
> +  Copyright 2020 NXP
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#ifndef CHASSIS_H__
> +#define CHASSIS_H__
> +
> +#define  NXP_LAYERSCAPE_CHASSIS2_DCFG_ADDRESS  0x1EE
> +
> +/* SMMU Defintions */
> +#define SMMU_BASE_ADDR 0x0900
> +#define SMMU_REG_SCR0  (SMMU_BASE_ADDR + 0x0)
> +#define SMMU_REG_SACR  (SMMU_BASE_ADDR + 0x10)
> +#define SMMU_REG_NSCR0 (SMMU_BASE_ADDR + 0x400)
> +
> +#define SCR0_USFCFG_MASK   0x0400
> +#define SCR0_CLIENTPD_MASK 0x0001
> +#define SACR_PAGESIZE_MASK 0x0001
> +
> +/**
> +  The Device Configuration Unit provides general purpose configuration and
> +  status for the device. These registers only support 32-bit accesses.
> +**/
> +#pragma pack(1)
> +typedef struct {
> +  UINT8   Reserved0[0x100 - 0x0];
> +  UINT32  RcwSr[16]; // Reset Control Word Status Register

If this is the formal word as used in official documentation, it still
needs listing in a glossary in the file comment header.

> +} NXP_LAYERSCAPE_CHASSIS2_DEVICE_CONFIG;
> +#pragma pack()
> +
> +#endif // CHASSIS_H__
> diff --git a/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c 
> b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
> new file mode 100644
> index ..816e0fa29c4a
> --- /dev/null
> +++ b/Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c
> @@ -0,0 +1,97 @@
> +/** @file
> +  Chassis specific functions common to all SOCs based on a specific Chessis
> +
> +  Copyright 2020 NXP
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 

Can you flip the two above lines around to get them in order?

> +#include 
> +#include 
> +
> +/**
> +  Read Dcfg register
> +
> +  @param  Address The MMIO register to read.
> +
> +  @return The value read.
> 

Re: [edk2-devel] [PATCH] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()

[Maciej Rabeda]

Hey Laszlo,

Thanks for your _meta-suggestions_ :)

(1a): BZ created, referenced to BZ 2174, marked with 'regression': 
https://bugzilla.tianocore.org/show_bug.cgi?id=2655

(2): This will occur upon passed review and pull request to edk2/master

Siyuan/Jiaxin,

Can we get this through?

Thanks,
Maciej

On 01-Apr-20 14:02, Laszlo Ersek wrote:

Hi Maciej,

On 04/01/20 11:53, Maciej Rabeda wrote:

This patch fixes reversed logic of recently added ASSERTs which should
ensure that Ip6IsNDOptionValid() implementation properly reacts to invalid
packets.

Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Signed-off-by: Maciej Rabeda 
Tested-by: Laszlo Ersek 
---
  NetworkPkg/Ip6Dxe/Ip6Nd.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks for the patch. Two meta-suggestions:

(1) we should do one of the following:

(1a) Create a new BZ for this issue, cross-link it -- via the See Also
field -- with TianoCore#2174, and reference the new BZ in this commit
message. If we file this new BZ, it should get the Regression keyword.

(1b) Or else we should reopen TianoCore#2174, and reference *that* BZ in
this commit message.

(2) Independently of (1), the commit message should carry the following tag:

Fixes: 9c20342eed70ec99ec50cd73cb81804299f05403

Regarding this patch, the above updates only affect the commit message,
so I'm not asking for a v2 -- you can implement the commit message
changes right before pushing.

Thanks!
Laszlo




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








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

View/Reply Online (#56848): https://edk2.groups.io/g/devel/message/56848
Mute This Topic: https://groups.io/mt/72696739/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 15:23:24 +0200, Laszlo Ersek wrote:
> On 03/31/20 11:22, Leif Lindholm wrote:
> > "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.

Agreed.

> 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.

And a daily CI job could spot breakages and send out alerts to
platform owners.

It would also provide more incentive for actually upstreaming platform
ports.

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

I have no strong comment on your alternative. It seems perfectly
feasible, and I agree there is precedent. Thanks for providing it.

I will let the MdeModulePkg maintainers specify their preference, or
provide other alternative solutions.

Best Regards,

Leif

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

View/Reply Online (#56847): https://edk2.groups.io/g/devel/message/56847
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 v2 17/28] Silicon/NXP: Use Clock retrieval PPI in modules

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:32 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Use NXP_PLATFORM_GET_CLOCK_PPI in various Layerscape IP modules.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc  |  2 --
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.c   |  3 +-
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.h   |  6 
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf |  2 +-
>  Silicon/NXP/Include/Chassis2/NxpSoc.h |  9 --
>  .../SerialUartClockLib/SerialUartClockLib.c   |  9 ++
>  .../SerialUartClockLib/SerialUartClockLib.inf |  2 +-
>  Silicon/NXP/Library/SocLib/Chassis.c  | 15 -
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c | 31 ---
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf  |  1 -
>  Silicon/NXP/NxpQoriqLs.dec|  5 ---
>  11 files changed, 6 insertions(+), 79 deletions(-)
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc 
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index e5383aaf0cc5..d486c9b36fab 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -39,8 +39,6 @@ [PcdsFixedAtBuild.common]
>gArmTokenSpaceGuid.PcdSystemMemorySize|0x7BE0
>gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0200
>  
> -  gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x1
> -
>#
># RTC Pcds
>#
> diff --git a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c 
> b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
> index a5aba47b3ed4..30804450d2b7 100644
> --- a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
> +++ b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "I2cDxe.h"
>  
> @@ -51,7 +52,7 @@ SetBusFrequency (
>  
>I2cBase = (UINTN)(I2c->Dev->Resources[0].AddrRangeMin);
>  
> -  I2cClock = GetBusFrequency ();
> +  I2cClock = gPlatformGetClockPpi.PlatformGetClock (NXP_I2C_CLOCK, 0);
>  
>I2cInitialize (I2cBase, I2cClock, *BusClockHertz);
>  
> diff --git a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.h 
> b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.h
> index 88316f313380..7c4a306c16a0 100644
> --- a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.h
> +++ b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.h
> @@ -37,12 +37,6 @@ typedef struct {
>NON_DISCOVERABLE_DEVICE *Dev;
>  } NXP_I2C_MASTER;
>  
> -extern
> -UINT64
> -GetBusFrequency (
> -  VOID
> -  );
> -
>  EFI_STATUS
>  NxpI2cInit (
>IN EFI_HANDLE  DriverBindingHandle,
> diff --git a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf 
> b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> index 867376044656..3bf7a8124fc6 100644
> --- a/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> +++ b/Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> @@ -25,13 +25,13 @@ [Sources.common]
>  
>  [LibraryClasses]
>ArmLib
> +  ArmPlatformLib
>BaseMemoryLib
>DevicePathLib
>I2cLib
>IoLib
>MemoryAllocationLib
>PcdLib
> -  SocLib
>TimerLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
> diff --git a/Silicon/NXP/Include/Chassis2/NxpSoc.h 
> b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> index 6812beafe447..3f00a2614131 100644
> --- a/Silicon/NXP/Include/Chassis2/NxpSoc.h
> +++ b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> @@ -27,10 +27,6 @@
>  #define SACR_PAGESIZE_MASK 0x0001
>  #define IDR1_PAGESIZE_MASK 0x8000
>  
> -typedef struct {
> -  UINTN FreqSystemBus;
> -} SYS_INFO;
> -
>  /* Device Configuration and Pin Control */
>  typedef struct {
>UINT8Res0[0x100-0x00];
> @@ -39,11 +35,6 @@ typedef struct {
>  #define CHASSIS2_RCWSR0_SYS_PLL_RAT_MASK  0x1f
>  } CCSR_GUR;
>  
> -VOID
> -GetSysInfo (
> -  OUT SYS_INFO *
> -  );
> -
>  UINT32
>  EFIAPI
>  GurRead (
> diff --git a/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c 
> b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c
> index 9d49d7b4748b..eb29cf0373cc 100644
> --- a/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c
> +++ b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c
> @@ -7,12 +7,7 @@
>  **/
>  
>  #include 
> -
> -extern
> -UINT64
> -GetBusFrequency (
> -  VOID
> -  );
> +#include 
>  
>  /**
>Return clock in for Uart IP
> @@ -23,5 +18,5 @@ BaseSerialPortGetClock (
>VOID
>)
>  {
> -  return GetBusFrequency ();
> +  return gPlatformGetClockPpi.PlatformGetClock (NXP_I2C_CLOCK, 0);
>  }
> diff --git a/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf 
> b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> index 9a3e80cf521d..c8840281763b 100644
> --- a/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> +++ b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> @@ -22,5 +22,5 @@ [Sources.common]
>SerialUartClockLib.c
>  
>  [LibraryClasses]
> -  SocLib
> +  ArmPlatformLib
>  
> diff --git a/Silicon/NXP/Library/SocLib/Chassis.c 
> b/Silicon/NXP/Library/SocLib/Chassis.c
> index 1ef99e8de25f..90677f0f36ca 100644
> --- 

Re: [edk2-devel] [PATCH v2 16/28] Platform/NXP/LS1043aRdbPkg: Add Clock retrieval APIs

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:31 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> The SOC takes primary clocking input from the external signal (a clock
> generator on board). The input (frequency) is multiplied using multiple
> phase locked loops (PLL) to create a variety of frequencies which can
> then be passed to a variety of internal logic, including cores and
> peripheral IP modules.
> 
> Therefore, move the clock retrieval APIs to Platform Lib.
> The Input clock is retrieved from board components in Platform Lib, and
> passed on to SOC Lib APIs to get the correct clock for an IP (after PLL
> multiplication).
> 
> Signed-off-by: Pankaj Bansal 
> ---
>  .../Library/ArmPlatformLib/ArmPlatformLib.c   | 51 ++
>  Silicon/NXP/Include/Library/SocLib.h  | 44 +++
>  Silicon/NXP/Include/Ppi/NxpPlatformGetClock.h | 53 +++
>  Silicon/NXP/LS1043A/Include/Soc.h | 11 
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c | 52 ++
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf  |  1 +
>  6 files changed, 212 insertions(+)
>  create mode 100644 Silicon/NXP/Include/Library/SocLib.h
>  create mode 100644 Silicon/NXP/Include/Ppi/NxpPlatformGetClock.h
> 
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index 718c71bf02eb..7f5872a78cfc 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -12,10 +12,60 @@
>  **/
>  
>  #include 
> +#include 
>  #include 
> +#include 
>  
>  extern VOID SocInit (VOID);
>  
> +/**
> +  Get the clocks supplied by Platform(Board) to NXP Layerscape SOC IPs
> +
> +  @param[in]  ClockType  Variable of Type NXP_IP_CLOCK. Indicates which IP 
> clock
> + is to be retrieved.
> +  @param[in]  ...Variable argument list which is parsed based on
> + ClockType. e.g. if the ClockType is NXP_I2C_CLOCK, 
> then
> + the second argument will be interpreted as 
> controller
> + number.
> + if ClockType is NXP_CORE_CLOCK, then second argument
> + is interpreted as cluster number and third argument 
> is
> + interpreted as core number (within the cluster)
> +
> +  @returnActual Clock Frequency. Return value 0 should be
> + interpreted as clock not being provided to IP.
> +**/
> +UINT64
> +EFIAPI
> +NxpPlatformGetClock(
> +  IN  UINT32  ClockType,
> +  ...
> +  )
> +{
> +  UINT64  Clock;
> +  VA_LIST Args;
> +
> +  Clock = 0;
> +
> +  VA_START (Args, ClockType);
> +
> +  switch (ClockType) {
> +  case NXP_SYSTEM_CLOCK:
> +Clock = 100 * 1000 * 1000; // 100 MHz
> +break;
> +  case NXP_I2C_CLOCK:
> +  case NXP_UART_CLOCK:
> +Clock = NxpPlatformGetClock (NXP_SYSTEM_CLOCK);
> +Clock = SocGetClock (Clock, ClockType, Args);
> +break;
> +  default:
> +break;
> +  }
> +
> +  VA_END (Args);
> +
> +  return Clock;
> +}
> +
>  /**
>Return the current Boot Mode
>  
> @@ -69,6 +119,7 @@ PrePeiCoreGetMpCoreInfo (
>  }
>  
>  ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };
> +NXP_PLATFORM_GET_CLOCK_PPI gPlatformGetClockPpi = { NxpPlatformGetClock };
>  
>  EFI_PEI_PPI_DESCRIPTOR  gPlatformPpiTable[] = {
>{
> diff --git a/Silicon/NXP/Include/Library/SocLib.h 
> b/Silicon/NXP/Include/Library/SocLib.h
> new file mode 100644
> index ..749aa230dec5
> --- /dev/null
> +++ b/Silicon/NXP/Include/Library/SocLib.h
> @@ -0,0 +1,44 @@
> +/** @file
> +
> +  Copyright 2020 NXP
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SOC_LIB_H__
> +#define SOC_LIB_H__
> +
> +#include 
> +#include 
> +
> +/**
> +  Return the input clock frequency to an IP Module.
> +  This function reads the RCW bits and calculates the  PLL multipler/divider
> +  values to be applied to various IP modules.
> +  If a module is disabled or doesn't exist on platform, then return zero.
> +
> +  @param[in]  BaseClock  Base clock to which PLL multipler/divider values is
> + to be applied.
> +  @param[in]  ClockType  Variable of Type NXP_IP_CLOCK. Indicates which IP 
> clock
> + is to be retrieved.
> +  @param[in]  Args   Variable argument list which is parsed based on
> + ClockType. e.g. if the ClockType is NXP_I2C_CLOCK, 
> then
> + the second argument will be interpreted as 
> controller
> + number. e.g. if there are four i2c controllers in 
> SOC,
> + then this value can be 0, 1, 2, 3
> + e.g. if ClockType is NXP_CORE_CLOCK, then second
> + argument is interpreted as 

Re: [edk2-devel] [PATCH v2 15/28] Silicon/NXP: Move RAM retrieval from SocLib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:30 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> RAM retrieval using SMC commands is common to all Layerscape SOCs.
> Therefore, move it to commom MemoryInit Pei Lib.
> 
> Signed-off-by: Pankaj Bansal 
> ---
>  Silicon/NXP/Include/DramInfo.h|  38 -
>  .../Library/MemoryInitPei/MemoryInitPeiLib.c  | 137 ++
>  .../Library/MemoryInitPei/MemoryInitPeiLib.h  |  25 
>  .../MemoryInitPei/MemoryInitPeiLib.inf|   7 +-
>  Silicon/NXP/Library/SocLib/Chassis.c  |  67 -
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf  |   1 -
>  6 files changed, 140 insertions(+), 135 deletions(-)
>  delete mode 100644 Silicon/NXP/Include/DramInfo.h
>  create mode 100644 Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> 
> diff --git a/Silicon/NXP/Include/DramInfo.h b/Silicon/NXP/Include/DramInfo.h
> deleted file mode 100644
> index a934aaeff1f5..
> --- a/Silicon/NXP/Include/DramInfo.h
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -/** @file
> -*  Header defining the structure for Dram Information
> -*
> -*  Copyright 2019 NXP
> -*
> -*  SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#ifndef DRAM_INFO_H_
> -#define DRAM_INFO_H_
> -
> -#include 
> -
> -#define SMC_DRAM_BANK_INFO  (0xC200FF12)
> -
> -typedef struct {
> -  UINTNBaseAddress;
> -  UINTNSize;
> -} DRAM_REGION_INFO;
> -
> -typedef struct {
> -  UINT32NumOfDrams;
> -  UINT32Reserved;
> -  DRAM_REGION_INFO  DramRegion[3];
> -} DRAM_INFO;
> -
> -EFI_STATUS
> -GetDramBankInfo (
> -  IN OUT DRAM_INFO *DramInfo
> -  );
> -
> -VOID
> -UpdateDpaaDram (
> -  IN OUT DRAM_INFO *DramInfo
> -  );
> -
> -#endif /* DRAM_INFO_H_ */
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c 
> b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> index 3ea773678667..54d026ef1270 100644
> --- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> @@ -17,8 +17,10 @@
>  #include 
>  #include 
>  #include 
> +#include 

Please insert alpabetically sorted.

> +
> +#include "MemoryInitPeiLib.h"
>  
> -#include 
>  
>  VOID
>  BuildMemoryTypeInformationHob (
> @@ -68,10 +70,17 @@ MemoryPeim (
>)
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> +  ARM_SMC_ARGS ArmSmcArgs;
> +  INT32Index;
> +  UINTNDramSize;
> +  UINTNBaseAddress;
> +  UINTNSize;
> +  UINTNTop;
> +  DRAM_REGION_INFO DramRegions[MAX_DRAM_REGIONS];
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  EFI_PEI_HOB_POINTERS NextHob;
> -  BOOLEAN  Found;
> -  DRAM_INFODramInfo;
> +  UINTNFdBase;
> +  UINTNFdTop;
> +  BOOLEAN  FoundSystemMem;
>  
>// Get Virtual Memory Map from the Platform Library
>ArmPlatformGetVirtualMemoryMap ();
> @@ -94,40 +103,112 @@ MemoryPeim (
>  EFI_RESOURCE_ATTRIBUTE_TESTED
>);
>  
> -  if (GetDramBankInfo ()) {
> -DEBUG ((DEBUG_ERROR, "Failed to get DRAM information, exiting...\n"));
> -return EFI_UNSUPPORTED;
> -  }
> -
> -  while (DramInfo.NumOfDrams--) {
> -//
> -// Check if the resource for the main system memory has been declared
> -//
> -Found = FALSE;
> -NextHob.Raw = GetHobList ();
> -while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, 
> NextHob.Raw)) != NULL) {
> -  if ((NextHob.ResourceDescriptor->ResourceType == 
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> -  (DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress >= 
> NextHob.ResourceDescriptor->PhysicalStart) &&
> -  (NextHob.ResourceDescriptor->PhysicalStart + 
> NextHob.ResourceDescriptor->ResourceLength <=
> -   DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress + 
> DramInfo.DramRegion[DramInfo.NumOfDrams].Size))
> -  {
> -Found = TRUE;
> -break;
> +  FoundSystemMem = FALSE;
> +  ZeroMem (DramRegions, sizeof (DramRegions));
> +
> +  Index = -1;
> +  do {
> +ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
> +ArmSmcArgs.Arg1 = Index++;
> +
> +ArmCallSmc ();
> +ASSERT (!(ArmSmcArgs.Arg0 && !Index));

This is being a bit too clever for its own good.
We're verifying that if one of the inputs to the function we just
called is zero (i.e. is the first time around the loop), the value
returned in the struct must also be zero?

Is this equivalent to
  if (Index == 0) {
ASSERT (ArmSmcArgs.Arg0 == SMC_WHATEVER_OK);
  }
?

If so, please use that form.

Don't use !Index when you mean Index == 0.

Regardless, a short explanation of the protocol and expected responses
are needed unless the code can be made more self-explanatory.

> +if (!Index) {
> +  DramSize = ArmSmcArgs.Arg1;
> +} else {
> +  if 

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

[Laszlo Ersek]

On 04/01/20 10:50, Ard Biesheuvel wrote:
> On Wed, 1 Apr 2020 at 10:37, Laszlo Ersek  wrote:
>>
>> On 04/01/20 00:17, Liran Alon wrote:
>>
>>> 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
>>
>> This is a very special case.
>>
>> Please see the justification in commit ad90df8ac018
>> ("ArmPlatformPkg/ArmVirtualizationPkg: Add private HobLib implementation
>> for DXE phase", 2014-09-18).
>>
>> The stock HobLib instance depends on DebugLib, for using the normal
>> ASSERT() macro.
>>
>> Furthermore, the stock serial-based DebugLib instance depends on
>> SerialPortLib, for printing messages.
>>
>> That produces a HobLib -> DebugLib -> SerialPortLib dependency chain.
>>
>> But, in case of this particular platform, our SerialPortLib instance
>> depends on HobLib, for retrieving the particulars of the serial port.
>> This creates a dependency cycle:
>>
>>   HobLib -> DebugLib -> SerialPortLib -> HobLib
>>
>> which makes the platform un-buildable.
>>
>> We had to break the dependency cycle somewhere, and the best (or maybe
>> only -- I don't recall exactly anymore) link to break was the HobLib ->
>> DebugLib dependency. We introduced our own HobLib instance, which (IIRC)
>> was almost identical to the stock one, except that its (only) DebugLib
>> dependency, namely the ASSERT(), was reimplemented with a plain
>> CpuDeadLoop(). And so the dependency chain ended up as:
>>
>>   DebugLib -> SerialPortLib -> HobLib
>>
>> Not circular any more.
>>
>>> and OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c)
>>
>> Very similar same case; here we even have a comment:
>>
>> //
>> // We can't use DebugLib due to a constructor dependency cycle between 
>> DebugLib
>> // and ourselves.
>> //
>>
>> The BaseDebugLibSerialPort instance depends on SerialPortLib, so a
>> SerialPortLib instance cannot "depend back" on DebugLib, in combination
>> with BaseDebugLibSerialPort.
>>
> 
> There's an additional problem in EDK2 that we never fixed, which is
> the fact that constructor dependencies are not transitive across
> library implementations that don't have a constructor themselves. For
> instance, in the following case
> 
> LibA (+)
>   depends on
> LibB (-)
>   depends on
> LibC (+)
> 
> where (+) means 'has constructor' and (-) means 'has no constructor',
> the EDK2 build tools may emit the LibA and LibC constructor
> invocations in any order. However, as soon as you try to fix this, we
> end up with circular dependencies all over the place, and none of the
> platforms can be built anymore.
> 

Yes, I've been aware of this; I've thought about it for a few minutes
just the other day. I was considering yet another "dynamic PCD setter"
NULL-class library instance, to be linked into various DXE modules.
Given the above problem, I figured if I ever started work on said lib
instance, I'd probably give it an empty CONSTRUCTOR -- not because that
function would have to do anything, but because the lib instance would
likely depend on other libraries, and *those* lib instances might have
important CONSTRUCTORs. And I wouldn't want this new lib instance to
break the dependency chain in the middle.

So in practice, all new lib instances seem to need CONSTRUCTORs now
(even if they are empty), just to avoid the problem you mention. And if
there is a build failure (circular dependency), try to deal with that on
a case by case basis.

(I admit that I didn't remember that we had tried fixing the problem in
BaseTools, and given up because of a multitude of sudden circular
dependencies! Thanks for the reminder!)

Thanks!
Laszlo


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

View/Reply Online (#56842): https://edk2.groups.io/g/devel/message/56842
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 v2] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Liran Alon]

On 01/04/2020 13:41, Laszlo Ersek wrote:

On 04/01/20 00:56, Liran Alon wrote:

  
+  //

+  // Setup rings against device
+  //
+  Status = PvScsiSetupRings (Dev);
+  if (EFI_ERROR (Status)) {
+goto FreeDMACommBuffer;
I'm going to rename this label to "FreeDmaCommBuffer" upon pushing (due
to de-capitalization of acronyms in edk2 CamelCase).


Thanks. Will note this for next time.

-Liran



Reviewed-by: Laszlo Ersek 

Thank you!
Laszlo



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

View/Reply Online (#56841): https://edk2.groups.io/g/devel/message/56841
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

[Laszlo Ersek]

On 04/01/20 00:54, Sean via Groups.Io wrote:
> 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.

Good points!

(1) There is a big difference between physical firmware and virtual
firmware. Similar difference between physical hardware and virtual
hardware. With virtualization, the "return/refund" case maps to "fix
OVMF, or fix QEMU". In particular the "fix QEMU" part is important,
because the assertion failure would show that QEMU is really broken, and
needs fixing. Thankfully, QEMU is fixable, and in that case, it *should*
be fixed; the "return/refund" path is both the right one, and not as
painful as with physical devices.

To exaggerate your point a bit, I would agree that ASSERT()s have no
place in space-craft software! :)

(2) I have actually considered -- at length -- returning an error code
from this function, instead of the failed assert / CpuDeadLoop(). The
error code from this function would be propagated out through the ext
scsi PassThru method. For that reason, I reviewed the error codes
defined for that protocol member function in the UEFI spec. All error
codes except EFI_TIMEOUT claim that "the SCSI operation was not sent /
not executed", therefore they do not apply in this case -- per spec, we
could only return EFI_TIMEOUT in case we fail to understand the PVSCSI
device model's response (because the device breaks "data sheet
compliance"). While I do think EFI_TIMEOUT is a *good* error code in
this case ultimately -- it shows that we simply can't tell whatever
happened on the device's side --, I figured (given point (1)) it would
be a very unwelcome complication for the code. Because upon returning
EFI_TIMEOUT, we also have to set HostAdapterStatus / TargetStatus to
something sensible (i.e., "fake"), and SenseDataLength to zero. Doable,
but didn't seem worth the churn.

(3) In RHEL OVMF packages, I exclusively ship DEBUG builds. For two
reasons: (a) many places in edk2 core code still use ASSERT() in place
of run-of-the-mill error checking (unfortunately), i.e., not explicit
"if"s; (b) bugs in software are the norm, not the exception, therefore
IMO debugging features should be an unalienable part of every software
package ever shipped to users. (If I could by my consumer electronics /
gadgets with full debug code enabled, I would, too.) When a RHEL user
reports an error, I don't want to start every one of my analyses with
the question, "can you please reproduce this with a debug build first".

I'm aware that debug code has additional cost (larger code size, perhaps
more CPU cycles spent, which is especially important on mobile devices,
yes). In my use case, the slowdown is not egregious -- first, most of
the boot time impact due to DEBUGs can be attributed to QEMU IO port
accesses, and that port is dynamically configurable (disabled by
default), so if it's not enabled, there's virtually no impact; second,
there don't seem to be insanely costly invariant checks built into OVMF
for the DEBUG target either. So I'm consciously committed to shipping
DEBUG only, and I admit that it does bias my thinking about RELEASE --
not that I'm against RELEASE *in general* for edk2, or ignoring RELEASE
willfully, it's just that I likely have blind spots wrt. RELEASE builds.

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

Thanks a lot for confirming! In this case I'll go ahead with this patch.

Thanks!
Laszlo


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

View/Reply Online (#56840): https://edk2.groups.io/g/devel/message/56840
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 v2] OvmfPkg/PvScsiDxe: Refactor setup of rings to separate function

[Laszlo Ersek]

On 04/01/20 00:56, 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 | 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;

I'm going to rename this label to "FreeDmaCommBuffer" upon pushing (due
to de-capitalization of acronyms in edk2 CamelCase).

Reviewed-by: Laszlo Ersek 

Thank you!
Laszlo

> +  }
> +
>//
>// 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.
> -  // 

Re: [edk2-devel] [PATCH v2 14/28] Platform/NXP: rename the ArmPlatformLib as per ArmPlatformPkg

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:29 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Keep the names and location of files as mentioned in ArmPlatformPkg.
> This helps in porting the common changes (if any in future) easily.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc  | 2 +-
>  .../AArch64/ArmPlatformHelper.S}  | 2 +-
>  .../Library/{PlatformLib => ArmPlatformLib}/ArmPlatformLib.c  | 2 +-
>  .../{PlatformLib => ArmPlatformLib}/ArmPlatformLib.inf| 4 ++--
>  .../NxpQoriqLsMem.c => ArmPlatformLib/ArmPlatformLibMem.c}| 0
>  5 files changed, 5 insertions(+), 5 deletions(-)
>  rename Platform/NXP/LS1043aRdbPkg/Library/{PlatformLib/NxpQoriqLsHelper.S => 
> ArmPlatformLib/AArch64/ArmPlatformHelper.S} (88%)
>  rename Platform/NXP/LS1043aRdbPkg/Library/{PlatformLib => 
> ArmPlatformLib}/ArmPlatformLib.c (93%)
>  rename Platform/NXP/LS1043aRdbPkg/Library/{PlatformLib => 
> ArmPlatformLib}/ArmPlatformLib.inf (89%)
>  rename Platform/NXP/LS1043aRdbPkg/Library/{PlatformLib/NxpQoriqLsMem.c => 
> ArmPlatformLib/ArmPlatformLibMem.c} (100%)
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc 
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index 1975f2c4c52c..e5383aaf0cc5 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -26,7 +26,7 @@ [Defines]
>  !include Silicon/NXP/LS1043A/LS1043A.dsc.inc
>  
>  [LibraryClasses.common]
> -  
> ArmPlatformLib|Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> +  
> ArmPlatformLib|Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
>RealTimeClockLib|Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf
>  
>  [PcdsFixedAtBuild.common]
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> similarity index 88%
> rename from Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
> rename to 
> Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> index 84ee8c9f9700..dfbf73675a2d 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
> +++ 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> @@ -1,7 +1,7 @@
>  #  @file
>  #
>  #  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> -#  Copyright 2017 NXP
> +#  Copyright 2017, 2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> similarity index 93%
> rename from Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
> rename to Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index eac7d4aa4e47..718c71bf02eb 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -6,7 +6,7 @@
>  *
>  *  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
>  *  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> -*  Copyright 2017 NXP
> +*  Copyright 2017, 2020 NXP
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
>  **/
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> similarity index 89%
> rename from Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> rename to Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 7563a1c43630..7a43ad86d183 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -27,8 +27,8 @@ [LibraryClasses]
>SocLib
>  
>  [Sources.common]
> -  NxpQoriqLsHelper.S| GCC
> -  NxpQoriqLsMem.c
> +  AArch64/ArmPlatformHelper.S| GCC
> +  ArmPlatformLibMem.c
>ArmPlatformLib.c
>  
>  [Ppis]
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c 
> b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> similarity index 100%
> rename from Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> rename to 
> Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> -- 
> 2.17.1
> 

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

View/Reply Online (#56838): https://edk2.groups.io/g/devel/message/56838
Mute This Topic: https://groups.io/mt/72077429/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 13/28] Silicon/NXP: Move dsc file

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:28 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> As per convention being followed in edk2-platforms, keep the dec
> file and dsc file together.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc | 2 +-
>  {Platform => Silicon}/NXP/NxpQoriqLs.dsc.inc | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename {Platform => Silicon}/NXP/NxpQoriqLs.dsc.inc (100%)
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc 
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index 385b6e067e26..1975f2c4c52c 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -22,7 +22,7 @@ [Defines]
>OUTPUT_DIRECTORY   = Build/LS1043aRdbPkg
>FLASH_DEFINITION   = 
> Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>  
> -!include Platform/NXP/NxpQoriqLs.dsc.inc
> +!include Silicon/NXP/NxpQoriqLs.dsc.inc
>  !include Silicon/NXP/LS1043A/LS1043A.dsc.inc
>  
>  [LibraryClasses.common]
> diff --git a/Platform/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
> similarity index 100%
> rename from Platform/NXP/NxpQoriqLs.dsc.inc
> rename to Silicon/NXP/NxpQoriqLs.dsc.inc
> -- 
> 2.17.1
> 

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

View/Reply Online (#56837): https://edk2.groups.io/g/devel/message/56837
Mute This Topic: https://groups.io/mt/72077436/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 12/28] Silicon/NXP: Remove unnecessary PCDs

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:27 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> The memory map of an SOC is fixed in hardware. it doesn't change with
> platform that uses SOC. So, there is no need to keep PCDs for these values
> and we can use macros for these in SOC header file.
> 
> Any Platform using the SOC, can make use of the SOC header file.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  .../Drivers/PlatformDxe/PlatformDxe.c | 15 ++--
>  .../Drivers/PlatformDxe/PlatformDxe.inf   | 10 +--
>  .../Library/PlatformLib/ArmPlatformLib.inf| 21 +
>  .../Library/PlatformLib/NxpQoriqLsMem.c   | 79 +--
>  Silicon/NXP/Include/Chassis2/NxpSoc.h |  2 +
>  Silicon/NXP/LS1043A/Include/Soc.h | 44 +++
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc   | 26 --
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c |  2 +-
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf  |  1 -
>  Silicon/NXP/NxpQoriqLs.dec| 47 ---
>  10 files changed, 97 insertions(+), 150 deletions(-)
>  create mode 100644 Silicon/NXP/LS1043A/Include/Soc.h
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c 
> b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> index f89dcdeff3c1..62c400eb1a58 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -1,7 +1,7 @@
>  /** @file
>LS1043 DXE platform driver.
>  
> -  Copyright 2018-2019 NXP
> +  Copyright 2018-2020 NXP
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -22,7 +23,7 @@ typedef struct {
>UINT8 EndDesc;
>  } ADDRESS_SPACE_DESCRIPTOR;
>  
> -STATIC ADDRESS_SPACE_DESCRIPTOR mI2cDesc[FixedPcdGet64 
> (PcdNumI2cController)];
> +STATIC ADDRESS_SPACE_DESCRIPTOR mI2cDesc[LS1043A_I2C_NUM_CONTROLLERS];
>  
>  STATIC
>  EFI_STATUS
> @@ -65,19 +66,19 @@ PopulateI2cInformation (
>  {
>UINT32 Index;
>  
> -  for (Index = 0; Index < FixedPcdGet32 (PcdNumI2cController); Index++) {
> +  for (Index = 0; Index < ARRAY_SIZE (mI2cDesc); Index++) {
>  mI2cDesc[Index].StartDesc.Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
>  mI2cDesc[Index].StartDesc.Len = sizeof 
> (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
>  mI2cDesc[Index].StartDesc.ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
>  mI2cDesc[Index].StartDesc.GenFlag = 0;
>  mI2cDesc[Index].StartDesc.SpecificFlag = 0;
>  mI2cDesc[Index].StartDesc.AddrSpaceGranularity = 32;
> -mI2cDesc[Index].StartDesc.AddrRangeMin = FixedPcdGet64 (PcdI2c0BaseAddr) 
> +
> - (Index * FixedPcdGet32 
> (PcdI2cSize));
> +mI2cDesc[Index].StartDesc.AddrRangeMin = LS1043A_I2C0_PHYS_ADDRESS +
> + (Index * LS1043A_I2C_SIZE);
>  mI2cDesc[Index].StartDesc.AddrRangeMax = 
> mI2cDesc[Index].StartDesc.AddrRangeMin +
> - FixedPcdGet32 (PcdI2cSize) - 1;
> + LS1043A_I2C_SIZE - 1;
>  mI2cDesc[Index].StartDesc.AddrTranslationOffset = 0;
> -mI2cDesc[Index].StartDesc.AddrLen = FixedPcdGet32 (PcdI2cSize);
> +mI2cDesc[Index].StartDesc.AddrLen = LS1043A_I2C_SIZE;
>  
>  mI2cDesc[Index].EndDesc = ACPI_END_TAG_DESCRIPTOR;
>}
> diff --git a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf 
> b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index d689cf4db58e..038d48949a39 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -2,7 +2,7 @@
>  #
>  #  Component description file for LS1043 DXE platform driver.
>  #
> -#  Copyright 2018-2019 NXP
> +#  Copyright 2018-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -21,9 +21,10 @@ [Sources]
>  
>  [Packages]
>ArmPkg/ArmPkg.dec
> -  MdePkg/MdePkg.dec
>MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
>Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.dec
> +  Silicon/NXP/LS1043A/LS1043A.dec
>Silicon/NXP/NxpQoriqLs.dec
>  
>  [LibraryClasses]
> @@ -43,10 +44,5 @@ [Protocols]
>gEdkiiNonDiscoverableDeviceProtocolGuid## PRODUCES
>gDs1307RealTimeClockLibI2cMasterProtocolGuid   ## PRODUCES
>  
> -[FixedPcd]
> -  gNxpQoriqLsTokenSpaceGuid.PcdI2c0BaseAddr
> -  gNxpQoriqLsTokenSpaceGuid.PcdI2cSize
> -  gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController
> -
>  [Depex]
>TRUE
> diff --git 
> a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> index f7ae74afc6ca..7563a1c43630 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> +++ 

Re: [edk2-devel] [PATCH v2 11/28] Silicon/NXP: remove not needed components

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:26 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> The structures elements and functions that are not necessary for booting
> for now are being deleted.
> Once the directory structure has been changed (i.e. we have clear
> distinction between chassis code and header files and SOC code and header
> files), we will put back the code and
> structure components back at their appropriate respective place.
> 
> Also right now all the elements are being defined in structures, which are
> not being used right now. So to simplify the code restructuring, I have
> removed those for now. When we need to use those elements, we can define
> those one by one.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc |   4 -
>  Silicon/NXP/Include/Chassis2/LsSerDes.h  |  62 
>  Silicon/NXP/Include/Chassis2/NxpSoc.h| 314 +--
>  Silicon/NXP/LS1043A/Include/SocSerDes.h  |  51 ---
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc  |   6 -
>  Silicon/NXP/Library/SocLib/Chassis.c | 220 -
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c|  79 -
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf |   7 +-
>  Silicon/NXP/Library/SocLib/NxpChassis.h  |  90 --
>  Silicon/NXP/Library/SocLib/SerDes.c  | 268 
>  Silicon/NXP/NxpQoriqLs.dec   |  27 --
>  11 files changed, 3 insertions(+), 1125 deletions(-)
>  delete mode 100644 Silicon/NXP/Include/Chassis2/LsSerDes.h
>  delete mode 100644 Silicon/NXP/LS1043A/Include/SocSerDes.h
>  delete mode 100644 Silicon/NXP/Library/SocLib/SerDes.c
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc 
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index 802cccdce63b..385b6e067e26 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -39,10 +39,6 @@ [PcdsFixedAtBuild.common]
>gArmTokenSpaceGuid.PcdSystemMemorySize|0x7BE0
>gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0200
>  
> -  #
> -  # Board Specific Pcds
> -  #
> -  gNxpQoriqLsTokenSpaceGuid.PcdSerdes2Enabled|FALSE
>gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x1
>  
>#
> diff --git a/Silicon/NXP/Include/Chassis2/LsSerDes.h 
> b/Silicon/NXP/Include/Chassis2/LsSerDes.h
> deleted file mode 100644
> index 9afbc522398a..
> --- a/Silicon/NXP/Include/Chassis2/LsSerDes.h
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -/** LsSerDes.h
> - The Header file of SerDes Module for Chassis 2
> -
> - Copyright 2017-2019 NXP
> -
> - SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#ifndef LS_SERDES_H_
> -#define LS_SERDES_H_
> -
> -#include 
> -
> -#define SRDS_MAX_LANES 4
> -
> -typedef enum {
> -  None = 0,
> -  Pcie1,
> -  Pcie2,
> -  Pcie3,
> -  Sata,
> -  SgmiiFm1Dtsec1,
> -  SgmiiFm1Dtsec2,
> -  SgmiiFm1Dtsec5,
> -  SgmiiFm1Dtsec6,
> -  SgmiiFm1Dtsec9,
> -  SgmiiFm1Dtsec10,
> -  QsgmiiFm1A,
> -  XfiFm1Mac9,
> -  XfiFm1Mac10,
> -  Sgmii2500Fm1Dtsec2,
> -  Sgmii2500Fm1Dtsec5,
> -  Sgmii2500Fm1Dtsec9,
> -  Sgmii2500Fm1Dtsec10,
> -  SerdesPrtclCount
> -} SERDES_PROTOCOL;
> -
> -typedef enum {
> -  Srds1  = 0,
> -  Srds2,
> -  SrdsMaxNum
> -} SERDES_NUMBER;
> -
> -typedef struct {
> -  UINT16 Protocol;
> -  UINT8  SrdsLane[SRDS_MAX_LANES];
> -} SERDES_CONFIG;
> -
> -typedef VOID
> -(*SERDES_PROBE_LANES_CALLBACK) (
> -  IN SERDES_PROTOCOL LaneProtocol,
> -  IN VOID *Arg
> -  );
> -
> -VOID
> -SerDesProbeLanes(
> -  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> -  IN VOID *Arg
> -  );
> -
> -#endif /* LS_SERDES_H_ */
> diff --git a/Silicon/NXP/Include/Chassis2/NxpSoc.h 
> b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> index f05a813750e8..74330b6205e7 100644
> --- a/Silicon/NXP/Include/Chassis2/NxpSoc.h
> +++ b/Silicon/NXP/Include/Chassis2/NxpSoc.h
> @@ -1,7 +1,7 @@
>  /** Soc.h
>  *  Header defining the Base addresses, sizes, flags etc for chassis 1
>  *
> -*  Copyright 2017-2019 NXP
> +*  Copyright 2017-2020 NXP
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -10,22 +10,7 @@
>  #ifndef NXP_SOC_H_
>  #define NXP_SOC_H_
>  
> -#define HWA_CGA_M1_CLK_SEL 0xe000
> -#define HWA_CGA_M1_CLK_SHIFT   29
> -
> -#define TP_CLUSTER_EOC_MASK0xc000  /* end of clusters mask */
> -#define NUM_CC_PLLS2
>  #define CLK_FREQ   1
> -#define MAX_CPUS   4
> -#define NUM_FMAN   1
> -#define CHECK_CLUSTER(Cluster)((Cluster & TP_CLUSTER_EOC_MASK) == 0x0)
> -
> -/* RCW SERDES MACRO */
> -#define RCWSR_INDEX4
> -#define RCWSR_SRDS1_PRTCL_MASK 0x
> -#define RCWSR_SRDS1_PRTCL_SHIFT16
> -#define RCWSR_SRDS2_PRTCL_MASK 0x
> -#define RCWSR_SRDS2_PRTCL_SHIFT0
>  
>  /* SMMU Defintions */
>  #define SMMU_BASE_ADDR 0x0900
> @@ -41,312 +26,17 @@
>  #define IDR1_PAGESIZE_MASK 

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

[Maciej Rabeda]

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 
Reviewed-by: Siyuan Fu 
Acked-by: Zhichao Gao 
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

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);
 }
 
 /**
-- 
2.24.0.windows.2


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

View/Reply Online (#56834): https://edk2.groups.io/g/devel/message/56834
Mute This Topic: https://groups.io/mt/72696861/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 10/28] Silicon/NXP: remove print information from Soc lib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:25 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> The Soc info being printed can be removed from SOC lib.
> We are in the process of implementing PEI Phase.
> After PEI phase impelmentation this info would be printed in
  implementation
> common PEIM based on the information retrieved from PPIs.
> e.g. gArmMpCoreInfoPpiGuid can be used to print cluser and
  cluster
> core info.
> 
> Signed-off-by: Pankaj Bansal 

With the commit message typos fixed:
Reviewed-by: Leif Lindholm 

> ---
>  Silicon/NXP/Library/SocLib/Chassis.c  | 132 --
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c |  16 +--
>  Silicon/NXP/Library/SocLib/NxpChassis.h   |  26 +
>  3 files changed, 2 insertions(+), 172 deletions(-)
> 
> diff --git a/Silicon/NXP/Library/SocLib/Chassis.c 
> b/Silicon/NXP/Library/SocLib/Chassis.c
> index b8a8118c5e24..2f192e890bcf 100644
> --- a/Silicon/NXP/Library/SocLib/Chassis.c
> +++ b/Silicon/NXP/Library/SocLib/Chassis.c
> @@ -216,67 +216,6 @@ CpuMaskNext (
>return Cpu;
>  }
>  
> -/*
> - * Print CPU information
> - */
> -VOID
> -PrintCpuInfo (
> -  VOID
> -  )
> -{
> -  SYS_INFO SysInfo;
> -  UINTNCoreIndex;
> -  UINTNCore;
> -  UINT32   Type;
> -  UINT32   NumCpus;
> -  UINT32   Mask;
> -  CHAR8*CoreName;
> -
> -  GetSysInfo ();
> -  DEBUG ((DEBUG_INIT, "Clock Configuration:"));
> -
> -  NumCpus = CpuNumCores ();
> -  Mask = CpuMask ();
> -
> -  for (CoreIndex = 0, Core = CpuMaskNext(-1, Mask);
> -   CoreIndex < NumCpus;
> -   CoreIndex++, Core = CpuMaskNext(Core, Mask))
> -  {
> -if (!(CoreIndex % 3)) {
> -  DEBUG ((DEBUG_INIT, "\n  "));
> -}
> -
> -Type = TP_ITYP_VERSION (QoriqCoreToType (Core));
> -switch (Type) {
> -  case TY_ITYP_VERSION_A7:
> -CoreName = "A7";
> -break;
> -  case TY_ITYP_VERSION_A53:
> -CoreName = "A53";
> -break;
> -  case TY_ITYP_VERSION_A57:
> -CoreName = "A57";
> -break;
> -  case TY_ITYP_VERSION_A72:
> -CoreName = "A72";
> -break;
> -  default:
> -CoreName = " Unknown Core ";
> -}
> -DEBUG ((DEBUG_INIT, "CPU%d(%a):%-4d MHz  ",
> -  Core, CoreName, SysInfo.FreqProcessor[Core] / MHZ));
> -  }
> -
> -  DEBUG ((DEBUG_INIT, "\n  Bus:  %-4d MHz  ", SysInfo.FreqSystemBus 
> / MHZ));
> -  DEBUG ((DEBUG_INIT, "DDR:  %-4d MT/s", SysInfo.FreqDdrBus / MHZ));
> -
> -  if (SysInfo.FreqFman[0] != 0) {
> -DEBUG ((DEBUG_INIT, "\n  FMAN: %-4d MHz  ",  SysInfo.FreqFman[0] 
> / MHZ));
> -  }
> -
> -  DEBUG ((DEBUG_INIT, "\n"));
> -}
> -
>  /*
>   * Return system bus frequency
>   */
> @@ -307,77 +246,6 @@ GetSdxcFrequency (
>return SocSysInfo.FreqSdhc;
>  }
>  
> -/*
> - * Print Soc information
> - */
> -VOID
> -PrintSoc (
> -  VOID
> -  )
> -{
> -  CHAR8Buf[20];
> -  CCSR_GUR *GurBase;
> -  UINTNCount;
> -  //
> -  // Svr : System Version Register
> -  //
> -  UINTNSvr;
> -  UINTNVer;
> -
> -  GurBase = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> -
> -  Svr = GurRead ((UINTN)>Svr);
> -  Ver = SVR_SOC_VER (Svr);
> -
> -  for (Count = 0; Count < ARRAY_SIZE (mCpuTypeList); Count++) {
> -if ((mCpuTypeList[Count].SocVer & SVR_WO_E) == Ver) {
> -  AsciiStrCpyS (Buf, sizeof (Buf), mCpuTypeList[Count].Name);
> -
> -  if (IS_E_PROCESSOR (Svr)) {
> -AsciiStrCatS (Buf, sizeof (Buf), "E");
> -  }
> -  break;
> -}
> -  }
> -
> -  DEBUG ((DEBUG_INFO, "SoC: %a (0x%x); Rev %d.%d\n",
> -  Buf, Svr, SVR_MAJOR (Svr), SVR_MINOR (Svr)));
> -
> -  return;
> -}
> -
> -/*
> - * Dump RCW (Reset Control Word) on console
> - */
> -VOID
> -PrintRCW (
> -  VOID
> -  )
> -{
> -  CCSR_GUR *Base;
> -  UINTNCount;
> -
> -  Base = (VOID *)PcdGet64 (PcdGutsBaseAddr);
> -
> -  /*
> -   * Display the RCW, so that no one gets confused as to what RCW
> -   * we're actually using for this boot.
> -   */
> -
> -  DEBUG ((DEBUG_INIT, "Reset Configuration Word (RCW):"));
> -  for (Count = 0; Count < ARRAY_SIZE (Base->RcwSr); Count++) {
> -UINT32 Rcw = SwapMmioRead32 ((UINTN)>RcwSr[Count]);
> -
> -if ((Count % 4) == 0) {
> -  DEBUG ((DEBUG_INIT, "\n  %08x:", Count * 4));
> -}
> -
> -DEBUG ((DEBUG_INIT, " %08x", Rcw));
> -  }
> -
> -  DEBUG ((DEBUG_INIT, "\n"));
> -}
> -
>  /*
>   * Setup SMMU in bypass mode
>   * and also set its pagesize
> diff --git a/Silicon/NXP/Library/SocLib/Chassis2/Soc.c 
> b/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> index bfb8b8cb339a..687a1d940066 100644
> --- a/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> +++ b/Silicon/NXP/Library/SocLib/Chassis2/Soc.c
> @@ -1,7 +1,7 @@
>  /** @Soc.c
>SoC specific Library containg functions to initialize various SoC 
> components
>  
> -  Copyright 2017-2019 NXP
> +  Copyright 2017-2020 NXP
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -131,10 +131,6 @@ GetSysInfo (
>  
>  /**
>  

Re: [edk2-devel] [PATCH v2 08/28] Silicon/NXP/LS1043A: Use BaseSerialPortLib16550 as SerialPortLib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:23 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> UART Programming model in LS1043A is compatible with PC16550D.
> Therefore, BaseSerialPortLib16550 can be used instead of our own
> implementation of SerialPortLib.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 
(If the need for SerialUartClockLib goes away, that line can be
dropped without re-review being required.)

> ---
>  Platform/NXP/NxpQoriqLs.dsc.inc | 3 +++
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc | 5 -
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/NXP/NxpQoriqLs.dsc.inc b/Platform/NXP/NxpQoriqLs.dsc.inc
> index 94d3e53a046b..234a5e2707cd 100644
> --- a/Platform/NXP/NxpQoriqLs.dsc.inc
> +++ b/Platform/NXP/NxpQoriqLs.dsc.inc
> @@ -98,6 +98,9 @@ [LibraryClasses.common]
>
> ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
>IoAccessLib|Silicon/NXP/Library/IoAccessLib/IoAccessLib.inf
>  
> +  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
> +
>  [LibraryClasses.common.SEC]
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>
> UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc 
> b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> index d2d4133428c3..f6f15a482a85 100644
> --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> @@ -8,8 +8,10 @@
>  #
>  
>  [LibraryClasses.common]
> +  
> PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
>SocLib|Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> -  SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
> +  
> SerialUartClockLib|Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> +  
> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>  
>  
> 
>  #
> @@ -25,6 +27,7 @@ [PcdsDynamicDefault.common]
>  
>  [PcdsFixedAtBuild.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x021c0500
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
>  
>#
># CCSR Address Space and other attached Memories
> -- 
> 2.17.1
> 

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

View/Reply Online (#56831): https://edk2.groups.io/g/devel/message/56831
Mute This Topic: https://groups.io/mt/72077433/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 09/28] Silicon/NXP: Drop DUartPortLib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:24 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> DUartPortLib is for ns16550 compatible UART in LS1043A. Therefore, we can
> remove it and use BaseSerialPortLib16550.
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/NXP/Library/DUartPortLib/DUart.h  | 122 --
>  .../NXP/Library/DUartPortLib/DUartPortLib.c   | 364 --
>  .../NXP/Library/DUartPortLib/DUartPortLib.inf |  34 --
>  3 files changed, 520 deletions(-)
>  delete mode 100644 Silicon/NXP/Library/DUartPortLib/DUart.h
>  delete mode 100644 Silicon/NXP/Library/DUartPortLib/DUartPortLib.c
>  delete mode 100644 Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
> 
> diff --git a/Silicon/NXP/Library/DUartPortLib/DUart.h 
> b/Silicon/NXP/Library/DUartPortLib/DUart.h
> deleted file mode 100644
> index c71e2ce55d1d..
> --- a/Silicon/NXP/Library/DUartPortLib/DUart.h
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -/** DUart.h
> -*  Header defining the DUART constants (Base addresses, sizes, flags)
> -*
> -*  Based on Serial I/O Port library headers available in PL011Uart.h
> -*
> -*  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
> -*  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> -*  Copyright 2017 NXP
> -*
> -*  SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#ifndef DUART_H_
> -#define DUART_H_
> -
> -// FIFO Control Register
> -#define DUART_FCR_FIFO_EN  0x01 /* Fifo enable */
> -#define DUART_FCR_CLEAR_RCVR   0x02 /* Clear the RCVR FIFO */
> -#define DUART_FCR_CLEAR_XMIT   0x04 /* Clear the XMIT FIFO */
> -#define DUART_FCR_DMA_SELECT   0x08 /* For DMA applications */
> -#define DUART_FCR_TRIGGER_MASK 0xC0 /* Mask for the FIFO trigger range */
> -#define DUART_FCR_TRIGGER_10x00 /* Mask for trigger set at 1 */
> -#define DUART_FCR_TRIGGER_40x40 /* Mask for trigger set at 4 */
> -#define DUART_FCR_TRIGGER_80x80 /* Mask for trigger set at 8 */
> -#define DUART_FCR_TRIGGER_14   0xC0 /* Mask for trigger set at 14 */
> -#define DUART_FCR_RXSR 0x02 /* Receiver soft reset */
> -#define DUART_FCR_TXSR 0x04 /* Transmitter soft reset */
> -
> -// Modem Control Register
> -#define DUART_MCR_DTR  0x01 /* Reserved  */
> -#define DUART_MCR_RTS  0x02 /* RTS   */
> -#define DUART_MCR_OUT1 0x04 /* Reserved */
> -#define DUART_MCR_OUT2 0x08 /* Reserved */
> -#define DUART_MCR_LOOP 0x10 /* Enable loopback test mode */
> -#define DUART_MCR_AFE  0x20 /* AFE (Auto Flow Control) */
> -#define DUART_MCR_DMA_EN   0x04
> -#define DUART_MCR_TX_DFR   0x08
> -
> -// Line Control Register
> -/*
> -* Note: if the word length is 5 bits (DUART_LCR_WLEN5), then setting
> -* DUART_LCR_STOP will select 1.5 stop bits, not 2 stop bits.
> -*/
> -#define DUART_LCR_WLS_MSK  0x03 /* character length select mask */
> -#define DUART_LCR_WLS_50x00 /* 5 bit character length */
> -#define DUART_LCR_WLS_60x01 /* 6 bit character length */
> -#define DUART_LCR_WLS_70x02 /* 7 bit character length */
> -#define DUART_LCR_WLS_80x03 /* 8 bit character length */
> -#define DUART_LCR_STB  0x04 /* # stop Bits, off=1, on=1.5 or 2) 
> */
> -#define DUART_LCR_PEN  0x08 /* Parity eneble */
> -#define DUART_LCR_EPS  0x10 /* Even Parity Select */
> -#define DUART_LCR_STKP 0x20 /* Stick Parity */
> -#define DUART_LCR_SBRK 0x40 /* Set Break */
> -#define DUART_LCR_BKSE 0x80 /* Bank select enable */
> -#define DUART_LCR_DLAB 0x80 /* Divisor latch access bit */
> -
> -// Line Status Register
> -#define DUART_LSR_DR   0x01 /* Data ready */
> -#define DUART_LSR_OE   0x02 /* Overrun */
> -#define DUART_LSR_PE   0x04 /* Parity error */
> -#define DUART_LSR_FE   0x08 /* Framing error */
> -#define DUART_LSR_BI   0x10 /* Break */
> -#define DUART_LSR_THRE 0x20 /* Xmit holding register empty */
> -#define DUART_LSR_TEMT 0x40 /* Xmitter empty */
> -#define DUART_LSR_ERR  0x80 /* Error */
> -
> -// Modem Status Register
> -#define DUART_MSR_DCTS 0x01 /* Delta CTS */
> -#define DUART_MSR_DDSR 0x02 /* Reserved */
> -#define DUART_MSR_TERI 0x04 /* Reserved */
> -#define DUART_MSR_DDCD 0x08 /* Reserved */
> -#define DUART_MSR_CTS  0x10 /* Clear to Send */
> -#define DUART_MSR_DSR  0x20 /* Reserved */
> -#define DUART_MSR_RI   0x40 /* Reserved */
> -#define DUART_MSR_DCD  0x80 /* Reserved */
> -
> -// Interrupt Identification Register
> -#define DUART_IIR_NO_INT   0x01 /* No interrupts pending */
> -#define DUART_IIR_ID   0x06 /* Mask for the interrupt ID */
> -#define 

Re: [edk2-devel] [PATCH v2 07/28] Silicon/NXP: Implement SerialUartClockLib

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:22 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Implement SerialUartClockLib for all NXP Layerscape Platforms.
> 
> Signed-off-by: Pankaj Bansal 
> ---
>  .../SerialUartClockLib/SerialUartClockLib.c   | 27 +++
>  .../SerialUartClockLib/SerialUartClockLib.inf | 26 ++

I requested after the initial submission that you "either follow the
manual git setup steps from
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers
or execute edk2/BaseTools/Scripts/SetupGit.py in each of the tianocore
repositories"

Now, neither appears to have happened, as .c still comes before .inf
and paths are still truncated.

But in addition to that, we realised that git happily ignores settings
for --stat. So, please, execute aforementioned script, but then
generate v3 with --stat=1000 --stat-graph-width=20.

For this particular patch, the side discussion on the edk2 portion may
make it redundant, so I'm deferring review.

/
Leif

>  2 files changed, 53 insertions(+)
>  create mode 100644 
> Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c
>  create mode 100644 
> Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> 
> diff --git a/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c 
> b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c
> new file mode 100644
> index ..9d49d7b4748b
> --- /dev/null
> +++ b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c
> @@ -0,0 +1,27 @@
> +/** @file
> +*
> +*  Copyright 2020 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include 
> +
> +extern
> +UINT64
> +GetBusFrequency (
> +  VOID
> +  );
> +
> +/**
> +  Return clock in for Uart IP
> +**/
> +UINT32
> +EFIAPI
> +BaseSerialPortGetClock (
> +  VOID
> +  )
> +{
> +  return GetBusFrequency ();
> +}
> diff --git a/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf 
> b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> new file mode 100644
> index ..9a3e80cf521d
> --- /dev/null
> +++ b/Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf
> @@ -0,0 +1,26 @@
> +#  @file
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +
> +[Defines]
> +  INF_VERSION= 1.27
> +  BASE_NAME  = SerialUartClockLib
> +  FILE_GUID  = fa65495e-d3c8-4ea3-9737-994e9ccbaf11
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = SerialUartClockLib
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[Sources.common]
> +  SerialUartClockLib.c
> +
> +[LibraryClasses]
> +  SocLib
> +
> -- 
> 2.17.1
> 

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

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

[edk2-devel] [PATCH] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()

[Maciej Rabeda]

This patch fixes reversed logic of recently added ASSERTs which should
ensure that Ip6IsNDOptionValid() implementation properly reacts to invalid
packets.

Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Signed-off-by: Maciej Rabeda 
Tested-by: Laszlo Ersek 
---
 NetworkPkg/Ip6Dxe/Ip6Nd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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
-- 
2.24.0.windows.2


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

View/Reply Online (#56829): https://edk2.groups.io/g/devel/message/56829
Mute This Topic: https://groups.io/mt/72696739/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 06/28] NXP/LS1043aRdb: Move Soc specific components to soc files

[Leif Lindholm]

On Fri, Mar 20, 2020 at 20:05:21 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal 
> 
> Soc specific components ought to be part of Soc files and not
> platform files. move the same to SOC files
> 
> Signed-off-by: Pankaj Bansal 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc | 7 +--
>  Platform/NXP/NxpQoriqLs.dsc.inc  | 2 ++
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc  | 7 ++-
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc 
> b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index c8105593533f..802cccdce63b 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -2,7 +2,7 @@
>  #
>  #  LS1043ARDB Board package.
>  #
> -#  Copyright 2017-2019 NXP
> +#  Copyright 2017-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -26,11 +26,7 @@ [Defines]
>  !include Silicon/NXP/LS1043A/LS1043A.dsc.inc
>  
>  [LibraryClasses.common]
> -  SocLib|Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
>
> ArmPlatformLib|Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> -  
> ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> -  SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
> -  IoAccessLib|Silicon/NXP/Library/IoAccessLib/IoAccessLib.inf
>RealTimeClockLib|Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf
>  
>  [PcdsFixedAtBuild.common]
> @@ -46,7 +42,6 @@ [PcdsFixedAtBuild.common]
>#
># Board Specific Pcds
>#
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x021c0500
>gNxpQoriqLsTokenSpaceGuid.PcdSerdes2Enabled|FALSE
>gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x1
>  
> diff --git a/Platform/NXP/NxpQoriqLs.dsc.inc b/Platform/NXP/NxpQoriqLs.dsc.inc
> index b28e0615f7ca..94d3e53a046b 100644
> --- a/Platform/NXP/NxpQoriqLs.dsc.inc
> +++ b/Platform/NXP/NxpQoriqLs.dsc.inc
> @@ -95,6 +95,8 @@ [LibraryClasses.common]
>
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>  
>I2cLib|Silicon/NXP/Library/I2cLib/I2cLib.inf
> +  
> ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> +  IoAccessLib|Silicon/NXP/Library/IoAccessLib/IoAccessLib.inf
>  
>  [LibraryClasses.common.SEC]
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc 
> b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> index dbd680b0ad28..d2d4133428c3 100644
> --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc
> @@ -1,12 +1,16 @@
>  #  LS1043A.dsc
>  #  LS1043A Soc package.
>  #
> -#  Copyright 2017-2019 NXP
> +#  Copyright 2017-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  #
>  
> +[LibraryClasses.common]
> +  SocLib|Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
> +  SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf
> +
>  
> 
>  #
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> @@ -20,6 +24,7 @@ [PcdsDynamicDefault.common]
>gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x01402000
>  
>  [PcdsFixedAtBuild.common]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x021c0500
>  
>#
># CCSR Address Space and other attached Memories
> -- 
> 2.17.1
> 

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

View/Reply Online (#56828): https://edk2.groups.io/g/devel/message/56828
Mute This Topic: https://groups.io/mt/72077427/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]

On 04/01/20 03:15, Michael Kubacki wrote:
> 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.

(1) *Normally*, here's what I would tell you:

Please log in to your groups.io account, and navigate to the following
setting:

Your name (upper right corner)
  Account
Preferences (top of left sidebar)
  I always want copies of my own emails (checkbox at the bottom)

Please *untick* this checkbox.

The name of the checkbox is very misleading. The actual behavior is the
following:

  "I am a gmail user, and gmail de-duplicates messages that are sent to
   me both directly, and reflected through the mailing list. As a
   special case, the same de-duplication applies to messages I send to
   the list, and are reflected to me by the list -- gmail just hides
   those from me as "seen". I don't like this; I want my reflected
   copies of messages that I sent. Because the de-duplication is
   Message-ID based, groups.io should please falsify the Message-ID on
   those messages that I send *AND* are delivered back to me *only*"

Meaning, if you have this box checked, then groups.io places
<@groups.io>-style message IDs in the emails that it sends back *to
you* (only to you) *and* that originate from you.


(2) *However*, that is not the problem here. (Or, more precisely, not
the only problem.) Because, as I say above, this message-ID
re-generation by groups.io only applies to *you*; so the threading of
your patch series would only be broken in *your* list folder, not in
mine (or in any other list subscriber's).

But, the threading *is* broken on my end too.

And the reason for that is that the SMTP server that you use, ignores
the Message-ID put in place by git-send-email, and generates its own.

Your original message ID (as stated above), from git-send-email, is:

  20200327215536.9556-1-michael.kuba...@outlook.com

But the message ID in my list folder is:

  mwhpr07mb3440a69b5afad0e373974fbfe9...@mwhpr07mb3440.namprd07.prod.outlook.com

Therefore, the original message-id (from git-send-email) is not moved to
the X-Microsoft-Original-Message-ID header by groups.io -- instead, it
is moved there by your SMTP server, at .

Can you disable that somehow? It is incorrect behavior. An SMTP server
itself should only put a Message-ID on an outgoing email if that email
doesn't already come with one.

To summarize, there are two issues:

(a) your groups.io account settings have the gmail-oriented Message-ID
falsification enabled, which breaks threading for you (and only for you)

(b) your SMTP server overwrites the git-send-email-generated Message-ID
with its own, which breaks threading for every list subscriber
(including you).

You can easily remedy (a) in your groups.io account settings. Not sure
how you can fix problem (b) -- can you use a different SMTP server
maybe?

(I've been very happy that we've finally seen patches posted to the list
from a @microsoft.com email address -- it would be fantastic if that
continued, with the threading fixed even!)

Thanks!
Laszlo


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

View/Reply Online (#56827): https://edk2.groups.io/g/devel/message/56827
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 v1 1/1] INF Spec: Add file dependency to [Sources] syntax

[Bob Feng]

Hi Pierre,

I will review the spec and code change in this week.

Thanks,
Bob

-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
PierreGondois
Sent: Monday, March 30, 2020 11:52 PM
To: devel@edk2.groups.io; Sami Mujawar ; Feng, Bob C 
; Gao, Liming 
Cc: nd 
Subject: Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add file dependency to 
[Sources] syntax

Hello Liming and Bob,
I couldn't find the list of maintainers for the specification files, but it 
seems Liming is a maintainer. If a maintainer is missing, please feel free to 
cc him,

Regards,
Pierre

-Original Message-
From: PierreGondois 
Sent: Monday, March 30, 2020 4:43 PM
To: devel@edk2.groups.io
Cc: Pierre Gondois ; liming@intel.com; 
sami.muja...@arm.org; nd 
Subject: [PATCH v1 1/1] INF Spec: Add file dependency to [Sources] syntax

From: Pierre Gondois 

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

When building an edk2 module, a C file was including a .hex file generated by 
the compilation of an ASL file.
To describe this dependency between an ASL file and a C file, the edk2 patch,
 - named "BaseTools: Build ASL files before C files",
 - discussed at: https://edk2.groups.io/g/devel/message/52550
has been created.
This patch allows to establish build dependencies in the [Sources] section, 
between files that are not of the same language.
E.g.:
[Sources]
  FileName1.X
  FileName2.Y : FileName1.X
  FileName3.Z : FileName1.X FileName2.Y

Here:
  * FileName1.X will be built prior to FileName2.Y.
  * FileName1.X and FileName2.Y will be built prior to
FileName3.Z.

This patch updates the Inf specification accordingly.

Signed-off-by: Pierre Gondois 
---

The changes can be seen at 
https://github.com/PierreARM/edk2-InfSpecification/tree/Bugzilla_2464_Enable_Build_Dependencies_v1

Notes:
v1:
 - Enable build dependencies in the [Sources] section

 2_inf_overview/25_[sources]_section.md  | 12 
 3_edk_ii_inf_file_format/32_component_inf_definition.md |  3 +++
 3_edk_ii_inf_file_format/39_[sources]_sections.md   |  6 --
 README.md   |  1 +
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/2_inf_overview/25_[sources]_section.md 
b/2_inf_overview/25_[sources]_section.md
index 
54757e61e37268eed293a5288e607cf2c7cfacf6..5b9f0a8395ef2be4497d99197dc695625d841830
 100644
--- a/2_inf_overview/25_[sources]_section.md
+++ b/2_inf_overview/25_[sources]_section.md
@@ -2,6 +2,7 @@
   2.5 [Sources] Section
 
   Copyright (c) 2007-2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2020, ARM Limited. All rights reserved.
 
   Redistribution and use in source (original document form) and 'compiled'
   forms (converted to PDF, epub, HTML and other formats) with or without @@ 
-94,6 +95,17 @@ The following is an example for sources sections.
 
 ```
 
+The following example depicts the syntax to establish dependencies 
+between files of different source types. As shown in the example below, 
+Dsdt.asl will be compiled before DadtHandler.c:
+
+```ini
+[Sources.common]
+  DsdtHandler.c : Dsdt.asl
+  DsdtHandler.h
+  Dsdt.asl
+```
+
 All Unicode files must be listed in the source section. If a Unicode file,  
`A.uni`, has the statement: `#include B.uni`, and `B.uni` has a statement:
 `#include C.uni`, both `B.uni` and `C.uni` files must be listed in the INF 
diff --git a/3_edk_ii_inf_file_format/32_component_inf_definition.md 
b/3_edk_ii_inf_file_format/32_component_inf_definition.md
index 
164771cb4cfff6e81fbf762a67ff741c190cecde..d776714c24c0baf2348f53dc2576c9feb6f3cb5e
 100644
--- a/3_edk_ii_inf_file_format/32_component_inf_definition.md
+++ b/3_edk_ii_inf_file_format/32_component_inf_definition.md
@@ -2,6 +2,7 @@
   3.2 Component INF Definition
 
   Copyright (c) 2007-2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2020, ARM Limited. All rights reserved.
 
   Redistribution and use in source (original document form) and 'compiled'
   forms (converted to PDF, epub, HTML and other formats) with or without @@ 
-133,6 +134,8 @@ The following are common definitions used by multiple section 
types.
  ::=  "=" 
  ::= "|"
  ::=   
+::= ":"
+   ::=   
::= "*"
  ::= "," *
  ::= "," *
diff --git a/3_edk_ii_inf_file_format/39_[sources]_sections.md 
b/3_edk_ii_inf_file_format/39_[sources]_sections.md
index 
810995df26ba409ca2cf3ebe6238aa5d55cf81f1..ac966425101fd44a57b09d36f95a0f732eab1c59
 100644
--- a/3_edk_ii_inf_file_format/39_[sources]_sections.md
+++ b/3_edk_ii_inf_file_format/39_[sources]_sections.md
@@ -2,6 +2,7 @@
   3.9 [Sources] Sections
 
   Copyright (c) 2007-2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2020, ARM Limited. All rights reserved.
 
   Redistribution and use in source (original document form) and 'compiled'
   forms (converted to PDF, epub, HTML and other 

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

[Ard Biesheuvel]

On Wed, 1 Apr 2020 at 10:37, Laszlo Ersek  wrote:
>
> On 04/01/20 00:17, Liran Alon wrote:
>
> > 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
>
> This is a very special case.
>
> Please see the justification in commit ad90df8ac018
> ("ArmPlatformPkg/ArmVirtualizationPkg: Add private HobLib implementation
> for DXE phase", 2014-09-18).
>
> The stock HobLib instance depends on DebugLib, for using the normal
> ASSERT() macro.
>
> Furthermore, the stock serial-based DebugLib instance depends on
> SerialPortLib, for printing messages.
>
> That produces a HobLib -> DebugLib -> SerialPortLib dependency chain.
>
> But, in case of this particular platform, our SerialPortLib instance
> depends on HobLib, for retrieving the particulars of the serial port.
> This creates a dependency cycle:
>
>   HobLib -> DebugLib -> SerialPortLib -> HobLib
>
> which makes the platform un-buildable.
>
> We had to break the dependency cycle somewhere, and the best (or maybe
> only -- I don't recall exactly anymore) link to break was the HobLib ->
> DebugLib dependency. We introduced our own HobLib instance, which (IIRC)
> was almost identical to the stock one, except that its (only) DebugLib
> dependency, namely the ASSERT(), was reimplemented with a plain
> CpuDeadLoop(). And so the dependency chain ended up as:
>
>   DebugLib -> SerialPortLib -> HobLib
>
> Not circular any more.
>
> > and OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c)
>
> Very similar same case; here we even have a comment:
>
> //
> // We can't use DebugLib due to a constructor dependency cycle between 
> DebugLib
> // and ourselves.
> //
>
> The BaseDebugLibSerialPort instance depends on SerialPortLib, so a
> SerialPortLib instance cannot "depend back" on DebugLib, in combination
> with BaseDebugLibSerialPort.
>

There's an additional problem in EDK2 that we never fixed, which is
the fact that constructor dependencies are not transitive across
library implementations that don't have a constructor themselves. For
instance, in the following case

LibA (+)
  depends on
LibB (-)
  depends on
LibC (+)

where (+) means 'has constructor' and (-) means 'has no constructor',
the EDK2 build tools may emit the LibA and LibC constructor
invocations in any order. However, as soon as you try to fix this, we
end up with circular dependencies all over the place, and none of the
platforms can be built anymore.

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

View/Reply Online (#56824): https://edk2.groups.io/g/devel/message/56824
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 04/01/20 00:17, Liran Alon wrote:

> 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

This is a very special case.

Please see the justification in commit ad90df8ac018
("ArmPlatformPkg/ArmVirtualizationPkg: Add private HobLib implementation
for DXE phase", 2014-09-18).

The stock HobLib instance depends on DebugLib, for using the normal
ASSERT() macro.

Furthermore, the stock serial-based DebugLib instance depends on
SerialPortLib, for printing messages.

That produces a HobLib -> DebugLib -> SerialPortLib dependency chain.

But, in case of this particular platform, our SerialPortLib instance
depends on HobLib, for retrieving the particulars of the serial port.
This creates a dependency cycle:

  HobLib -> DebugLib -> SerialPortLib -> HobLib

which makes the platform un-buildable.

We had to break the dependency cycle somewhere, and the best (or maybe
only -- I don't recall exactly anymore) link to break was the HobLib ->
DebugLib dependency. We introduced our own HobLib instance, which (IIRC)
was almost identical to the stock one, except that its (only) DebugLib
dependency, namely the ASSERT(), was reimplemented with a plain
CpuDeadLoop(). And so the dependency chain ended up as:

  DebugLib -> SerialPortLib -> HobLib

Not circular any more.

> and OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c)

Very similar same case; here we even have a comment:

//
// We can't use DebugLib due to a constructor dependency cycle between DebugLib
// and ourselves.
//

The BaseDebugLibSerialPort instance depends on SerialPortLib, so a
SerialPortLib instance cannot "depend back" on DebugLib, in combination
with BaseDebugLibSerialPort.

Thanks,
Laszlo


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

View/Reply Online (#56823): https://edk2.groups.io/g/devel/message/56823
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 v2 0/2] Use submodule way to access brotli

[Zhang, Shenglei]

Currently brotli is used and customized by edk2 in BaseTools
and MdeModulePkg. These two patches make brotli a submodule in
edk2.
https://bugzilla.tianocore.org/show_bug.cgi?id=2558
https://bugzilla.tianocore.org/show_bug.cgi?id=2559

v2: Add submodule path in CISettings.py

Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Bob Feng 
Shenglei Zhang (2):
  MdeModulePkg/BrotliCustomDecompressLib: Make brotli a submodule
  BaseTools: Make brotli a submodule

 .../C/BrotliCompress/common/dictionary.c  | 5905 -
 .../C/BrotliCompress/common/transform.c   |  235 -
 .../Source/C/BrotliCompress/dec/bit_reader.c  |   48 -
 .../Source/C/BrotliCompress/dec/decode.c  | 2497 ---
 .../Source/C/BrotliCompress/dec/huffman.c |  356 -
 BaseTools/Source/C/BrotliCompress/dec/state.c |  164 -
 .../BrotliCompress/enc/backward_references.c  |  144 -
 .../enc/backward_references_hq.c  |  830 ---
 .../Source/C/BrotliCompress/enc/bit_cost.c|   35 -
 .../C/BrotliCompress/enc/block_splitter.c |  194 -
 .../C/BrotliCompress/enc/brotli_bit_stream.c  | 1331 
 .../Source/C/BrotliCompress/enc/cluster.c |   56 -
 .../C/BrotliCompress/enc/compress_fragment.c  |  790 ---
 .../enc/compress_fragment_two_pass.c  |  645 --
 .../C/BrotliCompress/enc/dictionary_hash.c| 1120 
 .../Source/C/BrotliCompress/enc/encode.c  | 1864 --
 .../C/BrotliCompress/enc/encoder_dict.c   |   32 -
 .../C/BrotliCompress/enc/entropy_encode.c |  501 --
 .../Source/C/BrotliCompress/enc/histogram.c   |  100 -
 .../C/BrotliCompress/enc/literal_cost.c   |  175 -
 .../Source/C/BrotliCompress/enc/memory.c  |  170 -
 .../Source/C/BrotliCompress/enc/metablock.c   |  667 --
 .../Source/C/BrotliCompress/enc/static_dict.c |  486 --
 .../Source/C/BrotliCompress/enc/utf8_util.c   |   85 -
 .../Source/C/BrotliCompress/tools/brotli.c| 1143 
 .../BrotliDecUefiSupport.c|   31 +
 .../common/dictionary.c   | 5905 -
 .../common/transform.c|  235 -
 .../dec/bit_reader.c  |   48 -
 .../BrotliCustomDecompressLib/dec/decode.c| 2500 ---
 .../BrotliCustomDecompressLib/dec/huffman.c   |  356 -
 .../BrotliCustomDecompressLib/dec/state.c |  164 -
 .gitmodules   |6 +
 .pytool/CISettings.py |4 +
 BaseTools/Source/C/BrotliCompress/GNUmakefile |   54 +-
 BaseTools/Source/C/BrotliCompress/LICENSE |   19 -
 BaseTools/Source/C/BrotliCompress/Makefile|   52 +-
 BaseTools/Source/C/BrotliCompress/README.md   |   26 -
 BaseTools/Source/C/BrotliCompress/ReadMe.txt  |2 -
 BaseTools/Source/C/BrotliCompress/brotli  |1 +
 .../C/BrotliCompress/common/constants.h   |   64 -
 .../Source/C/BrotliCompress/common/context.h  |  261 -
 .../C/BrotliCompress/common/dictionary.h  |   64 -
 .../Source/C/BrotliCompress/common/platform.h |  558 --
 .../C/BrotliCompress/common/transform.h   |   80 -
 .../Source/C/BrotliCompress/common/version.h  |   26 -
 .../Source/C/BrotliCompress/dec/bit_reader.h  |  309 -
 .../Source/C/BrotliCompress/dec/huffman.h |   72 -
 .../Source/C/BrotliCompress/dec/prefix.h  |  750 ---
 BaseTools/Source/C/BrotliCompress/dec/state.h |  258 -
 .../brotli-comparison-study-2015-09-22.pdf|  Bin 215208 -> 0 bytes
 .../BrotliCompress/enc/backward_references.h  |   38 -
 .../enc/backward_references_hq.h  |   93 -
 .../enc/backward_references_inc.h |  153 -
 .../Source/C/BrotliCompress/enc/bit_cost.h|   63 -
 .../C/BrotliCompress/enc/bit_cost_inc.h   |  127 -
 .../C/BrotliCompress/enc/block_encoder_inc.h  |   34 -
 .../C/BrotliCompress/enc/block_splitter.h |   51 -
 .../C/BrotliCompress/enc/block_splitter_inc.h |  431 --
 .../C/BrotliCompress/enc/brotli_bit_stream.h  |   84 -
 .../Source/C/BrotliCompress/enc/cluster.h |   48 -
 .../Source/C/BrotliCompress/enc/cluster_inc.h |  317 -
 .../Source/C/BrotliCompress/enc/command.h |  190 -
 .../C/BrotliCompress/enc/compress_fragment.h  |   61 -
 .../enc/compress_fragment_two_pass.h  |   54 -
 .../C/BrotliCompress/enc/dictionary_hash.h|   24 -
 .../C/BrotliCompress/enc/encoder_dict.h   |   41 -
 .../C/BrotliCompress/enc/entropy_encode.h |  122 -
 .../enc/entropy_encode_static.h   |  539 --
 .../Source/C/BrotliCompress/enc/fast_log.h|  147 -
 .../C/BrotliCompress/enc/find_match_length.h  |   80 -
 BaseTools/Source/C/BrotliCompress/enc/hash.h  |  497 --
 .../C/BrotliCompress/enc/hash_composite_inc.h |  133 -
 .../enc/hash_forgetful_chain_inc.h|  254 -
 .../enc/hash_longest_match64_inc.h|  266 -
 .../enc/hash_longest_match_inc.h  |  258 -
 .../enc/hash_longest_match_quickly_inc.h  |  235 -
 .../C/BrotliCompress/enc/hash_rolling_inc.h   |  215 -
 .../enc/hash_to_binary_tree_inc.h |  327 -
 

[edk2-devel] [PATCH] .azurepipelines: Update CI steps

[Zhang, Shenglei]

From: Sean Brogan 

Update CI steps to build base tools after setup and update,
as basetools might have dependencies that need to be resolved.

Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Michael D Kinney 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---
 .azurepipelines/templates/pr-gate-steps.yml | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/.azurepipelines/templates/pr-gate-steps.yml 
b/.azurepipelines/templates/pr-gate-steps.yml
index a969661dea15..3fcc1e88d804 100644
--- a/.azurepipelines/templates/pr-gate-steps.yml
+++ b/.azurepipelines/templates/pr-gate-steps.yml
@@ -39,11 +39,6 @@ steps:
 arguments: -c .pytool/CISettings.py -p ${{ parameters.build_pkgs }} 
--pr-target origin/$(System.PullRequest.targetBranch) 
--output-csv-format-string "##vso[task.setvariable 
variable=pkgs_to_build;isOutpout=true]{pkgcsv}" --output-count-format-string 
"##vso[task.setvariable variable=pkg_count;isOutpout=true]{pkgcount}"
   condition: eq(variables['Build.Reason'], 'PullRequest')
 
-# build basetools
-- template: basetools-build-steps.yml
-  parameters:
-tool_chain_tag: ${{ parameters.tool_chain_tag }}
-
 # install spell check prereqs
 - template: spell-check-prereq-steps.yml
 
@@ -62,6 +57,13 @@ steps:
 arguments: -c .pytool/CISettings.py -p $(pkgs_to_build) -t ${{ 
parameters.build_targets}} -a ${{ parameters.build_archs}} TOOL_CHAIN_TAG=${{ 
parameters.tool_chain_tag}}
   condition: and(gt(variables.pkg_count, 0), succeeded())
 
+# build basetools
+#   do this after setup and update so that code base dependencies
+#   are all resolved.
+- template: basetools-build-steps.yml
+  parameters:
+tool_chain_tag: ${{ parameters.tool_chain_tag }}
+
 - task: CmdLine@1
   displayName: Build and Test ${{ parameters.build_pkgs }} ${{ 
parameters.build_archs}}
   inputs:
-- 
2.18.0.windows.1


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

View/Reply Online (#56819): https://edk2.groups.io/g/devel/message/56819
Mute This Topic: https://groups.io/mt/72695867/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 04/01/20 00: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).

OK.

Based on your answer, and also on Sean's
, for this patch:

Reviewed-by: Laszlo Ersek 

Thanks!
Laszlo

> 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 (#56818): https://edk2.groups.io/g/devel/message/56818
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] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

[Ard Biesheuvel]

On Tue, 31 Mar 2020 at 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 VS builds?
>

In my opinion, having these intrinsics libraries added via NULL
library class resolution was a mistake to begin with.

Every component that we build incorporates some kind of startup code
library that defines the _ModuleEntryPoint symbol, and it would be
much better to make those libraries include an IntrinsicsLibrary
dependency that can be satisfied by arch specific versions that also
encapsulate the toolchain dependencies (such as this _fltused symbol,
or memcpy/memset on ARM/GCC).

On another note, it is still deeply disappointing that we need to jump
through all of these hoops because the *only* single use of floating
point in OpenSSL is the entropy estimate of an RNG, which is in the
range of 0..1023 to begin with [IIRC). Remember that we also have a
softfloat submodule for 32-bit ARM, for the same stupid reason. If we
could stop pulling in that part of the code (or replace it with an
UINT16 when building for the UEFI target), the whole floating point
issue would mostly go away AFAICT.

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

View/Reply Online (#56817): https://edk2.groups.io/g/devel/message/56817
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 1/3] Platform/Intel: Add all pathes of feature domains to package path

[Ni, Ray]

Reviewed-by: Ray Ni 

> -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 (#56816): https://edk2.groups.io/g/devel/message/56816
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]
-=-=-=-=-=-=-=-=-=-=-=-