from:"rebecca"

edk2-basetools is finally fixed and releases can once again be published 
to PyPI.


However, in moving from setup.py to pyproject.toml the process has 
changed, and Joey suggested the old way might have been chosen 
deliberately to be different from edk2-pytool-library and 
edk2-pytool-extensions.



Previously it fetched the list of releases from PyPI and incremented the 
version number before publishing a new version, while it now depends on 
the git repo being tagged.



Should I work on migrating the old code and figuring out how to make it 
work with pyproject.toml?



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116069): https://edk2.groups.io/g/devel/message/116069
Mute This Topic: https://groups.io/mt/104615885/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2 Patch 1/1] BaseTools: Syntax warning invalid escape sequence \C


Merged as 3e91e421365027ee3e655feab33c67a4f544c777.


--
Rebecca Cran


On 2/6/24 00:02, Jayaprakash N wrote:

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

This commit fixes the issue reported through BZ4666.
The Syntax warning related to invalid escape sequence
for \C is seen on Windows OS based builds of edk2 sources.
On Windows the path seperator needs to prefixed with \
so essentially we need to use \\ as path seperator.

Cc: Rebecca Cran 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Jayaprakash N 
Signed-off-by: Jayaprakash N 
---
  BaseTools/Source/Python/Workspace/DscBuildData.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 4768099343..b69d406249 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -2949,7 +2949,7 @@ class DscBuildData(PlatformBuildClassObject):
  for include_file in IncFileList:
  MakeApp += "$(OBJECTS) : %s\n" % include_file
  if sys.platform == "win32":
-PcdValueCommonPath = 
os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"], 
"Source\C\Common\PcdValueCommon.c"))
+PcdValueCommonPath = 
os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"], 
"Source\\C\\Common\\PcdValueCommon.c"))
  MakeApp = MakeApp + '%s\\PcdValueCommon.c : %s\n' % 
(self.OutputPath, PcdValueCommonPath)
  MakeApp = MakeApp + '\tcopy /y %s $@\n' % (PcdValueCommonPath)
  else:



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116066): https://edk2.groups.io/g/devel/message/116066
Mute This Topic: https://groups.io/mt/104193926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [v2] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules


edk2-basetools v0.1.50 has just been published so we're back up and running.

I'll work my way through the backlog of BaseTools changes in the next 
few days.



--
Rebecca Cran


On 2/24/24 13:59, Laszlo Ersek wrote:

v1 posting:

   https://edk2.groups.io/g/devel/message/115193
   msgid <36593e23-d3e8-b71a-808d-ef94260b5...@redhat.com>

Bugzilla:

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

In version 2, the feature is structured differently. Following Mike's
advice, for compatibility, the ProcessLibraryConstructorList()
declaration in AutoGen.h is now gated on the SEC module having
INF_VERSION >= 1.30.

Accordingly,

- I now update the Build specification and the Inf specification (see
   patch sets posted in response to this email),

- edk2 only receives a single patch (for AutoGen), for the time being,

- the same edk2 patch is being ported to edk2-basetools:
   https://github.com/tianocore/edk2-basetools/pull/120.

Next steps: once all of the above is merged, *and* an edk2-basetools
release has been tagged and published, I'll rework the C code patches
for edk2 and edk2-platforms, from the v1 patch sets, as follows:

- all those SEC modules will have to see their INF_VERSIONs bumped to
   1.30, for triggering the new code generation,

- pip-requirements.txt/edk2-basetools will need to reference the new
   edk2-basetools release, for exposing the feature in the first place.

Laszlo




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116063): https://edk2.groups.io/g/devel/message/116063
Mute This Topic: https://groups.io/mt/104553597/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] PatchCheck Updates

Merged as 
dae8c29dab546fad2801e70967855a9f6ae14ae0...6d571c0070161b1b96049410f8584c0955d73536


--
Rebecca Cran

On 2/18/2024 1:59 PM, Michael D Kinney via groups.io wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4679
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4680
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4693
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4694

Fix a collection of PatchCheck issues and add a new PatchCheck
featue to check for a commit that modifies files in multiple
packages with option to disable the multiple package check as
a command line option or a commit message tag.

* Reject patches that match Author email "devel@edk2.groups.io"
* Update the current check for " via Groups.Io" to perform a
   case insensitive match. It appears that groups.io has changed the
   format of this string to use all lower case.
* Update logic in CommitMessageCheck class to return errors
   detected in commit message signatures instead of silently
   passing the check.
* Genenerate an error if no Cc tags are present to make sure
   all patches Cc the required maintainers/reviewers.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Michael Kubacki 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 

Michael D Kinney (4):
   BaseTools/Scripts/PatchCheck: Update Author checks
   BaseTools/Scripts/PatchCheck: Return CommitMessageCheck errors
   BaseTools/Scripts/PatchCheck: Error if no Cc tags are present
   BaseTools/Scripts/PatchCheck: Error if commit modifies multiple
 packages

  BaseTools/Scripts/PatchCheck.py | 91 +++--
  1 file changed, 86 insertions(+), 5 deletions(-)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116058): https://edk2.groups.io/g/devel/message/116058
Mute This Topic: https://groups.io/mt/104434581/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/4] BaseTools/Scripts/PatchCheck: Update Author checks


For the series:

Reviewed-by: Rebecca Cran 

--
Rebecca Cran


On 2/18/2024 1:59 PM, Michael D Kinney wrote:

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

* Reject patches that match Author email "devel@edk2.groups.io"
* Update the current check for " via Groups.Io" to perform a
   case insensitive match. It appears that groups.io has changed the
   format of this string to use all lower case.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Michael D Kinney 
Reviewed-by: Rebecca Cran 
Reviewed-by: Ard Biesheuvel 
---
  BaseTools/Scripts/PatchCheck.py | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 1675dcbd7321..e600e0be440f 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -85,7 +85,11 @@ class EmailAddressCheck:
  self.error("The email address cannot contain a space: " +
 mo.group(3))
  
-if ' via Groups.Io' in name and mo.group(3).endswith('@groups.io'):

+if mo.group(3) == 'devel@edk2.groups.io':
+self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
+   email)
+
+if ' via groups.io' in name.lower() and 
mo.group(3).endswith('@groups.io'):
  self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
 email)
  





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116057): https://edk2.groups.io/g/devel/message/116057
Mute This Topic: https://groups.io/mt/104434582/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] EmbeddedPkg/Scripts/LauterbachT32: Fix EfiLoadDxe.cmm


Merged as e59a40b92ce92f17e3d8d99917868c5678d408b7.

--
Rebecca Cran


On 2/12/2024 10:41 AM, Leif Lindholm via groups.io wrote:

On Sun, Feb 11, 2024 at 16:10:32 -0700, Rebecca Cran wrote:

I'm guessing there haven't been any reviews because so few people are
familiar with the PRACTICE scripting language.

If there's nobody who can review it, could I get approval to commit it
anyway?

I'm good with that (after stable tag, of course):
Reviewed-by: Leif Lindholm 

It would make sense to also add you as a maintainer for
EmbeddedPkg/Scripts/LauterbachT32/

Regards,

Leif



--

Rebecca Cran


On 1/29/24 12:01, Rebecca Cran via groups.io wrote:

There have been many changes since EfiLoadDxe.cmm was last updated in
2011. The EFI_SYSTEM_TABLE can no longer be found by scanning memory on
4KB boundaries, so require users pass in its address instead. Update
various offsets so that the debug information can be found and loaded
with a recent version of TRACE32.

Signed-off-by: Rebecca Cran 
---
   EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm| 77 
+++-
   EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm |  7 +-
   EmbeddedPkg/Scripts/LauterbachT32/Readme.md |  8 +-
   3 files changed, 19 insertions(+), 73 deletions(-)

diff --git a/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm 
b/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm
index 1605a9b478df..06d307e12d8c 100644
--- a/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm
+++ b/EmbeddedPkg/Scripts/LauterbachT32/EfiLoadDxe.cmm
@@ -1,79 +1,22 @@
   ;
+; Copyright (c) 2024, Ampere Computing LLC. All rights reserved.
   ; Copyright (c) 2011, Hewlett-Packard Company. All rights reserved.
   ;
   ; SPDX-License-Identifier: BSD-2-Clause-Patent
   ;
-  LOCAL   
-
-  =0x2000   ; default to 512MB
-
-  gosub FindSystemTable 
-  ENTRY 
-
-  if !=0
-  (
-print "found system table at "
-gosub FindDebugInfo 
-  )
-  else
-  (
-print "ERROR: system table not found, check memory size"
-  )
+  PARAMETERS 
+
+  gosub FindDebugInfo 
 enddo
-FindSystemTable:
-  LOCAL
-  ENTRY   
-
-  print "FindSystemTable"
-  print "top of mem is $"
-
-  =
-
-  ; align to highest 4MB boundary
-  =&0xFFC0
-
-  ; start at top and look on 4MB boundaries for system table ptr structure
-  while >0
-  (
-; low signature match
-if Data.Long(a:)==0x20494249
-(
-  ; high signature match
-  if Data.Long(a:+4)==0x54535953
-  (
-; less than 4GB?
-if Data.Long(a:+0x0c)==0
-(
-  ; less than top of ram?
-  if Data.Long(a:+8)<
-  (
-return Data.Long(a:+8)
-  )
-)
-  )
-)
-
-if <0x40
-(
-  return 0
-)
-=
-  )
-
-  return 0
-
-
   FindDebugInfo:
 LOCAL  
  
 ENTRY   
-  print "FindDebugInfo"
-
 =0
-  =Data.Long(a:+0x40)
-  =Data.Long(a:+0x44)
+  =Data.Long(a:+0x68)
+  =Data.Long(a:+0x70)
 print "config table is at  ( entries)"
@@ -82,7 +25,7 @@ FindDebugInfo:
 =0
 while <
 (
-=+(*0x14)
+=+(*0x18)
   if Data.Long(a:)==0x49152E77
   (
 if Data.Long(a:+4)==0x47641ADA
@@ -120,8 +63,10 @@ FindDebugInfo:
   (
 if Data.Long(a:)==1 ; normal debug info type
 (
-=Data.Long(a:+4)
-do EfiProcessPeImage Data.Long(a:+0x20)
+=Data.Long(a:+8)
+=+0x40
+=Data.Long(a:)
+do /EfiProcessPeImage.cmm ""
 )
   )
   =+1
diff --git a/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm 
b/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm
index c3aab9d06a47..b0d97eec71b0 100644
--- a/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm
+++ b/EmbeddedPkg/Scripts/LauterbachT32/EfiProcessPeImage.cmm
@@ -1,4 +1,5 @@
   ;
+; Copyright (c) 2024, Ampere Computing LLC. All rights reserved.
   ; Copyright (c) 2011, Hewlett-Packard Company. All rights reserved.
   ;
   ; SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,11 +11,11 @@
 =
 print "PE32 image found at "
-  ; offset from dos hdr to PE file hdr
+  ; offset from dos hdr to PE file hdr (i.e. 'PE\0\0' signature)
 =+Data.Long(c:+0x3C)
 ; offset to debug dir in PE hdrs
-  =Data.Long(c:+0xA8)
+  =Data.Long(c:+0xf10)
 if ==0
 (
   print "no debug dir for image at "
@@ -62,7 +63,7 @@
   =
 )
-  print "found path "
+  print "found path  with address "
   ON ERROR GOSUB
 return
 data.load.elf   /NOCODE /NOCLEAR
diff --git a/EmbeddedPkg/Scripts/LauterbachT32/Readme.md 
b/EmbeddedPkg/Scripts/LauterbachT32/Readme.md
index 51d2c8da5405..c30ec20a3d96 100644
--- a/EmbeddedPkg/Scripts/LauterbachT32/Readme.md
+++ b/EmbeddedPkg/Scripts/LauterbachT32/Readme.md
@@ -1,10 +1,10 @@
   # DXE Phase Debug
-Update the memsize variable in EfiLoadDxe.cmm for the actu

Re: [edk2-devel] [PATCH v1 1/1] BaseTools/GenFds: Resolve absolute workspace INF paths


Reviewed-by: Rebecca Cran 

On 2/20/2024 9:22 PM, mikub...@linux.microsoft.com wrote:

From: Michael Kubacki 

Currently, if an INF path is an absolute path on Linux (begins with
"/"), the "/" character will be removed. If the path is an absolute
system path, this creates an invalid path.

An example of when this may be an issue is in external dependencies
where an INF is within the external dependency, the `set_build_var`
flag is set, and DSC files refer to files by its build variable
(e.g. `$(SHARED_BINARIES)/Module.inf`). INFs in a binary distribution
like this example may contain a [Binaries] section and refer to
different section files that can be used by a platform to compose an
FFS file. For example, the PE32 (.efi) and DEPEX (.depex) files.

In this case, `$(SHARED_BINARIES)` will be an absolute path to the
ext dep directory and `FfsInfStatement.__InfParse__` will remove the
leading "/" character so the path is invalid.

This change first checks if the absolute path will resolve into the
current workspace. If it does (as will happen in the shared crypto
ext dep example above), it modifies the path to be relative to the
workspace so later logic dependent on relative paths can operate on
it. If the absolute path is not within the current workspace, it
follows previous behavior for backward compatibility to that
scenario.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Michael Kubacki 
---
  BaseTools/Source/Python/GenFds/FfsInfStatement.py | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py 
b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
index 568efb6d7685..6550d939d49c 100644
--- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
+++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
@@ -19,6 +19,7 @@ from .GenFdsGlobalVariable import GenFdsGlobalVariable
  from .Ffs import SectionSuffix,FdfFvFileTypeToFileType
  import subprocess
  import sys
+from pathlib import Path
  from . import Section
  from . import RuleSimpleFile
  from . import RuleComplexFile
@@ -156,7 +157,12 @@ class FfsInfStatement(FfsInfStatementClassObject):
  if len(self.InfFileName) > 1 and self.InfFileName[0] == '\\' and 
self.InfFileName[1] == '\\':
  pass
  elif self.InfFileName[0] == '\\' or self.InfFileName[0] == '/' :
-self.InfFileName = self.InfFileName[1:]
+ws_path = Path(GenFdsGlobalVariable.WorkSpaceDir)
+inf_path = Path(self.InfFileName)
+if ws_path in inf_path.parents:
+self.InfFileName = str(inf_path.relative_to(ws_path))
+else:
+self.InfFileName = self.InfFileName[1:]
  
  if self.InfFileName.find('$') == -1:

  InfPath = NormPath(self.InfFileName)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116049): https://edk2.groups.io/g/devel/message/116049
Mute This Topic: https://groups.io/mt/104482915/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] Fixing edk2-basetools CI

2024-02-26 Thread Rebecca Cran

Is anyone familiar with how publishing to PyPi works? The "Publish to 
PyPI" step is failing:


Starting: Publish to PyPI
==
Task : Command line
Description  : Run a command line script using Bash on Linux and macOS 
and cmd.exe on Windows

Version  : 2.231.1
Author   : Microsoft Corporation
Help : 
https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/command-line

==
Generating script.
Script contents: shell
twine upload -r Pypi-edk2-basetools --config-file 
D:\a\_temp\twineAuthenticate\HtPdQh\.pypirc dist/*
== Starting Command Output 
===
"C:\Windows\system32\cmd.exe" /D /E:ON /V:OFF /S /C "CALL 
"D:\a\_temp\53734b8d-4c1a-4bd2-99b3-15f4d79433fc.cmd""

Uploading distributions to https://upload.pypi.org/legacy/
Uploading edk2_basetools-0.1.50-py3-none-any.whl

  0%  0.0/16.9 kB � --:-- � ?
  0%  0.0/16.9 kB � --:-- � ?
100%  16.9/16.9 kB � 00:00 � ?
100%  16.9/16.9 kB � 00:00 � ?
WARNING  Error during upload. Retry with the --verbose option for more 
details.

ERROR    HTTPError: 403 Forbidden from https://upload.pypi.org/legacy/
 Invalid or non-existent authentication information. See
 https://pypi.org/help/#invalid-auth for more information.
##[error]Cmd.exe exited with code '1'.
Finishing: Publish to PyPI

--
Rebecca Cran


On 2/13/24 12:42, Joey Vagedes wrote:

I agree - there are multiple blocking issues that will require some fixes - One 
additional thing I noted is that multiple areas still perform relative path 
imports - i.e. the import starts with `from .`.

These will all need to be updated to import from edk2basetools as some point 
soon as importing via relative paths can result in it importing the wrong file 
if the pypath environment variable is influenced. This open issue actually 
stems from that: https://github.com/tianocore/edk2-basetools/issues/110 as the 
end result is that edk2-basetools pip module can unexpectedly end up using the 
BaseTools/* source code rather than the pip module source code.

Thanks,
Joey

-Original Message-
From: Rebecca Cran 
Sent: Tuesday, February 13, 2024 11:28 AM
To: Joey Vagedes ; Rebecca Cran ; 
devel@edk2.groups.io; Kinney, Michael D ; Sean 
; Michael Kubacki 
Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI

[You don't often get email from rebe...@bsdio.com. Learn why this is important 
at https://aka.ms/LearnAboutSenderIdentification ]

Thanks, I've updated
https://github.com/tianocore/edk2-basetools/pull/116 with the changes you 
suggested.

Unfortunately several of the tools don't work with the projects.scripts section 
because they don't have Main etc. It turns out even Ecc is currently broken in 
the edk2-basetools PyPI package because PYTHONPATH doesn't contain the parent 
directory. I'll plan to fix the issues in the near future, once I've worked 
through the other PRs.


--

Rebecca Cran


On 2/12/24 10:06, Joey Vagedes wrote:

Hello! Regarding flake8, I see. That will take some time to work through - I 
completely understand.

When ready to work through that, I suggest switching to ruff. It can resolve 
some errors automatically for you and allows you to customize it more than 
flake8 (it is also faster).

I just ran ruff on the project, showing about 11k errors. One nice thing is 
that a large majority of it is regarding importing using *. By fixing those, 
you can reduce the errors by almost 70% (to 3400 errors), so it is not *quite* 
as bad as it seems (though 3400 is still A LOT).

--
-

Regarding the setup tools question, the build-backend part of pytoml (L3) is the entry 
point that is invoked - so we are still invoking setuptools. However, it pulls the 
necessary metadata from pyproject.toml rather than setup.py's setup() function. The main 
thing that is different is the "get_version" functionality, which we replicate 
in the pyproject.toml with setuptools-scm[toml] which simply uses the latest tag to set 
the version.

--
-

Additionally, I wanted to comment on the wrappers that you mentioned.

Using the [projects.scripts] section (i.e. CLI Commands feature) is in my 
opinion the correct way to move forward as users will be able to invoke 
commands themselves easily (i.e. just call Bin2Pcd, and it will work). However, 
the wrappers will continue to be necessary until all python source is removed 
from Basetools, so that us

Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Bus/Usb/UsbNetwork: Check array index range before access

2024-02-26 Thread Rebecca Cran


Reviewed-by: Rebecca Cran 

On 2/20/2024 8:21 AM, mikub...@linux.microsoft.com wrote:

From: Michael Kubacki 

Checks that an offset used to access array elements is within the
expected range before accessing the array item.

Cc: Liming Gao 
Cc: Ray Ni 
Cc: Rebecca Cran 
Cc: Richard Ho 
Signed-off-by: Michael Kubacki 
---
  MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcEcm/UsbEcmFunction.c  | 2 +-
  MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcNcm/UsbNcmFunction.c  | 2 +-
  MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis/UsbRndisFunction.c | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcEcm/UsbEcmFunction.c 
b/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcEcm/UsbEcmFunction.c
index 29f4508a38ce..0c1f252b85df 100644
--- a/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcEcm/UsbEcmFunction.c
+++ b/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcEcm/UsbEcmFunction.c
@@ -769,7 +769,7 @@ ConvertFilter (
  
Count = sizeof (gTable)/sizeof (gTable[0]);
  
-  for (Index = 0; (gTable[Index].Src != 0) && (Index < Count); Index++) {

+  for (Index = 0; (Index < Count) && (gTable[Index].Src != 0); Index++) {
  if (gTable[Index].Src & Value) {
*CdcFilter |= gTable[Index].Dst;
  }
diff --git a/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcNcm/UsbNcmFunction.c 
b/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcNcm/UsbNcmFunction.c
index baa2225bf8a8..ef01a6f5458c 100644
--- a/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcNcm/UsbNcmFunction.c
+++ b/MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcNcm/UsbNcmFunction.c
@@ -855,7 +855,7 @@ ConvertFilter (
  
Count = sizeof (gTable)/sizeof (gTable[0]);
  
-  for (Index = 0; (gTable[Index].Src != 0) && (Index < Count); Index++) {

+  for (Index = 0; (Index < Count) && (gTable[Index].Src != 0); Index++) {
  if (gTable[Index].Src & Value) {
*CdcFilter |= gTable[Index].Dst;
  }
diff --git a/MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis/UsbRndisFunction.c 
b/MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis/UsbRndisFunction.c
index 2c0dcae4cf96..6d45a1b775ba 100644
--- a/MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis/UsbRndisFunction.c
+++ b/MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis/UsbRndisFunction.c
@@ -803,7 +803,7 @@ ConvertFilter (
  
Count = sizeof (gTable)/sizeof (gTable[0]);
  
-  for (Index = 0; (gTable[Index].Src != 0) && (Index < Count); Index++) {

+  for (Index = 0; (Index < Count) && (gTable[Index].Src != 0); Index++) {
  if (gTable[Index].Src & Value) {
*CdcFilter |= gTable[Index].Dst;
  }





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115992): https://edk2.groups.io/g/devel/message/115992
Mute This Topic: https://groups.io/mt/104469090/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] Fixing edk2-basetools CI

2024-02-23 Thread Rebecca Cran

Thanks. I've merged that PR, but there's another problem - the 'build' 
package isn't available.


I've created another PR to fix it: 
https://github.com/tianocore/edk2-basetools/pull/118



--
Rebecca Cran


On 2/23/24 07:48, Rebecca Cran wrote:
Could someone review Fix build-publish-whl-steps.yml: we no longer use 
setup.py by bcran · Pull Request #117 · tianocore/edk2-basetools · 
GitHub <https://github.com/tianocore/edk2-basetools/pull/117> please?

It should be the last change needed to fix publishing releases.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115896): https://edk2.groups.io/g/devel/message/115896
Mute This Topic: https://groups.io/mt/104306226/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] Fixing edk2-basetools CI

2024-02-23 Thread Rebecca Cran

Could someone review Fix build-publish-whl-steps.yml: we no longer use 
setup.py by bcran · Pull Request #117 · tianocore/edk2-basetools · 
GitHub <https://github.com/tianocore/edk2-basetools/pull/117> please?

It should be the last change needed to fix publishing releases.

--
Rebecca Cran

On 2/23/2024 12:55 AM, Chen, Christine wrote:

Thanks a lot Rebecca~~

Thanks,
Christine


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael
D Kinney
Sent: Saturday, February 17, 2024 1:24 AM
To: Rebecca Cran ; Joey Vagedes
; Rebecca Cran
; devel@edk2.groups.io; Sean
; Michael Kubacki

Cc: Kinney, Michael D 
Subject: Re: [edk2-devel] Fixing edk2-basetools CI

Hi Rebecca,

This approach makes a lot of sense.  Thank you for the updated.  I have
approved.

Can we open some new Issues to re-enable flake8/ruf  and codecov so those
tasks are not forgotten.

Thanks,

Mike


-Original Message-
From: Rebecca Cran 
Sent: Thursday, February 15, 2024 11:51 PM
To: Joey Vagedes ; Rebecca Cran
; devel@edk2.groups.io; Kinney,
Michael D ; Sean ;
Michael Kubacki 
Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI

I've updated the PR
https://github.com/tianocore/edk2-basetools/pull/116/ to include
commits from the pyflake8 disable PR and a couple more commits that
cause CI to start passing. I realize that disabling both flake8 and
codecov isn't good, but my thinking is to get builds working again
then we can work on fixing the issues in the background. The
edk2-basetools project needs lots of work.


Add pyproject.toml and fix setup.py deprecation warnings

Disable running Pyflakes in CI since it's blocking releases

Fix comments in build-publish-whl-steps.yml

Update basic-setup-steps.yml to require Python 3.10

Disable codecov to fix CI and due to very low coverage


--
Rebecca Cran


On 2/13/24 12:42, Joey Vagedes wrote:

I agree - there are multiple blocking issues that will require some

fixes - One additional thing I noted is that multiple areas still
perform relative path imports - i.e. the import starts with `from .`.

These will all need to be updated to import from edk2basetools as

some point soon as importing via relative paths can result in it
importing the wrong file if the pypath environment variable is
influenced. This open issue actually stems from that:
https://github.com/tianocore/edk2-basetools/issues/110 as the end
result is that edk2-basetools pip module can unexpectedly end up using
the BaseTools/* source code rather than the pip module source code.

Thanks,
Joey

-Original Message-
From: Rebecca Cran 
Sent: Tuesday, February 13, 2024 11:28 AM
To: Joey Vagedes ; Rebecca Cran

; devel@edk2.groups.io; Kinney,
Michael D ; Sean ;
Michael Kubacki 

Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI

[You don't often get email from rebe...@bsdio.com. Learn why this is

important at https://aka.ms/LearnAboutSenderIdentification ]

Thanks, I've updated
https://github.com/tianocore/edk2-basetools/pull/116 with the
changes

you suggested.

Unfortunately several of the tools don't work with the

projects.scripts section because they don't have Main etc. It turns
out even Ecc is currently broken in the edk2-basetools PyPI package
because PYTHONPATH doesn't contain the parent directory. I'll plan to
fix the issues in the near future, once I've worked through the other PRs.


--

Rebecca Cran


On 2/12/24 10:06, Joey Vagedes wrote:

Hello! Regarding flake8, I see. That will take some time to work

through - I completely understand.

When ready to work through that, I suggest switching to ruff. It
can

resolve some errors automatically for you and allows you to customize
it more than flake8 (it is also faster).

I just ran ruff on the project, showing about 11k errors. One nice

thing is that a large majority of it is regarding importing using *.
By fixing those, you can reduce the errors by almost 70% (to 3400
errors), so it is not *quite* as bad as it seems (though 3400 is still A LOT).

---
-

--

-

Regarding the setup tools question, the build-backend part of
pytoml

(L3) is the entry point that is invoked - so we are still invoking
setuptools. However, it pulls the necessary metadata from
pyproject.toml rather than setup.py's setup() function. The main thing
that is different is the "get_version" functionality, which we
replicate in the pyproject.toml with setuptools-scm[toml] which simply
uses the latest tag to set the version.

---
-

--

-

Additionally, I wanted to comment on the wrappers that you

mentioned.

Using the [projects.scripts] section (i.e. CLI Commands feature) is

in my opinion the correct way to move forward as users will be able to
invoke commands themselves easily (i.e. j

Re: [edk2-devel] Fixing edk2-basetools CI

2024-02-15 Thread Rebecca Cran

I've updated the PR 
https://github.com/tianocore/edk2-basetools/pull/116/ to include commits 
from the pyflake8 disable PR and a couple more commits that cause CI to 
start passing. I realize that disabling both flake8 and codecov isn't 
good, but my thinking is to get builds working again then we can work on 
fixing the issues in the background. The edk2-basetools project needs 
lots of work.



Add pyproject.toml and fix setup.py deprecation warnings

Disable running Pyflakes in CI since it's blocking releases

Fix comments in build-publish-whl-steps.yml

Update basic-setup-steps.yml to require Python 3.10

Disable codecov to fix CI and due to very low coverage


--
Rebecca Cran


On 2/13/24 12:42, Joey Vagedes wrote:

I agree - there are multiple blocking issues that will require some fixes - One 
additional thing I noted is that multiple areas still perform relative path 
imports - i.e. the import starts with `from .`.

These will all need to be updated to import from edk2basetools as some point 
soon as importing via relative paths can result in it importing the wrong file 
if the pypath environment variable is influenced. This open issue actually 
stems from that: https://github.com/tianocore/edk2-basetools/issues/110 as the 
end result is that edk2-basetools pip module can unexpectedly end up using the 
BaseTools/* source code rather than the pip module source code.

Thanks,
Joey

-Original Message-
From: Rebecca Cran 
Sent: Tuesday, February 13, 2024 11:28 AM
To: Joey Vagedes ; Rebecca Cran ; 
devel@edk2.groups.io; Kinney, Michael D ; Sean 
; Michael Kubacki 
Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI

[You don't often get email from rebe...@bsdio.com. Learn why this is important 
at https://aka.ms/LearnAboutSenderIdentification ]

Thanks, I've updated
https://github.com/tianocore/edk2-basetools/pull/116 with the changes you 
suggested.

Unfortunately several of the tools don't work with the projects.scripts section 
because they don't have Main etc. It turns out even Ecc is currently broken in 
the edk2-basetools PyPI package because PYTHONPATH doesn't contain the parent 
directory. I'll plan to fix the issues in the near future, once I've worked 
through the other PRs.


--

Rebecca Cran


On 2/12/24 10:06, Joey Vagedes wrote:

Hello! Regarding flake8, I see. That will take some time to work through - I 
completely understand.

When ready to work through that, I suggest switching to ruff. It can resolve 
some errors automatically for you and allows you to customize it more than 
flake8 (it is also faster).

I just ran ruff on the project, showing about 11k errors. One nice thing is 
that a large majority of it is regarding importing using *. By fixing those, 
you can reduce the errors by almost 70% (to 3400 errors), so it is not *quite* 
as bad as it seems (though 3400 is still A LOT).

--
-

Regarding the setup tools question, the build-backend part of pytoml (L3) is the entry 
point that is invoked - so we are still invoking setuptools. However, it pulls the 
necessary metadata from pyproject.toml rather than setup.py's setup() function. The main 
thing that is different is the "get_version" functionality, which we replicate 
in the pyproject.toml with setuptools-scm[toml] which simply uses the latest tag to set 
the version.

--
-

Additionally, I wanted to comment on the wrappers that you mentioned.

Using the [projects.scripts] section (i.e. CLI Commands feature) is in my 
opinion the correct way to move forward as users will be able to invoke 
commands themselves easily (i.e. just call Bin2Pcd, and it will work). However, 
the wrappers will continue to be necessary until all python source is removed 
from Basetools, so that users can toggle between pip or local. However, once 
source is removed form BaseTools, removing the wrappers is the next step 
forward.

In the meantime, you will be able to update the binpipwrappers to something 
akin to the following (untested, so I don't garuntee it will be exactly this:

@setlocal
Ecc %*

Rather than

@setlocal
@set ToolName=%~n0%
@%PYTHON_COMMAND% -m edk2basetools.%ToolName%.EccMain %*

--
-

One thing to keep in mind once Python is completely removed from BaseTools, is 
that consumers will need to get in the habit of verifying the correct version 
of basetools is installed for the commit they are on (or automate this 
checking) particularly for anyone that regularly switches between stable 
branches.

Thanks,
Joey

-Original Message-----
From: Rebecca Cran 
Sent: Monday, February 12, 2024 8:42 AM
To: devel@edk2.groups.io; Joey Vagedes ;
Rebecca Cra

Re: [edk2-devel] [PATCH v2 4/4] NetworkPkg: : Updating SecurityFixes.yaml

2024-02-15 Thread Rebecca Cran

I noticed the advisory at 
https://github.com/advisories/GHSA-h9v6-q439-p7j2 is labeled "Unreviewed".
Should it be updated, and should the 'package', 'affected' and 'patched' 
fields be updated?


--
Rebecca Cran


On 2/13/2024 11:46 AM, Doug Flick via groups.io wrote:

From: Doug Flick 

This captures the related security change for Dhcp6Dxe that is related
to CVE-2023-45229

Cc: Saloni Kasbekar 
Cc: Zachary Clark-williams 

Signed-off-by: Doug Flick [MSFT] 
---
  NetworkPkg/SecurityFixes.yaml | 1 +
  1 file changed, 1 insertion(+)

diff --git a/NetworkPkg/SecurityFixes.yaml b/NetworkPkg/SecurityFixes.yaml
index 7e900483fec5..fa42025e0d82 100644
--- a/NetworkPkg/SecurityFixes.yaml
+++ b/NetworkPkg/SecurityFixes.yaml
@@ -8,6 +8,7 @@ CVE_2023_45229:
commit_titles:

  - "NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Patch"

  - "NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Unit Tests"

+- "NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Related Patch"

cve: CVE-2023-45229

date_reported: 2023-08-28 13:56 UTC

description: "Bug 01 - edk2/NetworkPkg: Out-of-bounds read when processing 
IA_NA/IA_TA options in a DHCPv6 Advertise message"





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115523): https://edk2.groups.io/g/devel/message/115523
Mute This Topic: https://groups.io/mt/104339709/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] mail-archive.com (secondary archive) feed resumed

2024-02-15 Thread Rebecca Cran


On 2/15/2024 4:28 AM, Laszlo Ersek wrote:

mail-archive.com is an important secondary archive because it offers
message-id-based lookup. "git am --message-id" captures patch email
message IDs in the commit messages, and then those can be used for
locating the related review discussion in the archive.


Similarly, I've been running an public-inbox instance at 
https://openfw.io/edk2-devel/ for a while.



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115522): https://edk2.groups.io/g/devel/message/115522
Mute This Topic: https://groups.io/mt/104370765/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset

2024-01-24 Thread Rebecca Cran via groups.io


On 1/23/2024 7:10 AM, Sami Mujawar wrote:


@@ -310,6 +318,7 @@ GenericWatchdogEntry (
{
EFI_STATUS Status;
EFI_HANDLE Handle;
+ UINT32 WatchdogIId;
[SAMI] Minor, the above variable name should be WatchdogIid to comply with the 
coding standard.


I was wondering about the naming, since it's the "Watchdog Interface 
ID". Since the second "I" is for another word, should it still be 
lower-case?


--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114351): https://edk2.groups.io/g/devel/message/114351
Mute This Topic: https://groups.io/mt/103832319/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

2024-01-22 Thread Rebecca Cran via groups.io


On 1/22/2024 6:53 PM, Oliver Smith-Denny wrote:


I was able to repro your bug (by just turning on page guards on
ArmVirtQemu, allocating runtime mem and freeing it). I think you
are the first person to free runtime mem on ARM64 with page guards
enabled (and to care when it failed :).

The heap guard code is not written with ARM64 in mind (nor is much of
the codebase, of course). Specifically in this case the heap guard code
only wishes to preserve 4 KB alignment, it knows nothing of ARM64's
runtime page granularity required.

Let me take a look at this, I'm working on a solution here, but I want
to test this out further. I'll try to send a patch later this week or
next.


Thanks! I wonder if the same problem occurs on LoongArch64, which also 
defines the runtime page allocation granularity to be 0x1?



MdePkg/Include/X64/ProcessorBind.h
261:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/LoongArch64/ProcessorBind.h
89:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1)

MdePkg/Include/RiscV64/ProcessorBind.h
120:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/Ia32/ProcessorBind.h
262:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/AArch64/ProcessorBind.h
164:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1)

MdePkg/Include/Arm/ProcessorBind.h
170:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/Ebc/ProcessorBind.h
125:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114164): https://edk2.groups.io/g/devel/message/114164
Mute This Topic: https://groups.io/mt/103810212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

2024-01-22 Thread Rebecca Cran via groups.io


On 1/19/2024 1:03 PM, Oliver Smith-Denny wrote:


Thanks for trying. In lieu of being able to test myself, all I can offer
is adding some more prints, when the memory gets allocated, making sure
it is 64k aligned then. I'd be curious to see what the address is that
is attempting to be freed.

My guess (as it was earlier) is that it is going to be aligned to
64k but + 4k. I.e the guard page at the front is throwing it off. There
may have just been an error in my attempt to fix the check for that.

If however that address is not 64k + 4k aligned, then something else is
afoot.

Happy to look at some more data if you get it or can engineer an example
on an open source system (can you force the system to call this function
twice even without the extra SMBIOS entries, etc.).


These are the addresses it's allocating with and without HeapGuard (with 
the original, upstream Page.c code).



Without HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0xEF11 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0xFB7C)

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0xEF11, 1)
Allocated 0xEF11 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (4153), 0xFB8AEC8E)


--

WITH HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0xED36F000 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0xED38F000)

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0xED36F000, 1)


--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114150): https://edk2.groups.io/g/devel/message/114150
Mute This Topic: https://groups.io/mt/103810212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] EFI_SYSTEM_TABLE allocated by AllocateRuntimeCopyPool isn't aligned to 4KB

2024-01-22 Thread Rebecca Cran


On 1/22/2024 1:24 PM, Oliver Smith-Denny wrote:

Allocating pool memory will never be 4KB aligned because of the
POOL_HEAD. See: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Mem/Pool.c#L463-L537.


The allocated page is 4KB aligned, of course, but we store the POOL_HEAD
at the beginning of that page and return Head->Data to the caller.

In addition, this is only in the case where you allocate a new page for
the pool. In the case where the pool has the pages it needs, you don't
have any alignment guarantee, I believe, it is just offset further
into one of the already allocated pages.


Thanks. I'll update the script to require users to pass in the address 
of the EFI_SYSTEM_TABLE instead of searching for it!



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114149): https://edk2.groups.io/g/devel/message/114149
Mute This Topic: https://groups.io/mt/103894456/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] EFI_SYSTEM_TABLE allocated by AllocateRuntimeCopyPool isn't aligned to 4KB

2024-01-22 Thread Rebecca Cran

I've been working on updating the T32 script EfiLoadDxe.cmm in 
EmbeddedPkg/Scripts/LauterbachT32 and one of the issues I've run into is 
that the EFI_SYSTEM_TABLE - pointed to by `gST` and `gDxeCoreST` - isn't 
aligned to 4KB like the script expects.


Instead, I'm seeing AllocateRuntimeCopyPool return 0xFB7D0018 in 
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:


//
// Allocate the EFI System Table and EFI Runtime Service Table from 
EfiRuntimeServicesData
// Use the templates to initialize the contents of the EFI System Table 
and EFI Runtime Services Table

//
gDxeCoreST = AllocateRuntimeCopyPool (sizeof (EFI_SYSTEM_TABLE), 
);

ASSERT (gDxeCoreST != NULL);

I'm wondering which is wrong: should AllocateRuntimeCopyPool be 
returning a 4KB-aligned pointer, or is the script's assumption wrong?



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114146): https://edk2.groups.io/g/devel/message/114146
Mute This Topic: https://groups.io/mt/103894456/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Multi-ISA Driver Compatibility Survey

2024-01-22 Thread Rebecca Cran

Originally posted at
https://twitter.com/UEFIForum/status/1745518769232077208

The Multi-ISA Driver Compatibility Survey is at
https://docs.google.com/forms/d/e/1FAIpQLScXjwaSBgLeqB1coEDxCPxho5JWF3AMqshOTJ2wd6Tf0He4LA/viewform

From that page:

Did you know Tiano today supports four 64-bit architectures, yet plug-in
device OpRoms are still mostly limited to x64 and CSM? While
binary-translation approaches are a useful stop-gap solution for both
AArch64 and RV64 ecosystems, we need a common approach that is not a
technical debt nightmare and that will be adopted by IHVs and endorsed
by OSVs.

The Multi-ISA Driver Compatibility talk went over some of the possible
approaches, as a lead-in for an open discussion, and raised the
long-term importance of solving cross-architecture compatibility issues
for OpRom drivers.

This survey is an opportunity provide feedback to guide further
exploration in this space.

The discussed options were:

- Do nothing. This is an IHV problem. IHV should continue shipping
support for whatever architectures they deem necessary.
- Resurrect EFI Byte Code. Invest in compilers and tooling (e.g.
addr2line, JIT, etc) to get parity with existing native drivers.
- Look at WASM, and solve tooling constraints (around sandboxing) that
prevent adoption.
- eBPF, and solve tooling constraints (around sandboxing) that prevent
adoption.
- Constrained, well-defined subset of x64. This meets IHVs half-way by
avoiding significant perturbations to existing driver development and
release processes, and achieves compatibility with existing x64 systems
in the market "for free", while addressing most of the concerns around
binary translation of x64 code.

Note: the last three options (WASM, eBPF and constrained x64) are not
neutral with respect to host natural register width, which will mean a
break in compatibility with future TBD 128-bit environments, unless a
TBD OpRom sandboxing technology is invented.

Note 2: emails and names/sign-ins are not collected, this is an
anonymous response.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114144): https://edk2.groups.io/g/devel/message/114144
Mute This Topic: https://groups.io/mt/103893425/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned


On 1/18/2024 12:26 PM, Oliver Smith-Denny wrote:


Does this solve your issue? I have to run to a meeting, but I can
write this in actual patch form (and give it a quick test) later.


Unfortunately that didn't work: I still get the assert.

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table

ASSERT_EFI_ERROR (Status = Invalid Parameter)
ASSERT [SmbiosDxe] 
/local-data/src/ampereone/edk2/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c(145): 
!(((INTN)(RETURN_STATUS)(Status)) < 0)


--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114090): https://edk2.groups.io/g/devel/message/114090
Mute This Topic: https://groups.io/mt/103810212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements

Fixes and improvements to GenericWatchdogDxe.

PR: https://github.com/tianocore/edk2/pull/5176

Changes between v3 and v4:

- Check Interface Identification Register for architecture revision before
  setting the high offset register value.
- Use @par for reference.
- Move setting of EBS flag from patch 2/3 to 3/3.
- Disable the watchdog in the case that EBS has been called and the timer
  period is non-zero.

Rebecca Cran (3):
  ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
  ArmPkg: Introduce global mTimerPeriod and remove calculation
  ArmPkg: Disable watchdog interaction after exiting boot services

 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 11 +++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 69 
+---
 2 files changed, 54 insertions(+), 26 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114086): https://edk2.groups.io/g/devel/message/114086
Mute This Topic: https://groups.io/mt/103832317/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation

The calculation of the timer period was broken. Introduce a global
mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz
is only used in one place, remove the global and make it a local
variable. Do the same with mNumTimerTicks.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 35 
+---
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 20549aa91d94..8dd247c44e8f 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -28,13 +28,10 @@
in a second */
 #define TIME_UNITS_PER_SECOND  1000
 
-// Tick frequency of the generic timer basis of the generic watchdog.
-STATIC UINTN  mTimerFrequencyHz = 0;
-
 /* In cases where the compare register was set manually, information about
how long the watchdog was asked to wait cannot be retrieved from hardware.
It is therefore stored here. 0 means the timer is not running. */
-STATIC UINT64  mNumTimerTicks = 0;
+STATIC UINT64  mTimerPeriod = 0;
 
 STATIC UINT8  WatchdogRevision;
 
@@ -95,7 +92,7 @@ WatchdogExitBootServicesEvent (
   )
 {
   WatchdogDisable ();
-  mNumTimerTicks = 0;
+  mTimerPeriod= 0;
 }
 
 /* This function is called when the watchdog's first signal (WS0) goes high.
@@ -110,7 +107,6 @@ WatchdogInterruptHandler (
   )
 {
   STATIC CONST CHAR16  ResetString[] = L"The generic watchdog timer ran out.";
-  UINT64   TimerPeriod;
 
   WatchdogDisable ();
 
@@ -123,8 +119,7 @@ WatchdogInterruptHandler (
   // the timer period plus 1.
   //
   if (mWatchdogNotify != NULL) {
-TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
-mWatchdogNotify (TimerPeriod + 1);
+mWatchdogNotify (mTimerPeriod + 1);
   }
 
   gRT->ResetSystem (
@@ -204,22 +199,27 @@ WatchdogSetTimerPeriod (
   IN UINT64TimerPeriod  // In 100ns units
   )
 {
-  UINTN  SystemCount;
+  UINTN   SystemCount;
+  UINT64  TimerFrequencyHz;
+  UINT64  NumTimerTicks;
 
   // if TimerPeriod is 0, this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
-mNumTimerTicks = 0;
+mTimerPeriod = 0;
 WatchdogDisable ();
 return EFI_SUCCESS;
   }
 
   // Work out how many timer ticks will equate to TimerPeriod
-  mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
+  TimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
+  ASSERT (TimerFrequencyHz != 0);
+  mTimerPeriod  = TimerPeriod;
+  NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
 
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT48) {
+  if (NumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
@@ -227,9 +227,9 @@ WatchdogSetTimerPeriod (
 WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
-WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
+WatchdogWriteCompareRegister (SystemCount + NumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister (mNumTimerTicks);
+WatchdogWriteOffsetRegister (NumTimerTicks);
 WatchdogEnable ();
   }
 
@@ -264,7 +264,7 @@ WatchdogGetTimerPeriod (
 return EFI_INVALID_PARAMETER;
   }
 
-  *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+  *TimerPeriod = mTimerPeriod;
 
   return EFI_SUCCESS;
 }
@@ -332,9 +332,6 @@ GenericWatchdogEntry (
  This will avoid conflicts with the universal watchdog */
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, );
 
-  mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
-  ASSERT (mTimerFrequencyHz != 0);
-
   // Install interrupt handler
   Status = mInterruptProtocol->RegisterInterruptSource (
  mInterruptProtocol,
@@ -376,9 +373,9 @@ GenericWatchdogEntry (
   );
   ASSERT_EFI_ERROR (Status);
 
-  mNumTimerTicks   = 0;
   WatchdogIId  = MmioRead32 (GENERIC_WDOG_IID_REG);
   WatchdogRevision = (WatchdogIId >> GENERIC_WDOG_IID_REV_SHIFT) & 
GENERIC_WDOG_IID_REV_MASK;
+  mTimerPeriod = 0;
   WatchdogDisable ();
 
   return EFI_SUCCESS;
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114087): https://edk2.groups.io/g/devel/message/114087
Mute This Topic: https://groups.io/mt/103832318/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset

The generic watchdog offset register is 48 bits wide, and can be set by
performing two 32-bit writes.

Add support for writing the high 16 bits of the offset register and
update the signature of the WatchdogWriteOffsetRegister function to take
a UINT64 value.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 11 +-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 23 
+++-
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
index 9bc3bf47047c..fb117832683f 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
@@ -1,9 +1,13 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
+*  @par Reference(s):
+*  - Generic Watchdog specification in Arm Base System Architecture 1.0C:
+*https://developer.arm.com/documentation/den0094/c/
 **/
 
 #ifndef GENERIC_WATCHDOG_H_
@@ -14,12 +18,17 @@
 
 // Control Frame:
 #define GENERIC_WDOG_CONTROL_STATUS_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x000)
-#define GENERIC_WDOG_OFFSET_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_LOW  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x00C)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x010)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x014)
+#define GENERIC_WDOG_IID_REG ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0xFCC)
 
 // Values of bit 0 of the Control/Status Register
 #define GENERIC_WDOG_ENABLED   1
 #define GENERIC_WDOG_DISABLED  0
 
+#define GENERIC_WDOG_IID_REV_SHIFT  16
+#define GENERIC_WDOG_IID_REV_MASK   0xF
+
 #endif // GENERIC_WATCHDOG_H_
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 66c6c37c08b0..20549aa91d94 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -1,5 +1,6 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -35,16 +36,23 @@ STATIC UINTN  mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mNumTimerTicks = 0;
 
+STATIC UINT8  WatchdogRevision;
+
+#define MAX_UINT48  0xULL
+
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
 
 STATIC
 VOID
 WatchdogWriteOffsetRegister (
-  UINT32  Value
+  UINT64  Value
   )
 {
-  MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32);
+  if (WatchdogRevision == 1) {
+MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT16);
+  }
 }
 
 STATIC
@@ -211,17 +219,17 @@ WatchdogSetTimerPeriod (
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT32) {
+  if (mNumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
trigger an interrupt, set the offset to max. */
-WatchdogWriteOffsetRegister (MAX_UINT32);
+WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
 WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks);
+WatchdogWriteOffsetRegister (mNumTimerTicks);
 WatchdogEnable ();
   }
 
@@ -310,6 +318,7 @@ GenericWatchdogEntry (
 {
   EFI_STATUS  Status;
   EFI_HANDLE  Handle;
+  UINT32  WatchdogIId;
 
   Status = gBS->LocateProtocol (
   ,
@@ -367,7 +376,9 @@ GenericWatchdogEntry (
   );
   ASSERT_EFI_ERROR (Status);
 
-  mNumTimerTicks = 0;
+  mNumTimerTicks   = 0;
+  WatchdogIId  = MmioRead32 (GENERIC_WDOG_IID_REG);
+  WatchdogRevision = (WatchdogIId >> GENERIC_WDOG_IID_REV_SHIFT) & 
GENERIC_WDOG_IID_REV_MASK;
   WatchdogDisable ();
 
   return EFI_SUCCESS;
-- 
2.34.1



-

[edk2-devel] [PATCH v4 3/3] ArmPkg: Disable watchdog interaction after exiting boot services

Update GenericWatchdogDxe to disable watchdog interaction after exiting
boot services. Also, move the mEfiExitBootServicesEvent event to the top
of the file with the other static variables.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 8dd247c44e8f..189194a2b9ee 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -35,10 +35,14 @@ STATIC UINT64  mTimerPeriod = 0;
 
 STATIC UINT8  WatchdogRevision;
 
+/* disables watchdog interaction after Exit Boot Services */
+STATIC BOOLEAN  mExitedBootServices = FALSE;
+
 #define MAX_UINT48  0xULL
 
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;
 
 STATIC
 VOID
@@ -93,6 +97,7 @@ WatchdogExitBootServicesEvent (
 {
   WatchdogDisable ();
   mTimerPeriod= 0;
+  mExitedBootServices = TRUE;
 }
 
 /* This function is called when the watchdog's first signal (WS0) goes high.
@@ -203,7 +208,15 @@ WatchdogSetTimerPeriod (
   UINT64  TimerFrequencyHz;
   UINT64  NumTimerTicks;
 
-  // if TimerPeriod is 0, this is a request to stop the watchdog.
+  // If we've exited Boot Services but TimerPeriod isn't zero, this
+  // indicates that the caller is doing something wrong.
+  if (mExitedBootServices && (TimerPeriod != 0)) {
+mTimerPeriod = 0;
+WatchdogDisable ();
+return EFI_DEVICE_ERROR;
+  }
+
+  // If TimerPeriod is 0 this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
 mTimerPeriod = 0;
 WatchdogDisable ();
@@ -307,8 +320,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL  mWatchdogTimer = {
   WatchdogGetTimerPeriod
 };
 
-STATIC EFI_EVENT  mEfiExitBootServicesEvent;
-
 EFI_STATUS
 EFIAPI
 GenericWatchdogEntry (
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114089): https://edk2.groups.io/g/devel/message/114089
Mute This Topic: https://groups.io/mt/103832320/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 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation


On 1/5/2024 1:26 AM, Ard Biesheuvel wrote:


@@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent (
)
  {
WatchdogDisable ();
-  mNumTimerTicks = 0;
+  mTimerPeriod= 0;
+  mExitedBootServices = TRUE;


Where is this declared/defined?


Oh, it's defined in the 3rd patch - which obviously breaks bisect. I'll 
fix it.


--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114019): https://edk2.groups.io/g/devel/message/114019
Mute This Topic: https://groups.io/mt/103538116/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 3/3] ArmPkg: Disable watchdog interaction after exiting boot services


On 1/5/2024 4:12 AM, Sami Mujawar wrote:


- // if TimerPeriod is 0, this is a request to stop the watchdog.
+ // If we've exited Boot Services but TimerPeriod isn't zero, this
+ // indicates that the caller is doing something wrong.
+ if (mExitedBootServices && (TimerPeriod != 0)) {
[SAMI] Thanks for updating the code to return the error code.
However, I see you are not stopping the watchdog timer. Is this because you 
expect the watchdog period to expire and reset the system?


I removed the code to stop the watchdog timer because it's an error 
condition. However, I've updated it so it does also get stopped in this 
case too.



Also, did you see an issue that motivated this patch, or this was just a case 
of hardening the code?
Can you provide more information, please?
[/SAMI]


This was issues found and improvements made by various people in the 
last couple of years that we're now upstreaming to contribute 
improvements and reduce our diffs against upstream.


--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114018): https://edk2.groups.io/g/devel/message/114018
Mute This Topic: https://groups.io/mt/103538118/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned


On 1/18/2024 12:04 PM, Oliver Smith-Denny wrote:

On 1/18/2024 10:45 AM, Rebecca Cran via groups.io wrote:
No, I mean SbsaQemu from edk2-platforms: 
https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu




Sure, if you can repro there that is helpful.


I've realized it won't repro there or other virtual platforms because it 
requires that SmbiosDxe run the reallocation code, which I believe is 
only done on systems with a larger number of SMBIOS tables - for example 
where RAS etc. is supported.


--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114007): https://edk2.groups.io/g/devel/message/114007
Mute This Topic: https://groups.io/mt/103810212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned


On 1/18/2024 11:38 AM, Oliver Smith-Denny wrote:


Yeah, if you can get it running there, that would be a good data point.
I assume you mean the Project Mu QemuSbsaPkg? If so that is great, but
you will need to update the RUNTIME_PAGE_ALLOCATION_GRANULARITY back to
0x1. It was set to 0x1000 for a historical issue that we are
working on reconciling with what edk2 has.


No, I mean SbsaQemu from edk2-platforms: 
https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu



To clarify, if you turn off pool guard, does the assert go away?


Yes.

--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114004): https://edk2.groups.io/g/devel/message/114004
Mute This Topic: https://groups.io/mt/103810212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned


On 1/18/2024 9:48 AM, Oliver Smith-Denny via groups.io wrote:

Are you including this commit: 
https://github.com/tianocore/edk2/commit/00b51e0d78a547dd78119ec44fcc74a01b6f79c8?


Can you share some more details on where this is failing? I.e. what
assert is getting tripped? Presumably without HeapGuard enabled, you
aren't seeing the failure?

Are you hitting this case:
https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Core/Dxe/Mem/Page.c#L1570-L1573?

Does this repro on ArmVirtPkg?


Yes, I have that commit in my tree.

I'm hitting this assert in FreePages: 
https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Library/DxeCoreMemoryAllocationLib/MemoryAllocationLib.c#L190


It's called by SmbiosCreate64BitTable: 
https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c#L1342


And yes, that's the case I'm hitting.

I'm having trouble getting ArmVirtPkg to run. Would it be useful testing 
using SbsaQemu instead?


--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114002): https://edk2.groups.io/g/devel/message/114002
Mute This Topic: https://groups.io/mt/103810212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

I've been debugging an assert failure when using HeapGuard on AArch64. A 
call to FreePages in SmbiosDxe is failing because the memory is aligned 
to 0x1000 instead of 0x1 as defined by 
RUNTIME_PAGE_ALLOCATION_GRANULARITY.


I'm enabling HeapGuard by setting the PCDs to the following values:

gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC000
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC000

--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113995): https://edk2.groups.io/g/devel/message/113995
Mute This Topic: https://groups.io/mt/103810212/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] BaseTools: Fix raw strings containing valid escape characters

2024-01-10 Thread Rebecca Cran


On 1/9/2024 11:52 PM, Huang, Yanbo wrote:


@Gao, Liming <mailto:gaolim...@byosoft.com.cn> @'Rebecca Cran' 
<mailto:rebe...@bsdio.com>


Could you please help to merge this PR?

BaseTools: Fix raw strings containing valid escape characters by 
YuweiChen1110 · Pull Request #5238 · tianocore/edk2 (github.com) 
<https://github.com/tianocore/edk2/pull/5238>


Thanks in advance!


It's been merged.


--

Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113538): https://edk2.groups.io/g/devel/message/113538
Mute This Topic: https://groups.io/mt/103616495/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] ArmPkg: GenericWatchdogDxe fixes and improvements

2024-01-05 Thread Rebecca Cran via groups.io


On 1/5/2024 1:26 AM, Ard Biesheuvel via groups.io wrote:

On Fri, 5 Jan 2024 at 06:15, Rebecca Cran
 wrote:


Fixes and improvements to GenericWatchdogDxe.



What is the difference between v2 and v3?


Sorry, I should have said that. I forgot to build v2 and it had a bug in 
the exit boot services handler where I'd forgotten to update a line.


--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113298): https://edk2.groups.io/g/devel/message/113298
Mute This Topic: https://groups.io/mt/103538117/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset

The generic watchdog offset register is 48 bits wide, and can be set by
performing two 32-bit writes.

Add support for writing the high 16 bits of the offset register and
update the signature of the WatchdogWriteOffsetRegister function to take
a UINT64 value.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h|  6 +-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +-
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
index 9bc3bf47047c..2a0634e7e9f1 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
@@ -1,9 +1,12 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
+*  See Generic Watchdog specification in Arm Base System Architecture 1.0C:
+*  https://developer.arm.com/documentation/den0094/c/
 **/
 
 #ifndef GENERIC_WATCHDOG_H_
@@ -14,7 +17,8 @@
 
 // Control Frame:
 #define GENERIC_WDOG_CONTROL_STATUS_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x000)
-#define GENERIC_WDOG_OFFSET_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_LOW  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x00C)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x010)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x014)
 
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 66c6c37c08b0..f8c39458a53a 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -1,5 +1,6 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -35,16 +36,19 @@ STATIC UINTN  mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mNumTimerTicks = 0;
 
+#define MAX_UINT48  0xULL
+
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
 
 STATIC
 VOID
 WatchdogWriteOffsetRegister (
-  UINT32  Value
+  UINT64  Value
   )
 {
-  MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT16);
 }
 
 STATIC
@@ -211,17 +215,17 @@ WatchdogSetTimerPeriod (
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT32) {
+  if (mNumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
trigger an interrupt, set the offset to max. */
-WatchdogWriteOffsetRegister (MAX_UINT32);
+WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
 WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks);
+WatchdogWriteOffsetRegister (mNumTimerTicks);
 WatchdogEnable ();
   }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113215): https://edk2.groups.io/g/devel/message/113215
Mute This Topic: https://groups.io/mt/103538115/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation

The calculation of the timer period was broken. Introduce a global
mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz
is only used in one place, remove the global and make it a local
variable. Do the same with mNumTimerTicks.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 36 
++
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index f8c39458a53a..78cee62a19d6 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -28,13 +28,10 @@
in a second */
 #define TIME_UNITS_PER_SECOND  1000
 
-// Tick frequency of the generic timer basis of the generic watchdog.
-STATIC UINTN  mTimerFrequencyHz = 0;
-
 /* In cases where the compare register was set manually, information about
how long the watchdog was asked to wait cannot be retrieved from hardware.
It is therefore stored here. 0 means the timer is not running. */
-STATIC UINT64  mNumTimerTicks = 0;
+STATIC UINT64  mTimerPeriod = 0;
 
 #define MAX_UINT48  0xULL
 
@@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent (
   )
 {
   WatchdogDisable ();
-  mNumTimerTicks = 0;
+  mTimerPeriod= 0;
+  mExitedBootServices = TRUE;
 }
 
 /* This function is called when the watchdog's first signal (WS0) goes high.
@@ -106,7 +104,6 @@ WatchdogInterruptHandler (
   )
 {
   STATIC CONST CHAR16  ResetString[] = L"The generic watchdog timer ran out.";
-  UINT64   TimerPeriod;
 
   WatchdogDisable ();
 
@@ -119,8 +116,7 @@ WatchdogInterruptHandler (
   // the timer period plus 1.
   //
   if (mWatchdogNotify != NULL) {
-TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
-mWatchdogNotify (TimerPeriod + 1);
+mWatchdogNotify (mTimerPeriod + 1);
   }
 
   gRT->ResetSystem (
@@ -200,22 +196,27 @@ WatchdogSetTimerPeriod (
   IN UINT64TimerPeriod  // In 100ns units
   )
 {
-  UINTN  SystemCount;
+  UINTN   SystemCount;
+  UINT64  TimerFrequencyHz;
+  UINT64  NumTimerTicks;
 
   // if TimerPeriod is 0, this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
-mNumTimerTicks = 0;
+mTimerPeriod = 0;
 WatchdogDisable ();
 return EFI_SUCCESS;
   }
 
   // Work out how many timer ticks will equate to TimerPeriod
-  mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
+  TimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
+  ASSERT (TimerFrequencyHz != 0);
+  mTimerPeriod  = TimerPeriod;
+  NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
 
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT48) {
+  if (NumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
@@ -223,9 +224,9 @@ WatchdogSetTimerPeriod (
 WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
-WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
+WatchdogWriteCompareRegister (SystemCount + NumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister (mNumTimerTicks);
+WatchdogWriteOffsetRegister (NumTimerTicks);
 WatchdogEnable ();
   }
 
@@ -260,7 +261,7 @@ WatchdogGetTimerPeriod (
 return EFI_INVALID_PARAMETER;
   }
 
-  *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+  *TimerPeriod = mTimerPeriod;
 
   return EFI_SUCCESS;
 }
@@ -327,9 +328,6 @@ GenericWatchdogEntry (
  This will avoid conflicts with the universal watchdog */
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, );
 
-  mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
-  ASSERT (mTimerFrequencyHz != 0);
-
   // Install interrupt handler
   Status = mInterruptProtocol->RegisterInterruptSource (
  mInterruptProtocol,
@@ -371,7 +369,7 @@ GenericWatchdogEntry (
   );
   ASSERT_EFI_ERROR (Status);
 
-  mNumTimerTicks = 0;
+  mTimerPeriod = 0;
   WatchdogDisable ();
 
   return EFI_SUCCESS;
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113216): https://edk2.groups.io/g/devel/message/113216
Mute This Topic: https://groups.io/mt/103538116/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements

Fixes and improvements to GenericWatchdogDxe.

PR: https://github.com/tianocore/edk2/pull/5176

Rebecca Cran (3):
  ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
  ArmPkg: Introduce global mTimerPeriod and remove calculation
  ArmPkg: Disable watchdog interaction after exiting boot services

 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h|  6 ++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 60 
+-
 2 files changed, 40 insertions(+), 26 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113217): https://edk2.groups.io/g/devel/message/113217
Mute This Topic: https://groups.io/mt/103538117/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 3/3] ArmPkg: Disable watchdog interaction after exiting boot services

Update GenericWatchdogDxe to disable watchdog interaction after exiting
boot services. Also, move the mEfiExitBootServicesEvent event to the top
of the file with the other static variables.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 78cee62a19d6..ddf131660f9d 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -33,10 +33,14 @@
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mTimerPeriod = 0;
 
+/* disables watchdog interaction after Exit Boot Services */
+STATIC BOOLEAN  mExitedBootServices = FALSE;
+
 #define MAX_UINT48  0xULL
 
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;
 
 STATIC
 VOID
@@ -200,7 +204,13 @@ WatchdogSetTimerPeriod (
   UINT64  TimerFrequencyHz;
   UINT64  NumTimerTicks;
 
-  // if TimerPeriod is 0, this is a request to stop the watchdog.
+  // If we've exited Boot Services but TimerPeriod isn't zero, this
+  // indicates that the caller is doing something wrong.
+  if (mExitedBootServices && (TimerPeriod != 0)) {
+return EFI_DEVICE_ERROR;
+  }
+
+  // If TimerPeriod is 0 this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
 mTimerPeriod = 0;
 WatchdogDisable ();
@@ -304,8 +314,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL  mWatchdogTimer = {
   WatchdogGetTimerPeriod
 };
 
-STATIC EFI_EVENT  mEfiExitBootServicesEvent;
-
 EFI_STATUS
 EFIAPI
 GenericWatchdogEntry (
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113218): https://edk2.groups.io/g/devel/message/113218
Mute This Topic: https://groups.io/mt/103538118/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/3] ArmPkg: Fix the calculation of the timer period in GenericWatchdogDxe


Thanks, I've incorporated the changes into the v2 patch series.

--
Rebecca

On 1/3/2024 3:56 PM, Ard Biesheuvel wrote:

Hi Rebecca,

On Wed, 3 Jan 2024 at 21:44, Rebecca Cran
 wrote:


Fix the calculation of the timer period in GenericWatchdogDxe: we need
to multiply before dividing to keep the values as integers.

Signed-off-by: Rebecca Cran 
---
  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 05df101d5f4b..8f02f38c64e3 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -119,7 +119,7 @@ WatchdogInterruptHandler (
// the timer period plus 1.
//
if (mWatchdogNotify != NULL) {
-TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / 
mTimerFrequencyHz);


Could we just store the timer period in a global mTimerPeriod, and get
rid of mNumTimerTicks entirely? AFAICT, that would get rid of these
calculations as well.


  mWatchdogNotify (TimerPeriod + 1);
}

@@ -260,7 +260,7 @@ WatchdogGetTimerPeriod (
  return EFI_INVALID_PARAMETER;
}

-  *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+  *TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / 
mTimerFrequencyHz);

return EFI_SUCCESS;
  }
--
2.34.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113214): https://edk2.groups.io/g/devel/message/113214
Mute This Topic: https://groups.io/mt/103510102/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 3/3] ArmPkg: Disable watchdog interaction after exiting boot services


Thanks, I've incorporated the changes into the v2 patch series.

--
Rebecca

On 1/4/2024 3:01 AM, Sami Mujawar wrote:

Hi Rebecca,

Thank you for this patch.

I have some minor suggestions marked inline as [SAMI].

Regards,

Sami Mujawar

On 03/01/2024, 20:44, "Rebecca Cran" mailto:rebe...@os.amperecomputing.com>> wrote:


Update GenericWatchdogDxe to disable watchdog interaction after exiting
boot services. Also, move the mEfiExitBootServicesEvent event to the top
of the file with the other static variables.


Signed-off-by: Rebecca Cran mailto:rebe...@os.amperecomputing.com>>
---
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)


diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 8f02f38c64e3..912106eb6ad2 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -36,10 +36,14 @@ STATIC UINTN mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
STATIC UINT64 mNumTimerTicks = 0;


+/* disables watchdog interaction after Exit Boot Services */
+STATIC BOOLEAN mExitedBootServices = FALSE;
+
#define MAX_UINT48 0xULL


STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;


STATIC
VOID
@@ -91,7 +95,8 @@ WatchdogExitBootServicesEvent (
)
{
WatchdogDisable ();
- mNumTimerTicks = 0;
+ mNumTimerTicks = 0;
+ mExitedBootServices = TRUE;
}


/* This function is called when the watchdog's first signal (WS0) goes high.
@@ -202,8 +207,9 @@ WatchdogSetTimerPeriod (
{
UINTN SystemCount;


- // if TimerPeriod is 0, this is a request to stop the watchdog.
- if (TimerPeriod == 0) {
+ // if TimerPeriod is 0 or we've exited boot services,
+ // this is a request to stop the watchdog.
+ if (TimerPeriod == 0 || mExitedBootServices) {
[SAMI] I think we need extra brackets here. Also, should EFI_DEVICE_ERROR be 
returned if ExitBootServices is signalled but TimerPeriod != 0?
  i.e. we stop the watchdog but return an error to the caller so that it knows 
to fix the broken code.
[/SAMI]
mNumTimerTicks = 0;
WatchdogDisable ();
return EFI_SUCCESS;
@@ -303,8 +309,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = {
WatchdogGetTimerPeriod
};


-STATIC EFI_EVENT mEfiExitBootServicesEvent;
-
EFI_STATUS
EFIAPI
GenericWatchdogEntry (




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113213): https://edk2.groups.io/g/devel/message/113213
Mute This Topic: https://groups.io/mt/103510105/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation

The calculation of the timer period was broken. Introduce a global
mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz
is only used in one place, remove the global and make it a local
variable. Do the same with mNumTimerTicks.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 36 
++
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index f8c39458a53a..f74d0d1ced2e 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -28,13 +28,10 @@
in a second */
 #define TIME_UNITS_PER_SECOND  1000
 
-// Tick frequency of the generic timer basis of the generic watchdog.
-STATIC UINTN  mTimerFrequencyHz = 0;
-
 /* In cases where the compare register was set manually, information about
how long the watchdog was asked to wait cannot be retrieved from hardware.
It is therefore stored here. 0 means the timer is not running. */
-STATIC UINT64  mNumTimerTicks = 0;
+STATIC UINT64  mTimerPeriod = 0;
 
 #define MAX_UINT48  0xULL
 
@@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent (
   )
 {
   WatchdogDisable ();
-  mNumTimerTicks = 0;
+  mNumTimerTicks  = 0;
+  mExitedBootServices = TRUE;
 }
 
 /* This function is called when the watchdog's first signal (WS0) goes high.
@@ -106,7 +104,6 @@ WatchdogInterruptHandler (
   )
 {
   STATIC CONST CHAR16  ResetString[] = L"The generic watchdog timer ran out.";
-  UINT64   TimerPeriod;
 
   WatchdogDisable ();
 
@@ -119,8 +116,7 @@ WatchdogInterruptHandler (
   // the timer period plus 1.
   //
   if (mWatchdogNotify != NULL) {
-TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
-mWatchdogNotify (TimerPeriod + 1);
+mWatchdogNotify (mTimerPeriod + 1);
   }
 
   gRT->ResetSystem (
@@ -200,22 +196,27 @@ WatchdogSetTimerPeriod (
   IN UINT64TimerPeriod  // In 100ns units
   )
 {
-  UINTN  SystemCount;
+  UINTN   SystemCount;
+  UINT64  TimerFrequencyHz;
+  UINT64  NumTimerTicks;
 
   // if TimerPeriod is 0, this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
-mNumTimerTicks = 0;
+mTimerPeriod = 0;
 WatchdogDisable ();
 return EFI_SUCCESS;
   }
 
   // Work out how many timer ticks will equate to TimerPeriod
-  mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
+  TimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
+  ASSERT (TimerFrequencyHz != 0);
+  mTimerPeriod  = TimerPeriod;
+  NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND;
 
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT48) {
+  if (NumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
@@ -223,9 +224,9 @@ WatchdogSetTimerPeriod (
 WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
-WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
+WatchdogWriteCompareRegister (SystemCount + NumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister (mNumTimerTicks);
+WatchdogWriteOffsetRegister (NumTimerTicks);
 WatchdogEnable ();
   }
 
@@ -260,7 +261,7 @@ WatchdogGetTimerPeriod (
 return EFI_INVALID_PARAMETER;
   }
 
-  *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+  *TimerPeriod = mTimerPeriod;
 
   return EFI_SUCCESS;
 }
@@ -327,9 +328,6 @@ GenericWatchdogEntry (
  This will avoid conflicts with the universal watchdog */
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, );
 
-  mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
-  ASSERT (mTimerFrequencyHz != 0);
-
   // Install interrupt handler
   Status = mInterruptProtocol->RegisterInterruptSource (
  mInterruptProtocol,
@@ -371,7 +369,7 @@ GenericWatchdogEntry (
   );
   ASSERT_EFI_ERROR (Status);
 
-  mNumTimerTicks = 0;
+  mTimerPeriod = 0;
   WatchdogDisable ();
 
   return EFI_SUCCESS;
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113211): https://edk2.groups.io/g/devel/message/113211
Mute This Topic: https://groups.io/mt/103537819/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 3/3] ArmPkg: Disable watchdog interaction after exiting boot services

Update GenericWatchdogDxe to disable watchdog interaction after exiting
boot services. Also, move the mEfiExitBootServicesEvent event to the top
of the file with the other static variables.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index f74d0d1ced2e..44d812193031 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -33,10 +33,14 @@
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mTimerPeriod = 0;
 
+/* disables watchdog interaction after Exit Boot Services */
+STATIC BOOLEAN  mExitedBootServices = FALSE;
+
 #define MAX_UINT48  0xULL
 
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;
 
 STATIC
 VOID
@@ -200,7 +204,13 @@ WatchdogSetTimerPeriod (
   UINT64  TimerFrequencyHz;
   UINT64  NumTimerTicks;
 
-  // if TimerPeriod is 0, this is a request to stop the watchdog.
+  // If we've exited Boot Services but TimerPeriod isn't zero, this
+  // indicates that the caller is doing something wrong.
+  if (mExitedBootServices && (TimerPeriod != 0)) {
+return EFI_DEVICE_ERROR;
+  }
+
+  // If TimerPeriod is 0 this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
 mTimerPeriod = 0;
 WatchdogDisable ();
@@ -304,8 +314,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL  mWatchdogTimer = {
   WatchdogGetTimerPeriod
 };
 
-STATIC EFI_EVENT  mEfiExitBootServicesEvent;
-
 EFI_STATUS
 EFIAPI
 GenericWatchdogEntry (
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113212): https://edk2.groups.io/g/devel/message/113212
Mute This Topic: https://groups.io/mt/103537820/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/3] ArmPkg: GenericWatchdogDxe fixes and improvements

Fixes and improvements to GenericWatchdogDxe.

PR: https://github.com/tianocore/edk2/pull/5176

Rebecca Cran (3):
  ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
  ArmPkg: Introduce global mTimerPeriod and remove calculation
  ArmPkg: Disable watchdog interaction after exiting boot services

 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h|  6 ++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 60 
+-
 2 files changed, 40 insertions(+), 26 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113209): https://edk2.groups.io/g/devel/message/113209
Mute This Topic: https://groups.io/mt/103537817/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset

The generic watchdog offset register is 48 bits wide, and can be set by
performing two 32-bit writes.

Add support for writing the high 16 bits of the offset register and
update the signature of the WatchdogWriteOffsetRegister function to take
a UINT64 value.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h|  6 +-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +-
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
index 9bc3bf47047c..2a0634e7e9f1 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
@@ -1,9 +1,12 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
+*  See Generic Watchdog specification in Arm Base System Architecture 1.0C:
+*  https://developer.arm.com/documentation/den0094/c/
 **/
 
 #ifndef GENERIC_WATCHDOG_H_
@@ -14,7 +17,8 @@
 
 // Control Frame:
 #define GENERIC_WDOG_CONTROL_STATUS_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x000)
-#define GENERIC_WDOG_OFFSET_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_LOW  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x00C)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x010)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x014)
 
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 66c6c37c08b0..f8c39458a53a 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -1,5 +1,6 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -35,16 +36,19 @@ STATIC UINTN  mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mNumTimerTicks = 0;
 
+#define MAX_UINT48  0xULL
+
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
 
 STATIC
 VOID
 WatchdogWriteOffsetRegister (
-  UINT32  Value
+  UINT64  Value
   )
 {
-  MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT16);
 }
 
 STATIC
@@ -211,17 +215,17 @@ WatchdogSetTimerPeriod (
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT32) {
+  if (mNumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
trigger an interrupt, set the offset to max. */
-WatchdogWriteOffsetRegister (MAX_UINT32);
+WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
 WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks);
+WatchdogWriteOffsetRegister (mNumTimerTicks);
 WatchdogEnable ();
   }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113210): https://edk2.groups.io/g/devel/message/113210
Mute This Topic: https://groups.io/mt/103537818/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Updates to .mailmap needed for Jeff Brasen, Jake Garver, Joey Vagades and Michael Roth?

I noticed recent commits by Jeff Brasen, Jake Garver, Joey Vagades and 
Michael Roth have funky Author lines, which I think means .mailmap needs 
updated?


commit 7a5823f85be99b9a92751fcf4141f7982fa5cc80
Author: Jeff Brasen via groups.io 
Date:   Thu Dec 28 12:47:08 2023 -0800

EmbeddedPkg: Add DtPlatformLoaderLib gmock support

Add Google Mock Library for DtPlatformLoaderDtbLib

Signed-off-by: Jeff Brasen 



commit 9f0061a03b61d282fbc0ba5be22155d06a5e64a1
Author: Joey Vagedes via groups.io 
Date:   Wed Dec 6 12:27:02 2023 -0800

BaseTools: Resolve regex syntax warnings

Switches regex patterns to raw text to resolve python 3.12 syntax
warnings in regards to invalid escape sequences, as is suggested by the
re (regex) module in python.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Joey Vagedes 
Reviewed-by: Rebecca Cran 


Author: Jake Garver via groups.io 
Date:   Tue Oct 3 23:04:54 2023 +0800

MdeModulePkg/RegularExpressinoDxe: Fix clang error

Ignore old style declaration warnings in oniguruma/src/st.c.  This was
already ignored for MSFT, but newer versions of clang complain as well.

Signed-off-by: Jake Garver 
Reviewed-by: Nhi Pham 
Tested-by: Nhi Pham 
Reviewed-by: Liming Gao 

commit f008890ae55929f7f17e7d2f8aff929255007d33
Author: Roth, Michael via groups.io 
Date:   Wed Aug 16 15:11:45 2023 -0500

OvmfPkg/AmdSev: fix BdsPlatform.c assertion failure during boot

Booting an SEV guest with AmdSev OVMF package currently triggers the
following assertion with QEMU:

  InstallQemuFwCfgTables: installed 7 tables
  PcRtc: Write 0x20 to CMOS location 0x32
  [Variable]END_OF_DXE is signaled
  Initialize variable error flag (FF)

  ASSERT_EFI_ERROR (Status = Not Found)
  ASSERT [BdsDxe] 
/home/VT_BUILD/ovmf/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c(1711): 
!(((INTN)(RETURN_STATUS)(Status)) < 0)


This seems to be due to commit 81dc0d8b4c, which switched to using
PlatformBootManagerLib instead of PlatformBootManagerLibGrub. That 
pulls

in a dependency on gEfiS3SaveStateProtocolGuid provider being available
(which is asserted for in
BdsPlatform.c:PlatformBootManagerBeforeConsole()/SaveS3BootScript()),
but the libraries that provide it aren't currently included in the
build. Add them similarly to what's done for OvmfPkg.

Fixes: 81dc0d8b4c ("OvmfPkg/AmdSev: stop using 
PlatformBootManagerLibGrub")

Signed-off-by: Michael Roth 
Acked-by: Gerd Hoffmann 
Acked-by: Jiewen Yao 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113201): https://edk2.groups.io/g/devel/message/113201
Mute This Topic: https://groups.io/mt/103534194/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH v1 2/7] Platform/Sgi: add no-stack-protector flag for StMM builds

Wouldn't it be better to add the Stack Protector library 
(MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf) instead of 
disabling it?


--
Rebecca Cran


On 1/4/2024 11:49 AM, Prabin CA via groups.io wrote:

Add the no-stack-protector compiler flag to allow StandaloneMM builds
on both AArch64 and x86 host. Without this flag, the link stage fails
with the following errors on multiple files when built with gcc
(Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0:

undefined reference to `__stack_chk_guard'
undefined reference to `__stack_chk_fail'

Signed-off-by: Vijayenthiran Subramaniam 
Signed-off-by: Prabin CA 
---
  Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc 
b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
index ab54b3b25f4c..2a8c678c0816 100644
--- a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
@@ -145,5 +145,5 @@ [Components.AARCH64]
  #
  
###
  [BuildOptions.AARCH64]
-  GCC:*_*_*_CC_FLAGS = -mstrict-align -march=armv8-a -D 
DISABLE_NEW_DEPRECATED_INTERFACES
+  GCC:*_*_*_CC_FLAGS = -mstrict-align -march=armv8-a -fno-stack-protector -D 
DISABLE_NEW_DEPRECATED_INTERFACES
GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113199): https://edk2.groups.io/g/devel/message/113199
Mute This Topic: https://groups.io/mt/103528421/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/3] ArmPkg: Fix the calculation of the timer period in GenericWatchdogDxe

Fix the calculation of the timer period in GenericWatchdogDxe: we need
to multiply before dividing to keep the values as integers.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 05df101d5f4b..8f02f38c64e3 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -119,7 +119,7 @@ WatchdogInterruptHandler (
   // the timer period plus 1.
   //
   if (mWatchdogNotify != NULL) {
-TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / 
mTimerFrequencyHz);
 mWatchdogNotify (TimerPeriod + 1);
   }
 
@@ -260,7 +260,7 @@ WatchdogGetTimerPeriod (
 return EFI_INVALID_PARAMETER;
   }
 
-  *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+  *TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / 
mTimerFrequencyHz);
 
   return EFI_SUCCESS;
 }
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113105): https://edk2.groups.io/g/devel/message/113105
Mute This Topic: https://groups.io/mt/103510102/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/3] ArmPkg: Disable watchdog interaction after exiting boot services

Update GenericWatchdogDxe to disable watchdog interaction after exiting
boot services. Also, move the mEfiExitBootServicesEvent event to the top
of the file with the other static variables.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 8f02f38c64e3..912106eb6ad2 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -36,10 +36,14 @@ STATIC UINTN  mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mNumTimerTicks = 0;
 
+/* disables watchdog interaction after Exit Boot Services */
+STATIC BOOLEAN  mExitedBootServices = FALSE;
+
 #define MAX_UINT48  0xULL
 
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;
 
 STATIC
 VOID
@@ -91,7 +95,8 @@ WatchdogExitBootServicesEvent (
   )
 {
   WatchdogDisable ();
-  mNumTimerTicks = 0;
+  mNumTimerTicks  = 0;
+  mExitedBootServices = TRUE;
 }
 
 /* This function is called when the watchdog's first signal (WS0) goes high.
@@ -202,8 +207,9 @@ WatchdogSetTimerPeriod (
 {
   UINTN  SystemCount;
 
-  // if TimerPeriod is 0, this is a request to stop the watchdog.
-  if (TimerPeriod == 0) {
+  // if TimerPeriod is 0 or we've exited boot services,
+  // this is a request to stop the watchdog.
+  if (TimerPeriod == 0 || mExitedBootServices) {
 mNumTimerTicks = 0;
 WatchdogDisable ();
 return EFI_SUCCESS;
@@ -303,8 +309,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL  mWatchdogTimer = {
   WatchdogGetTimerPeriod
 };
 
-STATIC EFI_EVENT  mEfiExitBootServicesEvent;
-
 EFI_STATUS
 EFIAPI
 GenericWatchdogEntry (
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113107): https://edk2.groups.io/g/devel/message/113107
Mute This Topic: https://groups.io/mt/103510105/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset

The generic watchdog offset register is 48 bits wide, and can be set by
performing two 32-bit writes.

Add support for writing the high 16 bits of the offset register and
update the signature of the WatchdogWriteOffsetRegister function to take
a UINT64 value.

Signed-off-by: Rebecca Cran 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h|  4 +++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +-
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
index 9bc3bf47047c..504bc4cacb33 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
@@ -1,5 +1,6 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -14,7 +15,8 @@
 
 // Control Frame:
 #define GENERIC_WDOG_CONTROL_STATUS_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x000)
-#define GENERIC_WDOG_OFFSET_REG  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_LOW  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
+#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x00C)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x010)
 #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH  ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x014)
 
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 66c6c37c08b0..05df101d5f4b 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -1,5 +1,6 @@
 /** @file
 *
+*  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
 *  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -35,16 +36,19 @@ STATIC UINTN  mTimerFrequencyHz = 0;
It is therefore stored here. 0 means the timer is not running. */
 STATIC UINT64  mNumTimerTicks = 0;
 
+#define MAX_UINT48  0xULL
+
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL  *mInterruptProtocol;
 STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify;
 
 STATIC
 VOID
 WatchdogWriteOffsetRegister (
-  UINT32  Value
+  UINT64  Value
   )
 {
-  MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32);
+  MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT32);
 }
 
 STATIC
@@ -211,17 +215,17 @@ WatchdogSetTimerPeriod (
   /* If the number of required ticks is greater than the max the watchdog's
  offset register (WOR) can hold, we need to manually compute and set
  the compare register (WCV) */
-  if (mNumTimerTicks > MAX_UINT32) {
+  if (mNumTimerTicks > MAX_UINT48) {
 /* We need to enable the watchdog *before* writing to the compare register,
because enabling the watchdog causes an "explicit refresh", which
clobbers the compare register (WCV). In order to make sure this doesn't
trigger an interrupt, set the offset to max. */
-WatchdogWriteOffsetRegister (MAX_UINT32);
+WatchdogWriteOffsetRegister (MAX_UINT48);
 WatchdogEnable ();
 SystemCount = ArmGenericTimerGetSystemCount ();
 WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks);
   } else {
-WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks);
+WatchdogWriteOffsetRegister (mNumTimerTicks);
 WatchdogEnable ();
   }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113106): https://edk2.groups.io/g/devel/message/113106
Mute This Topic: https://groups.io/mt/103510103/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements

Fixes and improvements to GenericWatchdogDxe.

PR: https://github.com/tianocore/edk2/pull/5176

Rebecca Cran (3):
  ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
  ArmPkg: Fix the calculation of the timer period in GenericWatchdogDxe
  ArmPkg: Disable watchdog interaction after exiting boot services

 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h|  4 +++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 32 
+++---
 2 files changed, 23 insertions(+), 13 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113104): https://edk2.groups.io/g/devel/message/113104
Mute This Topic: https://groups.io/mt/103510101/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 0/1] Resolve regex syntax warnings


Queued as https://github.com/tianocore/edk2/pull/5178 .

On 12/6/2023 1:27 PM, Joey Vagedes wrote:

Python 3.12 now produces syntax warnings when using an invalid escape
character (\ followed by an unexpected character). This happens
throughout BaseTools due the usage of regular expressions. the re module
in python suggests that when creating regex patterns, to use raw text.
This patch series adds the r prefix to any regex pattern that uses an
invalid escape sequence.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Joey Vagedes 

Joey Vagedes (1):
   BaseTools: Resolve regex syntax warnings

  BaseTools/Source/Python/AmlToC/AmlToC.py |  2 +-
  BaseTools/Source/Python/AutoGen/BuildEngine.py   |  2 +-
  BaseTools/Source/Python/AutoGen/GenDepex.py  |  2 +-
  BaseTools/Source/Python/AutoGen/GenMake.py   |  2 +-
  BaseTools/Source/Python/AutoGen/IdfClassObject.py|  2 +-
  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py |  4 ++--
  BaseTools/Source/Python/AutoGen/StrGather.py |  2 +-
  BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py  |  2 +-
  BaseTools/Source/Python/Common/Expression.py | 16 
++---
  BaseTools/Source/Python/Common/GlobalData.py |  4 ++--
  BaseTools/Source/Python/Common/Misc.py   | 24 
++--
  BaseTools/Source/Python/Common/ToolDefClassObject.py |  6 ++---
  BaseTools/Source/Python/GenFds/FdfParser.py  | 10 
  BaseTools/Source/Python/GenFds/GenFds.py |  2 +-
  BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py | 12 +-
  BaseTools/Source/Python/Trim/Trim.py | 18 
+++
  BaseTools/Source/Python/Workspace/DscBuildData.py|  8 +++
  BaseTools/Source/Python/Workspace/MetaFileParser.py  |  2 +-
  18 files changed, 60 insertions(+), 60 deletions(-)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112776): https://edk2.groups.io/g/devel/message/112776
Mute This Topic: https://groups.io/mt/103021364/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools: VfrCompiler Adds DefaultValueError Feature

I see two patches with the same subject line sent within about 15 
minutes of each other.
Could you confirm whether I should review this one (1:43am) or the other 
(1:27am) please?


Thanks.
Rebecca Cran

On 12/12/2023 1:43 AM, Yuting Yang wrote:

Add --catch_default option
Raise a DefaultValueError when encountering VFR default definitions
to help remove default variables.
S
Add --except_list option
Exclude packages that don't require enabling the catch_default function.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Christine Chen 
Cc: Yuting Yang 

Signed-off-by: Yuting Yang 
---
  BaseTools/Source/C/VfrCompile/VfrCompiler.cpp |  40 ++-
  BaseTools/Source/C/VfrCompile/VfrCompiler.h   |   3 +
  BaseTools/Source/C/VfrCompile/VfrError.cpp|   3 +-
  BaseTools/Source/C/VfrCompile/VfrError.h  |   3 +-
  BaseTools/Source/C/VfrCompile/VfrFormPkg.h|   1 +
  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 238 ++
  6 files changed, 184 insertions(+), 104 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index 5f4d262d85..e97cebff65 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -78,6 +78,9 @@ CVfrCompiler::OptionInitialization (
mOptions.WarningAsError= FALSE;

mOptions.AutoDefault   = FALSE;

mOptions.CheckDefault  = FALSE;

+  mOptions.IsCatchDefaultEnable  = FALSE;

+  mOptions.ExceptionList = NULL;

+  mOptions.ExceptionListSize = 0;

memset (, 0, sizeof (EFI_GUID));

  


if (Argc == 1) {

@@ -95,6 +98,23 @@ CVfrCompiler::OptionInitialization (
Version ();

SET_RUN_STATUS (STATUS_DEAD);

return;

+} else if (stricmp(Argv[Index], "--catch_default") == 0){

+  mOptions.IsCatchDefaultEnable = TRUE;

+} else if (stricmp(Argv[Index], "--except_list") == 0){

+  INT32 Start = ++Index;

+  if ((Start >= Argc) || (Argv[Start][0] == '-')) {

+DebugError (NULL, 0, 1001, "Missing option", "-exception_list missing 
except list");

+goto Fail;

+  }

+  while ((Argv[Index][0] != '-') && (Index < Argc - 1)) {

+Index++;

+  }

+  INT32 End = Index--;

+  mOptions.ExceptionListSize = End - Start;

+  for (INT32 i = Start; i < End; i++) {

+mOptions.ExceptionList = (CHAR8**) realloc(mOptions.ExceptionList, (i 
- Start + 1) * sizeof(CHAR8*));

+mOptions.ExceptionList[i-Start] = Argv[i];

+  }

  } else if (stricmp(Argv[Index], "-l") == 0) {

mOptions.CreateRecordListFile = TRUE;

gCIfrRecordInfoDB.TurnOn ();

@@ -179,7 +199,11 @@ CVfrCompiler::OptionInitialization (
goto Fail;

  }

  strcpy (mOptions.VfrFileName, Argv[Index]);

-

+for (int i = 0; i < mOptions.ExceptionListSize; i++) {

+  if (strstr(mOptions.VfrFileName, mOptions.ExceptionList[i]) != NULL) {

+mOptions.IsCatchDefaultEnable = FALSE;

+  }

+}

  if (mOptions.OutputDirectory == NULL) {

mOptions.OutputDirectory = (CHAR8 *) malloc (1);

if (mOptions.OutputDirectory == NULL) {

@@ -217,6 +241,11 @@ Fail:
  free (mOptions.VfrFileName);

  mOptions.VfrFileName = NULL;

}

+  if (mOptions.ExceptionList != NULL) {

+free(mOptions.ExceptionList);

+mOptions.ExceptionList   = NULL;

+

+  }

if (mOptions.VfrBaseFileName != NULL) {

  free (mOptions.VfrBaseFileName);

  mOptions.VfrBaseFileName = NULL;

@@ -496,6 +525,11 @@ CVfrCompiler::~CVfrCompiler (
  mOptions.VfrBaseFileName = NULL;

}

  


+  if (mOptions.ExceptionList != NULL) {

+free (mOptions.ExceptionList);

+mOptions.ExceptionList = NULL;

+  }

+

if (mOptions.OutputDirectory != NULL) {

  free (mOptions.OutputDirectory);

  mOptions.OutputDirectory = NULL;

@@ -679,7 +713,7 @@ CVfrCompiler::Compile (
  DebugError (NULL, 0, 0001, "Error opening the input file", "%s", 
InFileName);

  goto Fail;

}

-

+  InputInfo.IsCatchDefaultEnable = mOptions.IsCatchDefaultEnable;

if (mOptions.HasOverrideClassGuid) {

  InputInfo.OverrideClassGuid = 

} else {

@@ -937,5 +971,3 @@ main (
  


return GetUtilityStatus ();

  }

-

-

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.h 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
index b6e207d2ce..39e0a89a29 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
@@ -52,6 +52,9 @@ typedef struct {
BOOLEAN WarningAsError;

BOOLEAN AutoDefault;

BOOLEAN CheckDefault;

+  BOOLEAN IsCatchDefaultEnable;

+  CHAR8**  ExceptionList;

+  INT16ExceptionListSize;

  } OPTIONS;

  


  typedef enum {

diff --git a/BaseTools/Sourc

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Resolve regex syntax warnings


Thanks.

Reviewed-by: Rebecca Cran 

--
Rebecca Cran

On 12/6/2023 1:27 PM, Joey Vagedes wrote:

Switches regex patterns to raw text to resolve python 3.12 syntax
warnings in regards to invalid escape sequences, as is suggested by the
re (regex) module in python.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Signed-off-by: Joey Vagedes 
---
  BaseTools/Source/Python/AmlToC/AmlToC.py |  2 +-
  BaseTools/Source/Python/AutoGen/BuildEngine.py   |  2 +-
  BaseTools/Source/Python/AutoGen/GenDepex.py  |  2 +-
  BaseTools/Source/Python/AutoGen/GenMake.py   |  2 +-
  BaseTools/Source/Python/AutoGen/IdfClassObject.py|  2 +-
  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py |  4 ++--
  BaseTools/Source/Python/AutoGen/StrGather.py |  2 +-
  BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py  |  2 +-
  BaseTools/Source/Python/Common/Expression.py | 16 
++---
  BaseTools/Source/Python/Common/GlobalData.py |  4 ++--
  BaseTools/Source/Python/Common/Misc.py   | 24 
++--
  BaseTools/Source/Python/Common/ToolDefClassObject.py |  6 ++---
  BaseTools/Source/Python/GenFds/FdfParser.py  | 10 
  BaseTools/Source/Python/GenFds/GenFds.py |  2 +-
  BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py | 12 +-
  BaseTools/Source/Python/Trim/Trim.py | 18 
+++
  BaseTools/Source/Python/Workspace/DscBuildData.py|  8 +++
  BaseTools/Source/Python/Workspace/MetaFileParser.py  |  2 +-
  18 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/BaseTools/Source/Python/AmlToC/AmlToC.py 
b/BaseTools/Source/Python/AmlToC/AmlToC.py
index 346de7159de7..63931c9720c9 100644
--- a/BaseTools/Source/Python/AmlToC/AmlToC.py
+++ b/BaseTools/Source/Python/AmlToC/AmlToC.py
@@ -17,7 +17,7 @@ from Common.BuildToolError import *
  import sys

  import os

  


-__description__ = """

+__description__ = r"""

  Convert an AML file to a .c file containing the AML bytecode stored in a C

  array. By default, Tables\Dsdt.aml will generate Tables\Dsdt.c.

  Tables\Dsdt.c will contain a C array named "dsdt_aml_code" that contains

diff --git a/BaseTools/Source/Python/AutoGen/BuildEngine.py 
b/BaseTools/Source/Python/AutoGen/BuildEngine.py
index 752a1a1f6a86..45b39d7878d5 100644
--- a/BaseTools/Source/Python/AutoGen/BuildEngine.py
+++ b/BaseTools/Source/Python/AutoGen/BuildEngine.py
@@ -306,7 +306,7 @@ class BuildRule:
  _SubSectionList = [_InputFile, _OutputFile, _Command]

  


  _PATH_SEP = "(+)"

-_FileTypePattern = re.compile("^[_a-zA-Z][_\-0-9a-zA-Z]*$")

+_FileTypePattern = re.compile(r"^[_a-zA-Z][_\-0-9a-zA-Z]*$")

  _BinaryFileRule = FileBuildRule(TAB_DEFAULT_BINARY_FILE, [], 
[os.path.join("$(OUTPUT_DIR)", "${s_name}")],

  ["$(CP) ${src} ${dst}"], [])

  


diff --git a/BaseTools/Source/Python/AutoGen/GenDepex.py 
b/BaseTools/Source/Python/AutoGen/GenDepex.py
index f2f2e9d65b5f..b6db6645a4fb 100644
--- a/BaseTools/Source/Python/AutoGen/GenDepex.py
+++ b/BaseTools/Source/Python/AutoGen/GenDepex.py
@@ -126,7 +126,7 @@ class DependencyExpression:
  #

  # open and close brace must be taken as individual tokens

  #

-TokenPattern = re.compile("(\(|\)|\{[^{}]+\{?[^{}]+\}?[ ]*\}|\w+)")

+TokenPattern = re.compile(r"(\(|\)|\{[^{}]+\{?[^{}]+\}?[ ]*\}|\w+)")

  


  ## Constructor

  #

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index daec9c6d54b2..c416fe172fe5 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -28,7 +28,7 @@ from Common.DataType import TAB_COMPILER_MSFT
  gIncludePattern = re.compile(r"^[ \t]*[#%]?[ \t]*include(?:[ \t]*(?:\\(?:\r\n|\r|\n))*[ 
\t]*)*(?:\(?[\"<]?[ \t]*)([-\w.\\/() \t]+)(?:[ \t]*[\">]?\)?)", re.MULTILINE | 
re.UNICODE | re.IGNORECASE)

  


  ## Regular expression for matching macro used in header file inclusion

-gMacroPattern = re.compile("([_A-Z][_A-Z0-9]*)[ \t]*\((.+)\)", re.UNICODE)

+gMacroPattern = re.compile(r"([_A-Z][_A-Z0-9]*)[ \t]*\((.+)\)", re.UNICODE)

  


  gIsFileMap = {}

  


diff --git a/BaseTools/Source/Python/AutoGen/IdfClassObject.py 
b/BaseTools/Source/Python/AutoGen/IdfClassObject.py
index a6b8123c2539..bb413c6a26e3 100644
--- a/BaseTools/Source/Python/AutoGen/IdfClassObject.py
+++ b/BaseTools/Source/Python/AutoGen/IdfClassObject.py
@@ -18,7 +18,7 @@ import os
  from Common.GlobalData import gIdentifierPattern

  from .UniClassObject import StripComments

  


-IMAGE_TOKEN = re.compile('IMAGE_TOKEN *\(([A-Z

Re: [edk2-devel] [PATCH 1/1] BaseTools/Scripts/PatchCheck.py: Check for Change-id


Thanks for the reminder - I'm catching up on patches today.
One nit that would be good to fix before being merged: the help for 
"--ignore-change-id" should use the same capitalization of 'Change-Id' 
as the rest of the code (i.e. 'Change-Id' not 'Change-id').


With that fixed:

Reviewed-by: Rebecca Cran 

--
Rebecca Cran


On 12/12/2023 2:38 AM, Pierre Gondois wrote:

Hello BaseTools maintainers,
Just a ping in case you have any comment for this patch,

Regards,
Pierre

On 11/29/23 10:37, Sami Mujawar wrote:

Hi Ray,

On 29/11/2023, 00:56, "Ni, Ray" <mailto:ray...@intel.com>> wrote:



It's good. But I am curious why --ignore-change-id is needed?
[SAMI] This option can be useful if an internal CI uses the same 
script for checking patches before they are posted on the list.


Regards,

Sami Mujawar

Thanks,
Ray

-Original Message-
From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
mailto:devel@edk2.groups.io>> On Behalf Of Yuwei

Chen
Sent: Wednesday, November 29, 2023 8:23 AM
To: Pierre Gondois <mailto:pierre.gond...@arm.com>>; devel@edk2.groups.io 
<mailto:devel@edk2.groups.io>
Cc: Rebecca Cran mailto:rebe...@bsdio.com>>; 
Gao, Liming
mailto:gaolim...@byosoft.com.cn>>; Feng, 
Bob C mailto:bob.c.f...@intel.com>>; Sami
Mujawar mailto:sami.muja...@arm.com>>; 
yeoreum@arm.com <mailto:yeoreum@arm.com>

Subject: Re: [edk2-devel] [PATCH 1/1] BaseTools/Scripts/PatchCheck.py:
Check for Change-id

The patch is good for me.

Reviewed-by: Yuwei Chen <mailto:yuwei.c...@intel.com>>



-Original Message-
From: Pierre Gondois <mailto:pierre.gond...@arm.com>>

Sent: Wednesday, November 22, 2023 9:15 PM
To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
Cc: Rebecca Cran mailto:rebe...@bsdio.com>>; 
Gao, Liming
mailto:gaolim...@byosoft.com.cn>>; Feng, 
Bob C mailto:bob.c.f...@intel.com>>; Chen,
Christine mailto:yuwei.c...@intel.com>>; 
Sami Mujawar
mailto:sami.muja...@arm.com>>; 
yeoreum@arm.com <mailto:yeoreum@arm.com>
Subject: [PATCH 1/1] BaseTools/Scripts/PatchCheck.py: Check for 
Change-id


Code review tools like gerrit might use a 'Change-id' tag to track 
the evolution
of patches. This tag should be removed before submitting a patch to 
the

mailing-list.
It has been observed that contributors sometimes forget to remove 
this tag.

Add a check in PatchCheck.py to automate this.

Also add a '--ignore-change-id' command line parameter to ignore 
the above

check.

Signed-off-by: Pierre Gondois <mailto:pierre.gond...@arm.com>>

---
BaseTools/Scripts/PatchCheck.py | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py
b/BaseTools/Scripts/PatchCheck.py index 7f372d40b570..7770d1e37318
100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -3,7 +3,7 @@
# # Copyright (c) 2015 - 2021, Intel Corporation. All rights 
reserved. #
Copyright (C) 2020, Red Hat, Inc.-# Copyright (c) 2020, ARM 
Ltd. All

rights

reserved.+# Copyright (c) 2020 - 2023, Arm Limited. All rights
reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ 
-26,6

+26,9 @@ class Verbose:
SILENT, ONELINE, NORMAL = range(3) level = NORMAL +class
PatchCheckConf:+ ignore_change_id = False+ class EmailAddressCheck:
"""Checks an email address.""" @@ -111,6 +114,8 @@ class
CommitMessageCheck:
self.check_signed_off_by() self.check_misc_signatures()
self.check_overall_format()+ if not PatchCheckConf.ignore_change_id:+
self.check_change_id_format() self.report_message_result() url =
'https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message- 
<https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message->

Format'@@ -307,6 +312,12 @@ class CommitMessageCheck:
break last_sig_line = line.strip() + def
check_change_id_format(self):+ cid='Change-Id:'+ if
self.msg.find(cid) != -1:+ self.error('\"%s\" found in commit

message:' %

cid)+ return+ (START, PRE_PATCH, PATCH) = range(3) class
GitDiffCheck:@@ -780,11 +791,16 @@ class PatchCheckApp:
group.add_argument("--silent", action="store_true",
help="Print nothing")+ group.add_argument("--ignore-change-id",+
action="store_true",+ help="Ignore the presence of 'Change-
id:' tags in commit message") self.args = parser.parse_args() if
self.args.oneline: Verbose.level = Verbose.ONELINE if
self.args.silent: Verbose.level = Verbose.SILENT+ if
self.args.ignore_change_id:+ PatchCheckConf.ignore_change_id = True
if __name__ == "__main__": sys.exit(PatchCheckApp().retval)--
2.25.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112773): https://edk2.groups.io/g/devel/message/112773
Mute This Topic: https://groups.io/mt/102748141/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: 回复: [edk2-devel] [PATCH] BaseTools: FMMT GuidTool Auto Select Config file Enabling


Queued as https://github.com/tianocore/edk2/pull/5177 .

On 12/19/2023 6:02 AM, gaoliming wrote:

The change is good.

Reviewed-by: Liming Gao 


-邮件原件-
发件人: devel@edk2.groups.io  代表 Guo, Gua
发送时间: 2023年12月19日 15:32
收件人: devel@edk2.groups.io; Chen, Christine ;
Rebecca Cran ; Gao, Liming

抄送: Feng, Bob C 
主题: Re: [edk2-devel] [PATCH] BaseTools: FMMT GuidTool Auto Select Config
file Enabling

Hi @Gao, Liming and @Rebecca Cran

I may need to get your help for review it, any concern you can also share

for

us.

Thanks,
Gua

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Yuwei
Chen
Sent: Friday, December 15, 2023 5:12 PM
To: devel@edk2.groups.io
Cc: Rebecca Cran ; Gao, Liming
; Feng, Bob C 
Subject: [edk2-devel] [PATCH] BaseTools: FMMT GuidTool Auto Select Config
file Enabling

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

Currently, Python FMMT tool does not support automatically select
FMMTConf.ini file which saves GuidTool settings.
This patch supports this features.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 

Signed-off-by: Yuwei Chen 
---
  BaseTools/Source/Python/FMMT/core/GuidTools.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/FMMT/core/GuidTools.py
b/BaseTools/Source/Python/FMMT/core/GuidTools.py
index a25681709b..f6bdeffa50 100644
--- a/BaseTools/Source/Python/FMMT/core/GuidTools.py
+++ b/BaseTools/Source/Python/FMMT/core/GuidTools.py
@@ -110,7 +110,7 @@ class GUIDTools:
  if os.environ['FmmtConfPath']: self.tooldef_file =
os.path.join(os.environ['FmmtConfPath'], 'FmmtConf.ini') else:-
PathList = os.environ['PATH']+PathList =
os.environ['PATH'].split(os.pathsep) for CurrentPath in

PathList:

if os.path.exists(os.path.join(CurrentPath, 'FmmtConf.ini')):
self.tooldef_file = os.path.join(CurrentPath, 'FmmtConf.ini')--
2.42.0.windows.2





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112772): https://edk2.groups.io/g/devel/message/112772
Mute This Topic: https://groups.io/mt/103261683/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools: FMMT GuidTool Auto Select Config file Enabling


Reviewed-by: Rebecca Cran 

On 12/15/2023 2:11 AM, Yuwei Chen wrote:

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

Currently, Python FMMT tool does not support automatically
select FMMTConf.ini file which saves GuidTool settings.
This patch supports this features.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 

Signed-off-by: Yuwei Chen 
---
  BaseTools/Source/Python/FMMT/core/GuidTools.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/FMMT/core/GuidTools.py 
b/BaseTools/Source/Python/FMMT/core/GuidTools.py
index a25681709b..f6bdeffa50 100644
--- a/BaseTools/Source/Python/FMMT/core/GuidTools.py
+++ b/BaseTools/Source/Python/FMMT/core/GuidTools.py
@@ -110,7 +110,7 @@ class GUIDTools:
  if os.environ['FmmtConfPath']:

  self.tooldef_file = os.path.join(os.environ['FmmtConfPath'], 
'FmmtConf.ini')

  else:

-PathList = os.environ['PATH']

+PathList = os.environ['PATH'].split(os.pathsep)

  for CurrentPath in PathList:

  if os.path.exists(os.path.join(CurrentPath, 'FmmtConf.ini')):

  self.tooldef_file = os.path.join(CurrentPath, 
'FmmtConf.ini')





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112771): https://edk2.groups.io/g/devel/message/112771
Mute This Topic: https://groups.io/mt/103187642/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] BaseTools/GenFw: Correct offset when relocating an ADR


Reviewed-by: Rebecca Cran 


On 12/20/2023 12:31 PM, Jake Garver wrote:

In the R_AARCH64_ADR_GOT_PAGE case on AARCH64, we may encounter an ADR
instead of an ADRP when the toolchain is working around Cortex-A53
erratum #843419.  If that's the case, be sure to calculate the offset
appropriately.

This resolves an issue experienced when building a StandaloneMm image
with stack protection enabled on GCC compiled with
"--enable-fix-cortex-a53-843419".  This scenario sometimes generates an
ADR with a R_AARCH64_ADR_GOT_PAGE relocation.

In this scenario, the following code is being generated by the
toolchain:

 # Load to set the stack canary
 2ffc:  10028020adr x0, 8000 
 3008:  f940d400ldr x0, [x0, #424]

 # Load to check the stack canary
 30cc:  b020adrpx0, 8000 
 30d0:  f940d400ldr x0, [x0, #424]

GenFw rewrote that to:

 # Load to set the stack canary
 2ffc:  1480adr x0, 0x308c
 3008:  912ec000add x0, x0, #0xbb0

 # Load to check the stack canary
 30cc:  f460adrpx0, 0x92000
 30d0:  912ec000add x0, x0, #0xbb0

Note that we're now setting the stack canary from the wrong address,
resulting in an erroneous stack fault.

After this fix, the offset will be calculated correctly for an ADR and
the stack canary is set correctly.

Signed-off-by: Jake Garver 
---

Notes:
   v2: Implement approach proposed by Ard Biesheuvel.
 - title changed to: Correct offset when relocating an ADR
   v1: Original title: Change opcode when converting ADR to ADRP

  BaseTools/Source/C/GenFw/Elf64Convert.c | 22 +-
  1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 9911db65af..9d04fc612e 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1562,7 +1562,27 @@ WriteSections64 (
  // subsequent LDR instruction (covered by a 
R_AARCH64_LD64_GOT_LO12_NC
  // relocation) into an ADD instruction - this is handled above.
  //
-Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+// In order to handle Cortex-A53 erratum #843419, the GCC toolchain
+// may convert an ADRP instruction at the end of a page (0xffc
+// offset) into an ADR instruction. If so, be sure to calculate the
+// offset for an ADR instead of ADRP.
+//
+if ((*(UINT32 *)Targ & BIT31) == 0) {
+  //
+  // Calculate the offset for an ADR.
+  //
+  Offset = (Sym->st_value & ~0xfff) - Rel->r_offset;
+  if (Offset < -0x10 || Offset > 0xf) {
+Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s  due to its 
size (> 1 MB), unable to relocate ADR.",
+  mInImageName);
+break;
+  }
+} else {
+  //
+  // Calculate the offset for an ADRP.
+  //
+  Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+}
  
  *(UINT32 *)Targ &= 0x901f;

  *(UINT32 *)Targ |= ((Offset & 0x1c) << (5 - 2)) | ((Offset & 0x3) 
<< 29);





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112770): https://edk2.groups.io/g/devel/message/112770
Mute This Topic: https://groups.io/mt/103287393/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] Copyright for an individual contributor

2023-12-17 Thread Rebecca Cran


On 12/17/23 20:01, Chang, Abner via groups.io wrote:


[AMD Official Use Only - General]


Hi all,

How do we have the copyright for the contribution from an individual? 
I don’t see the formatting defined in edk2 C coding standard. Is the 
below format ok?


Copyright (c) 2023, Contributor 



I think to follow the style guide, it should have a "" at the end, 
though most files with copyrights from individuals don't have that.



--

Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112635): https://edk2.groups.io/g/devel/message/112635
Mute This Topic: https://groups.io/mt/103236466/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-test v2 0/4] Various improvements to the repo

2023-12-15 Thread Rebecca Cran


Could someone push these changes to the edk2-test repo please?

On 11/10/2023 12:08 PM, Rebecca Cran wrote:

Some improvements to the edk2-test repository.

Since buildzip.sh creates output that appears more useful for running on
the target machine, I wonder if we might want to mention it in the
documentation someplace?

Also, I suspect the HowToBuildSctInUdk2017.txt file is now pretty
outdated. Should we delete it?

Finally, is the TianoCore Contribution Agreement still relevant? I know
it is for the edk2 docs repositories, but not sure if edk2-test should
be updated to match the other code repos like edk2 and edk2-platforms?

CHANGES FROM v1 to v2:

Converted tabs to spaces in the echo line added to buildzip.sh

Rebecca Cran (4):
   Unbreak buildzip.sh
   Rename files in HowToBuild to avoid spaces in filenames
   Point users to the URL for edk2-test-parser if it doesn't exist
   Fix the URL for the edk2-test repo

  Maintainers.txt   
   | 2 +-
  uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} | 0
  uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt}   
   | 0
  uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt}  | 0
  uefi-sct/SctPkg/buildzip.sh   
   | 4 +---
  5 files changed, 2 insertions(+), 4 deletions(-)
  rename uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} (100%)
  rename uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt} (100%)
  rename uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt} (100%)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112607): https://edk2.groups.io/g/devel/message/112607
Mute This Topic: https://groups.io/mt/102513312/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/1] MdePkg: Add DDR5 SPD defs to IndustryStandard per JESD400-5A.01

2023-12-15 Thread Rebecca Cran


On 4/7/2023 10:58 AM, Rebecca Cran wrote:
Could I get some reviews on this please? It was sent out back in 
February.


I'm not sure how we should solve the CI breakage, since as I noted in 
the v1 revision it contains fields that begin with a lower-case letter 
following the style of the DDR4 header 
(https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/SdramSpdDdr4.h).


This never did get reviewed, but it can be discarded since I no longer 
care about getting it committed.


Somebody else can figure it out when they need the DDR5 SPD definitions.


--

Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112606): https://edk2.groups.io/g/devel/message/112606
Mute This Topic: https://groups.io/mt/96962257/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] Add DEBUG_MANAGEABILITY to debug level comments

2023-12-11 Thread Rebecca Cran


On 12/11/2023 6:17 PM, Laszlo Ersek wrote:

On 12/9/23 01:33, Rebecca Cran via groups.io wrote:

On 12/8/2023 5:19 PM, Rebecca Cran wrote:

Add the new DEBUG_MANAGEABILITY debug level to MdePkg.dec and MdePkg.uni.

Improve the wording of the description of the DEBUG_MANAGEABILITY
level in
DebugLib.h.

Update the comment block in ArmVirtPkg.dsc.inc with the new list and
updated
formatting.


PR: https://github.com/tianocore/edk2/pull/5127


... commenting only because I was CC'd:

Please confirm on the list whenever a series is merged; it saves time
for reviewers.

(This problem should disappear once we move reviews to github.com, but
until then, a short confirmation on-list can prevent wasted time.)

For the record: commit range
5b5481526fc9b89e5f3843d9bb3d6c4f5ce41060..aa2f32cefa567133d94d574672a4479e004211ee.


Oh, thanks! I was meaning to do that but forgot between setting the 
'push' label and waiting for CI to complete.



--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112352): https://edk2.groups.io/g/devel/message/112352
Mute This Topic: https://groups.io/mt/103066384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-test v2 0/4] Various improvements to the repo

2023-12-11 Thread Rebecca Cran


I don't appear to have permission to push to the edk2-test repo.
Could somebody merge my change please?

Thanks.
Rebecca

On 11/10/2023 12:08 PM, Rebecca Cran wrote:

Some improvements to the edk2-test repository.

Since buildzip.sh creates output that appears more useful for running on
the target machine, I wonder if we might want to mention it in the
documentation someplace?

Also, I suspect the HowToBuildSctInUdk2017.txt file is now pretty
outdated. Should we delete it?

Finally, is the TianoCore Contribution Agreement still relevant? I know
it is for the edk2 docs repositories, but not sure if edk2-test should
be updated to match the other code repos like edk2 and edk2-platforms?

CHANGES FROM v1 to v2:

Converted tabs to spaces in the echo line added to buildzip.sh

Rebecca Cran (4):
   Unbreak buildzip.sh
   Rename files in HowToBuild to avoid spaces in filenames
   Point users to the URL for edk2-test-parser if it doesn't exist
   Fix the URL for the edk2-test repo

  Maintainers.txt   
   | 2 +-
  uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} | 0
  uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt}   
   | 0
  uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt}  | 0
  uefi-sct/SctPkg/buildzip.sh   
   | 4 +---
  5 files changed, 2 insertions(+), 4 deletions(-)
  rename uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} (100%)
  rename uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt} (100%)
  rename uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt} (100%)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112322): https://edk2.groups.io/g/devel/message/112322
Mute This Topic: https://groups.io/mt/102513312/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-test v2 2/4] Rename files in HowToBuild to avoid spaces in filenames

2023-12-11 Thread Rebecca Cran


On 11/29/2023 1:06 AM, G Edhaya Chandran wrote:

Hi Rebecca,
Thank you for these updates.
Just a matter of style, but can we do HowToBuild*SCT*.txt instead of 
HowToBuild*Sct*.txt? 


Sure! I can make that change just before I push the series.


--

Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112315): https://edk2.groups.io/g/devel/message/112315
Mute This Topic: https://groups.io/mt/102513314/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] Add DEBUG_MANAGEABILITY to debug level comments


On 12/8/2023 5:19 PM, Rebecca Cran wrote:

Add the new DEBUG_MANAGEABILITY debug level to MdePkg.dec and MdePkg.uni.

Improve the wording of the description of the DEBUG_MANAGEABILITY level in
DebugLib.h.

Update the comment block in ArmVirtPkg.dsc.inc with the new list and updated
formatting.


PR: https://github.com/tianocore/edk2/pull/5127

--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112246): https://edk2.groups.io/g/devel/message/112246
Mute This Topic: https://groups.io/mt/103066384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 4/4] ArmVirtPkg: Sync debug level comments in ArmVirt.dsc.inc

Update the debug level comments in ArmVirt.dsc.inc to sync with
MdePkg/Include/Library/DebugLib.h.

Signed-off-by: Rebecca Cran 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 42 ++--
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 6bc72f1decbd..9b23ef97ec5f 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -315,26 +315,28 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
 !endif
 
-  #  DEBUG_INIT  0x0001  // Initialization
-  #  DEBUG_WARN  0x0002  // Warnings
-  #  DEBUG_LOAD  0x0004  // Load events
-  #  DEBUG_FS0x0008  // EFI File system
-  #  DEBUG_POOL  0x0010  // Alloc & Free (pool)
-  #  DEBUG_PAGE  0x0020  // Alloc & Free (page)
-  #  DEBUG_INFO  0x0040  // Informational debug messages
-  #  DEBUG_DISPATCH  0x0080  // PEI/DXE/SMM Dispatchers
-  #  DEBUG_VARIABLE  0x0100  // Variable
-  #  DEBUG_BM0x0400  // Boot Manager
-  #  DEBUG_BLKIO 0x1000  // BlkIo Driver
-  #  DEBUG_NET   0x4000  // SNP Driver
-  #  DEBUG_UNDI  0x0001  // UNDI Driver
-  #  DEBUG_LOADFILE  0x0002  // LoadFile
-  #  DEBUG_EVENT 0x0008  // Event messages
-  #  DEBUG_GCD   0x0010  // Global Coherency Database changes
-  #  DEBUG_CACHE 0x0020  // Memory range cachability changes
-  #  DEBUG_VERBOSE   0x0040  // Detailed debug messages that may
-  #  // significantly impact boot performance
-  #  DEBUG_ERROR 0x8000  // Error
+  #  DEBUG_INIT   0x0001  // Initialization
+  #  DEBUG_WARN   0x0002  // Warnings
+  #  DEBUG_LOAD   0x0004  // Load events
+  #  DEBUG_FS 0x0008  // EFI File system
+  #  DEBUG_POOL   0x0010  // Alloc & Free (pool)
+  #  DEBUG_PAGE   0x0020  // Alloc & Free (page)
+  #  DEBUG_INFO   0x0040  // Informational debug messages
+  #  DEBUG_DISPATCH   0x0080  // PEI/DXE/SMM Dispatchers
+  #  DEBUG_VARIABLE   0x0100  // Variable
+  #  DEBUG_BM 0x0400  // Boot Manager
+  #  DEBUG_BLKIO  0x1000  // BlkIo Driver
+  #  DEBUG_NET0x4000  // Network Io Driver
+  #  DEBUG_UNDI   0x0001  // UNDI Driver
+  #  DEBUG_LOADFILE   0x0002  // LoadFile
+  #  DEBUG_EVENT  0x0008  // Event messages
+  #  DEBUG_GCD0x0010  // Global Coherency Database changes
+  #  DEBUG_CACHE  0x0020  // Memory range cachability changes
+  #  DEBUG_VERBOSE0x0040  // Detailed debug messages that may
+  #   // significantly impact boot performance
+  #  DEBUG_MANAGEABILITY  0x0080  // Detailed debug and payload 
manageability messages
+  #   // related to modules such as Redfish, 
IPMI, MCTP etc.
+  #  DEBUG_ERROR  0x8000  // Error
 !if $(TARGET) != RELEASE
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|$(DEBUG_PRINT_ERROR_LEVEL)
 !endif
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112245): https://edk2.groups.io/g/devel/message/112245
Mute This Topic: https://groups.io/mt/103066386/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/4] MdePkg: Add manageability debug level to PcdFixedDebugPrintErrorLevel

Update MdePkg.dec to add the manageability debug level to
PcdFixedDebugPrintErrorLevel.

Signed-off-by: Rebecca Cran 
---
 MdePkg/MdePkg.dec | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index ac54338089e8..bec63f5416e2 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2213,6 +2213,7 @@ [PcdsFixedAtBuild]
   #  BIT20 - Global Coherency Database changes message.
   #  BIT21 - Memory range cachability changes message.
   #  BIT22 - Detailed debug message.
+  #  BIT23 - Manageability debug message.
   #  BIT31 - Error message.
   # @Prompt Fixed Debug Message Print Level.
   
gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel|0x|UINT32|0x30001016
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112242): https://edk2.groups.io/g/devel/message/112242
Mute This Topic: https://groups.io/mt/103066383/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 0/4] Add DEBUG_MANAGEABILITY to debug level comments

Add the new DEBUG_MANAGEABILITY debug level to MdePkg.dec and MdePkg.uni.

Improve the wording of the description of the DEBUG_MANAGEABILITY level in
DebugLib.h.

Update the comment block in ArmVirtPkg.dsc.inc with the new list and updated
formatting.


Rebecca Cran (4):
  MdePkg: Improve wording of manageability debug level comment
  MdePkg: Add manageability debug level to PcdFixedDebugPrintErrorLevel
  MdePkg: Update MdePkg.uni with manageability debug level
  ArmVirtPkg: Sync debug level comments in ArmVirt.dsc.inc

 MdePkg/MdePkg.dec |  1 +
 ArmVirtPkg/ArmVirt.dsc.inc| 42 ++--
 MdePkg/Include/Library/DebugLib.h |  4 +-
 MdePkg/MdePkg.uni |  2 +
 4 files changed, 27 insertions(+), 22 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112243): https://edk2.groups.io/g/devel/message/112243
Mute This Topic: https://groups.io/mt/103066384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/4] MdePkg: Update MdePkg.uni with manageability debug level

Update MdePkg.uni with the manageability debug level.

Signed-off-by: Rebecca Cran 
---
 MdePkg/MdePkg.uni | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
index 5c1fa24065c7..75dbc6ffe3c0 100644
--- a/MdePkg/MdePkg.uni
+++ b/MdePkg/MdePkg.uni
@@ -214,6 +214,7 @@

"BIT20 - Global Coherency Database changes message.\n"

"BIT21 - Memory range cacheability changes message.\n"

"BIT22 - Detailed debug message.\n"
+   
"BIT23 - Manageability debug message.\n"

"BIT31 - Error message."
 
 #string STR_gEfiMdePkgTokenSpaceGuid_PcdFixedDebugPrintErrorLevel_PROMPT  
#language en-US "Fixed Debug Message Print Level"
@@ -237,6 +238,7 @@

"BIT20 - Global Coherency Database changes message.\n"

"BIT21 - Memory range cacheability changes message.\n"

"BIT22 - Detailed debug message.\n"
+   
"BIT23 - Manageability debug message.\n"

"BIT31 - Error message."
 
 #string STR_gEfiMdePkgTokenSpaceGuid_PcdReportStatusCodePropertyMask_PROMPT  
#language en-US "Report Status Code Property"
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112244): https://edk2.groups.io/g/devel/message/112244
Mute This Topic: https://groups.io/mt/103066385/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/4] MdePkg: Improve wording of manageability debug level comment

Improve the wording of the comment explaining the DEBUG_MANAGEABILITY
debug level.

Signed-off-by: Rebecca Cran 
---
 MdePkg/Include/Library/DebugLib.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/DebugLib.h 
b/MdePkg/Include/Library/DebugLib.h
index f0c9f6448794..40772f2e0f7b 100644
--- a/MdePkg/Include/Library/DebugLib.h
+++ b/MdePkg/Include/Library/DebugLib.h
@@ -48,8 +48,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define DEBUG_CACHE 0x0020   // Memory range cachability changes
 #define DEBUG_VERBOSE   0x0040   // Detailed debug messages that may
  // significantly impact boot 
performance
-#define DEBUG_MANAGEABILITY  0x0080  // Detailed debug and payload message 
of manageability
- // related modules, such Redfish, 
IPMI, MCTP and etc.
+#define DEBUG_MANAGEABILITY  0x0080  // Detailed debug and payload 
manageability messages
+ // related to modules such as 
Redfish, IPMI, MCTP etc.
 #define DEBUG_ERROR  0x8000  // Error
 
 //
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112241): https://edk2.groups.io/g/devel/message/112241
Mute This Topic: https://groups.io/mt/103066382/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] OvmfPkg/Bhyve: use a proper PCI IO range

2023-11-17 Thread Rebecca Cran


On 11/17/2023 5:43 AM, Corvin Köhne wrote:

Bhyve uses an io port range of [ 0x2000, 0x1 ] [1]. At the moment,
EDKII is using a subset of this range [ 0xC000, 0x1 ] [2]. Even
though the EDKII range doesn't exeed the bhyve range, it's causing
issues on some guests like OpenBSD. We don't know why it's causing
issues yet. However, using the same IO port range in EDKII fixes the
issue and is a good idea anyway.


Reviewed-by: Rebecca Cran 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111408): https://edk2.groups.io/g/devel/message/111408
Mute This Topic: https://groups.io/mt/102646333/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-14 Thread Rebecca Cran via groups.io


On 11/14/2023 7:51 AM, Laszlo Ersek via groups.io wrote:


Funnily enough, my stance is quite the opposite. I happen to disagree
with some patterns that uncrustify enforces, but I'm thankful that at
any given state of CI (= using any given version of uncrustify), we
can't have any more debates about patch formatting (that is, it's
especially its central nature that I like). I've found uncrustify
relatively easy to use locally, too.


There's _one_ place we can still have a debate, but I'm hoping we can 
easily agree, and update CI to enforce it.


I'd like to scrub the tree of all the NT-style function documentation 
blocks and replace them with Doxygen style.


As an example of the NT style, see OvmfPkg/QemuVideoDxe/Gop.c:

EFI_STATUS
EFIAPI
QemuVideoGraphicsOutputQueryMode (
  IN  EFI_GRAPHICS_OUTPUT_PROTOCOL  *This,
  IN  UINT32ModeNumber,
  OUT UINTN *SizeOfInfo,
  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  **Info
  )

/*++

Routine Description:

  Graphics Output protocol interface to query video mode

  Arguments:
This  - Protocol instance pointer.
ModeNumber- The mode number to return information on.
Info  - Caller allocated buffer that returns 
information about ModeNumber.
SizeOfInfo- A pointer to the size, in bytes, of the 
Info buffer.


  Returns:
EFI_SUCCESS   - Mode information returned.
EFI_BUFFER_TOO_SMALL  - The Info buffer was too small.
EFI_DEVICE_ERROR  - A hardware error occurred trying to 
retrieve the video mode.
EFI_NOT_STARTED   - Video display is not initialized. Call 
SetMode ()

EFI_INVALID_PARAMETER - One of the input args was NULL.

--*/
{
  QEMU_VIDEO_PRIVATE_DATA  *Private;
  QEMU_VIDEO_MODE_DATA *ModeData;
...


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111203): https://edk2.groups.io/g/devel/message/111203
Mute This Topic: https://groups.io/mt/102559740/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Rebecca Cran


On 11/13/2023 1:08 PM, Michael Kubacki wrote:
Yes. I just did it. It is relatively minor and impacts expected code 
areas.


https://github.com/tianocore/edk2/pull/5043/files



Could you update .git-blame-ignore-revs please?

https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame


--

Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79): https://edk2.groups.io/g/devel/message/79
Mute This Topic: https://groups.io/mt/102559740/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Rebecca Cran via groups.io


On 11/13/2023 5:29 AM, Marcin Juszkiewicz via groups.io wrote:

Still a fan of adding edk2-uncrustify to BaseTools. If we are expected 
to use it then let it get installed at same moment as "build" command is.


The issue with doing this is there's a push to remove all C/C++ code 
from BaseTools (including porting existing code to Python), and adding 
edk2-uncrustify would work against that.


--
Rebecca Cran




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76): https://edk2.groups.io/g/devel/message/76
Mute This Topic: https://groups.io/mt/102559740/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/5] BaseTools/Scripts/GetMaintainer: Handle reviewer only case


For the series:

Acked-by: Rebecca Cran 


On 11/10/23 12:30, Leif Lindholm wrote:

OK, so this a bit of a backwards review, but I figured I might as
well show how I would prefer the split. I'm adding a patch that
changes internal returns to dictionaries instead of multiple return
values.

There are no functional differences between the original submission
and this for 1-2,4-5/5, so I'm happy to give
Reviewed-by: Leif Lindholm 
for those. 3/5 is new and requires review by someone else.

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

Fix logic bug where maintainers was incorrectly added to lists.

If a package only has reviewers and no maintainers, then also
return the  maintainers.

In order to detect this case, get_maintainers() is updated to
return maintainers, reviews, and lists separately instead of
a single merged list.  This also allows this module to be used
by other scripts that need to distinguish between maintainers,
reviewers, and lists.

Simplify logic that accumulates maintainers, reviewers, lists.

Sort the list of output addresses alphabetically and use set()
instead of OrderedDict() to accumulate unique addresses.

Changes since v2:
- Reworked internal return logic to use dictionaries.
- Reordered cleanup before new functionality.
Changes since v1:
- Split into patch series

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Michael D Kinney 

Leif Lindholm (1):
   BaseTools/Scripts/GetMaintainer: refactor internal returns as dicts

Michael D Kinney (4):
   BaseTools/Scripts/GetMaintainer: Fix logic bug collecting maintainers
   BaseTools/Scripts/GetMaintainer: Simplify logic
   BaseTools/Scripts/GetMaintainer: Handle reviewer only case
   BaseTools/Scripts/GetMaintainer: Sort output addresses

  BaseTools/Scripts/GetMaintainer.py | 43 +++---
  1 file changed, 27 insertions(+), 16 deletions(-)




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111070): https://edk2.groups.io/g/devel/message/111070
Mute This Topic: https://groups.io/mt/102513765/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test v2 4/4] Fix the URL for the edk2-test repo

Fix the URL for the edk2-test repo: the uefi-sct is a directory inside
the repo.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 Maintainers.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index a94255f015ad..ae6dd7008bcd 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -28,7 +28,7 @@ Descriptions of section entries:
 
 UEFI-SCT
 --
-T: git - https://github.com/tianocore/edk2-test/uefi-sct
+T: git - https://github.com/tianocore/edk2-test
 
 
 Responsible Disclosure, Reporting Security Issues
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111062): https://edk2.groups.io/g/devel/message/111062
Mute This Topic: https://groups.io/mt/102513318/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test v2 3/4] Point users to the URL for edk2-test-parser if it doesn't exist

If edk2-test-parser doesn't exist, tell the user where they can clone it
from.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 uefi-sct/SctPkg/buildzip.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/uefi-sct/SctPkg/buildzip.sh b/uefi-sct/SctPkg/buildzip.sh
index c7f4673f62b2..639d5152c9d2 100755
--- a/uefi-sct/SctPkg/buildzip.sh
+++ b/uefi-sct/SctPkg/buildzip.sh
@@ -54,6 +54,7 @@ cp Build/MdeModule/RELEASE_GCC5/${TARGET_ARCH}/CapsuleApp.efi 
${TARGET_ARCH}_SCT
 if [ ! -d "${pwd}edk2-test-parser" ]
 then
 echo "edk2-test-parser repo is missing from current directory, please 
clone and try again"
+echo "The URL for edk2-test-parser is 
https://git.gitlab.arm.com/systemready/edk2-test-parser.git;
 exit
 fi
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111061): https://edk2.groups.io/g/devel/message/111061
Mute This Topic: https://groups.io/mt/102513315/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test v2 2/4] Rename files in HowToBuild to avoid spaces in filenames

Rename the text files in the HowToBuild directory to remove the spaces
and use CamelCase instead.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} | 0
 uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt}
  | 0
 uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt}  | 0
 3 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/uefi-sct/HowToBuild/How to accelerate SCT execution.txt 
b/uefi-sct/HowToBuild/HowToAccelerateSctExecution.txt
similarity index 100%
rename from uefi-sct/HowToBuild/How to accelerate SCT execution.txt
rename to uefi-sct/HowToBuild/HowToAccelerateSctExecution.txt
diff --git a/uefi-sct/HowToBuild/How to build SCT.txt 
b/uefi-sct/HowToBuild/HowToBuildSct.txt
similarity index 100%
rename from uefi-sct/HowToBuild/How to build SCT.txt
rename to uefi-sct/HowToBuild/HowToBuildSct.txt
diff --git a/uefi-sct/HowToBuild/How to build SCT in UDK2017.txt 
b/uefi-sct/HowToBuild/HowToBuildSctInUdk2017.txt
similarity index 100%
rename from uefi-sct/HowToBuild/How to build SCT in UDK2017.txt
rename to uefi-sct/HowToBuild/HowToBuildSctInUdk2017.txt
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111060): https://edk2.groups.io/g/devel/message/111060
Mute This Topic: https://groups.io/mt/102513314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test v2 1/4] Unbreak buildzip.sh

Unbreak buildzip.sh by removing the line sourcing edksetup.sh: unless
WORKSPACE and PACKAGES_PATH are already configured, edksetup.sh will
error out.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 uefi-sct/SctPkg/buildzip.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/uefi-sct/SctPkg/buildzip.sh b/uefi-sct/SctPkg/buildzip.sh
index 625475701f12..c7f4673f62b2 100755
--- a/uefi-sct/SctPkg/buildzip.sh
+++ b/uefi-sct/SctPkg/buildzip.sh
@@ -32,9 +32,6 @@ esac
 # clear all positional parameters
 set --
 
-source ./edk2/edksetup.sh
-
-
 NUM_CPUS=$((`getconf _NPROCESSORS_ONLN` + 2))
 
 make -j"$NUM_CPUS" -C edk2/BaseTools/
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111059): https://edk2.groups.io/g/devel/message/111059
Mute This Topic: https://groups.io/mt/102513313/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test v2 0/4] Various improvements to the repo

Some improvements to the edk2-test repository.

Since buildzip.sh creates output that appears more useful for running on
the target machine, I wonder if we might want to mention it in the
documentation someplace?

Also, I suspect the HowToBuildSctInUdk2017.txt file is now pretty
outdated. Should we delete it?

Finally, is the TianoCore Contribution Agreement still relevant? I know
it is for the edk2 docs repositories, but not sure if edk2-test should
be updated to match the other code repos like edk2 and edk2-platforms?

CHANGES FROM v1 to v2:

Converted tabs to spaces in the echo line added to buildzip.sh

Rebecca Cran (4):
  Unbreak buildzip.sh
  Rename files in HowToBuild to avoid spaces in filenames
  Point users to the URL for edk2-test-parser if it doesn't exist
  Fix the URL for the edk2-test repo

 Maintainers.txt
  | 2 +-
 uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} | 0
 uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt}
  | 0
 uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt}  | 0
 uefi-sct/SctPkg/buildzip.sh
  | 4 +---
 5 files changed, 2 insertions(+), 4 deletions(-)
 rename uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} (100%)
 rename uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt} (100%)
 rename uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt} (100%)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111058): https://edk2.groups.io/g/devel/message/111058
Mute This Topic: https://groups.io/mt/102513312/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-test 0/4] Various improvements to the repo

It looks like Maintainers.txt also needs to be updated since I got 
bounces from Eric Jin and Arvin Chen's email addresses.



On 11/10/23 12:01, Rebecca Cran wrote:

Some improvements to the edk2-test repository.

Since buildzip.sh creates output that appears more useful for running on
the target machine, I wonder if we might want to mention it in the
documentation someplace?

Also, I suspect the HowToBuildSctInUdk2017.txt file is now pretty
outdated. Should we delete it?

Finally, is the TianoCore Contribution Agreement still relevant? I know
it is for the edk2 docs repositories, but not sure if edk2-test should
be updated to match the other code repos like edk2 and edk2-platforms?

Rebecca Cran (4):
   Unbreak buildzip.sh
   Rename files in HowToBuild to avoid spaces in filenames
   Point users to the URL for edk2-test-parser if it doesn't exist
   Fix the URL for the edk2-test repo

  Maintainers.txt   
   | 2 +-
  uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} | 0
  uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt}   
   | 0
  uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt}  | 0
  uefi-sct/SctPkg/buildzip.sh   
   | 4 +---
  5 files changed, 2 insertions(+), 4 deletions(-)
  rename uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} (100%)
  rename uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt} (100%)
  rename uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt} (100%)




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111057): https://edk2.groups.io/g/devel/message/111057
Mute This Topic: https://groups.io/mt/102513210/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test 4/4] Fix the URL for the edk2-test repo

Fix the URL for the edk2-test repo: the uefi-sct is a directory inside
the repo.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 Maintainers.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index a94255f015ad..ae6dd7008bcd 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -28,7 +28,7 @@ Descriptions of section entries:
 
 UEFI-SCT
 --
-T: git - https://github.com/tianocore/edk2-test/uefi-sct
+T: git - https://github.com/tianocore/edk2-test
 
 
 Responsible Disclosure, Reporting Security Issues
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111056): https://edk2.groups.io/g/devel/message/111056
Mute This Topic: https://groups.io/mt/102513216/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test 3/4] Point users to the URL for edk2-test-parser if it doesn't exist

If edk2-test-parser doesn't exist, tell the user where they can clone it
from.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 uefi-sct/SctPkg/buildzip.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/uefi-sct/SctPkg/buildzip.sh b/uefi-sct/SctPkg/buildzip.sh
index c7f4673f62b2..cc02ad3056b6 100755
--- a/uefi-sct/SctPkg/buildzip.sh
+++ b/uefi-sct/SctPkg/buildzip.sh
@@ -54,6 +54,7 @@ cp Build/MdeModule/RELEASE_GCC5/${TARGET_ARCH}/CapsuleApp.efi 
${TARGET_ARCH}_SCT
 if [ ! -d "${pwd}edk2-test-parser" ]
 then
 echo "edk2-test-parser repo is missing from current directory, please 
clone and try again"
+   echo "The URL for edk2-test-parser is 
https://git.gitlab.arm.com/systemready/edk2-test-parser.git;
 exit
 fi
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111055): https://edk2.groups.io/g/devel/message/111055
Mute This Topic: https://groups.io/mt/102513215/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test 1/4] Unbreak buildzip.sh

Unbreak buildzip.sh by removing the line sourcing edksetup.sh: unless
WORKSPACE and PACKAGES_PATH are already configured, edksetup.sh will
error out.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 uefi-sct/SctPkg/buildzip.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/uefi-sct/SctPkg/buildzip.sh b/uefi-sct/SctPkg/buildzip.sh
index 625475701f12..c7f4673f62b2 100755
--- a/uefi-sct/SctPkg/buildzip.sh
+++ b/uefi-sct/SctPkg/buildzip.sh
@@ -32,9 +32,6 @@ esac
 # clear all positional parameters
 set --
 
-source ./edk2/edksetup.sh
-
-
 NUM_CPUS=$((`getconf _NPROCESSORS_ONLN` + 2))
 
 make -j"$NUM_CPUS" -C edk2/BaseTools/
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111053): https://edk2.groups.io/g/devel/message/111053
Mute This Topic: https://groups.io/mt/102513211/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test 2/4] Rename files in HowToBuild to avoid spaces in filenames

Rename the text files in the HowToBuild directory to remove the spaces
and use CamelCase instead.

Signed-off-by: Rebecca Cran 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 uefi-sct/HowToBuild/{How to accelerate SCT execution.txt => 
HowToAccelerateSctExecution.txt} | 0
 uefi-sct/HowToBuild/{How to build SCT.txt => HowToBuildSct.txt}
  | 0
 uefi-sct/HowToBuild/{How to build SCT in UDK2017.txt => 
HowToBuildSctInUdk2017.txt}  | 0
 3 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/uefi-sct/HowToBuild/How to accelerate SCT execution.txt 
b/uefi-sct/HowToBuild/HowToAccelerateSctExecution.txt
similarity index 100%
rename from uefi-sct/HowToBuild/How to accelerate SCT execution.txt
rename to uefi-sct/HowToBuild/HowToAccelerateSctExecution.txt
diff --git a/uefi-sct/HowToBuild/How to build SCT.txt 
b/uefi-sct/HowToBuild/HowToBuildSct.txt
similarity index 100%
rename from uefi-sct/HowToBuild/How to build SCT.txt
rename to uefi-sct/HowToBuild/HowToBuildSct.txt
diff --git a/uefi-sct/HowToBuild/How to build SCT in UDK2017.txt 
b/uefi-sct/HowToBuild/HowToBuildSctInUdk2017.txt
similarity index 100%
rename from uefi-sct/HowToBuild/How to build SCT in UDK2017.txt
rename to uefi-sct/HowToBuild/HowToBuildSctInUdk2017.txt
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111054): https://edk2.groups.io/g/devel/message/111054
Mute This Topic: https://groups.io/mt/102513212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-test 0/4] Various improvements to the repo