Re: [edk2-devel] [PATCH] Emulator/X86EmulatorDxe: Replace with MultiArchUefiPkg build

2024-09-06 Thread Rebecca Cran via groups.io

On 9/6/2024 9:25 AM, Ard Biesheuvel via groups.io wrote:

arm64 firmware? Or RISC-V? There are many more options now for native
drivers on arm64, so I'd expect the relevance of this hack to diminish
but I guess RISC-V is at a different point on this curve.


arm64. The big thing that's missing is support for PCIe graphics cards 
during boot: lots of people want to be able to see the splash screen and 
change settings without needing to connect to the BMC VGA port.


Currently that means we need an x86 emulator to load the optrom on the 
vast majority of cards.



LGPL is more permissive, so the combined work cannot be used under the
LGPL terms only under GPL (with the usual disclaimer of me not being a
lawyer)

Note that it doesn't really matter for edk2-non-osi as long as the
LICENSE file is accurate.


That's my understanding too. My confusion is that Unicorn has two 
license files:


https://github.com/intel/unicorn-for-efi/blob/main/COPYING

https://github.com/intel/unicorn-for-efi/blob/main/COPYING.LGPL2

I don't know if that just means that parts are under the GPL and others 
the LGPL (which would still mean the combined project is GPL-licensed).


--

Rebecca



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




Re: [edk2-devel] [PATCH] Emulator/X86EmulatorDxe: Replace with MultiArchUefiPkg build

2024-09-06 Thread Rebecca Cran via groups.io

Hi Andrei,


I've been talking to a few people about X86EmulatorPkg and their 
experience with it hasn't been positive: apparently there are lots of 
cases where it's failed to work (i.e. caused crashes) which is why it's 
not been more widely used.


Since suggesting they try MultiArchUefiPkg they've said it's working 
much better, and in fact some people have proceeded with enabling it in 
their firmware.



I'm a bit confused about the licensing of MultiArchUefiPkg given the 
license file says it's LGPL while it uses the GPL-licensed Unicorn library.


Does that mean the overall licensing for it will be GPL?


--

Rebecca


On 9/5/2024 11:32 PM, Andrei Warkentin via groups.io wrote:

Hey folks,

Sorry for the late response.

- My suggestion is not to replace, but augment. MultiArchUefiPkg is fairly new, 
not particularly well adopted, surely with some flaws lurking here and there 
and for certain with no guarantee of fitness or proof that it will work 100% or 
100% better than X86EmulatorPkg for every single use case of X86EmulatorPkg in 
the wild. X86EmulatorPkg has been around for a lot longer than its rewrite, and 
I think the people actively using X86EmulatorPkg should continue to have the 
option to continue using it.

- Unicorn has been a blessing and a curse, although abstracting the emulator 
with an API seems like a winner. An API-compatible emulator or JIT is something 
I've been thinking about, but not really doing anything about it for the 
moment. So yes, actual licensing of binaries is cursed.

- All memory but the zero page is visible to the emulator. See 
CpuNullReadCb/CpuNullWriteCb. See also TestNullDeref in 
Application/EmulatorTest. I don't remember if I added the behavior because this 
is what the original did, or because I actively tripped on the NULL accesses 
from some x86 code I was testing...

A


-Original Message-
From: Ard Biesheuvel 
Sent: Sunday, September 1, 2024 3:05 AM
To: Rebecca Cran ; Warkentin, Andrei

Cc: devel@edk2.groups.io; quic_llind...@quicinc.com; Kinney, Michael D

Subject: Re: [PATCH] Emulator/X86EmulatorDxe: Replace with
MultiArchUefiPkg build

Hi Rebecca,

On Sun, 1 Sept 2024 at 00:33, Rebecca Cran  wrote:

Replace the old X86EmulatorDxe with one built from
https://github.com/intel/MultiArchUefiPkg. This is a much more modern,
recent implementation that's more reliable and is actively maintained.

Add driver binaries for both AArch64 and RISCV64, along with the
LoadOpRom application.

Signed-off-by: Rebecca Cran 
---
  Emulator/X86EmulatorDxe/AArch64/EmulatorDxe.depex | Bin 0 -> 54

bytes

  Emulator/X86EmulatorDxe/AArch64/EmulatorDxe.efi   | Bin 0 -> 573440

bytes

  Emulator/X86EmulatorDxe/AArch64/LoadOpRom.efi | Bin 0 -> 28672

bytes

  Emulator/X86EmulatorDxe/README.md |  11 +++
  Emulator/X86EmulatorDxe/RISCV64/EmulatorDxe.depex | Bin 0 -> 54

bytes

  Emulator/X86EmulatorDxe/RISCV64/EmulatorDxe.efi   | Bin 0 -> 561216

bytes

  Emulator/X86EmulatorDxe/RISCV64/LoadOpRom.efi | Bin 0 -> 30848

bytes

  Emulator/X86EmulatorDxe/X86EmulatorDxe.depex  | Bin 36 -> 0 bytes
  Emulator/X86EmulatorDxe/X86EmulatorDxe.efi| Bin 913408 -> 0

bytes

  Emulator/X86EmulatorDxe/X86EmulatorDxe.inf|  10 +++---
  10 files changed, 14 insertions(+), 7 deletions(-)


Happy to see that this work has been absorbed into a project that will
improve and maintain it going forward.

However, according to the github.com repo Readme of
MultiArchUefiCpuPkg, these binaries include statically linked UniCorn builds,
which are a mix of LPGL and GPLv2, so you will need to update the LICENSE
file accordingly.

Also, glancing over the code, it seems like it removes the NULL pointer
dereference handling that the original X86EmulatorPkg has - this code was
added for a purpose, as some Nvidia option ROMs will happily dereference
NULL pointers, which happens to work on most older
X64 firmware because page 0x0 happens to be mapped. Perhaps Andrei can
comment on this point?








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




Re: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc: Move to github pull request workflow

2024-09-04 Thread Rebecca Cran via groups.io
We don't need AssignReviewers.yaml since there are no reviewers only 
maintainers.


Otherwise it looks good.


--

Rebecca


On 9/4/2024 9:20 AM, Jayaprakash, N via groups.io wrote:

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

Migrate data from Maintainers.txt to the GitHub standard CODEOWNERS
and CONTRIBUTORS.md. The latter file contains mappings from name to
email address and GitHub usernames, which will help people who want
to email maintainers instead of using GitHub.

Add .github/workflows/AssignReviewers.yml which adds reviewers to a
Pull Request based on the content of the CODEOWNERS file.

Cc: Rebecca Cran 
Cc: Michael D Kinney 
Cc: Jayaprakash N 
Signed-off-by: Jayaprakash N 
---
  .github/workflows/AssignReviewers.yaml | 34 
  CODEOWNERS | 21 
  CONTRIBUTORS.md| 13 +
  Maintainers.txt| 71 --
  4 files changed, 68 insertions(+), 71 deletions(-)
  create mode 100644 .github/workflows/AssignReviewers.yaml
  create mode 100644 CODEOWNERS
  create mode 100644 CONTRIBUTORS.md
  delete mode 100644 Maintainers.txt

diff --git a/.github/workflows/AssignReviewers.yaml 
b/.github/workflows/AssignReviewers.yaml
new file mode 100644
index 000..4d0779e
--- /dev/null
+++ b/.github/workflows/AssignReviewers.yaml
@@ -0,0 +1,34 @@
+## @file
+# Assign reviewers from a REVIEWERS file using CODEOWNERS syntax
+#
+# Copyright (c) 2024, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+name: Assign reviewers from a REVIEWERS file using CODEOWNERS syntax
+
+on:
+  pull_request_target:
+types: [opened, synchronize, reopened, ready_for_review]
+branches:
+  - master
+
+jobs:
+  assign_reviewers:
+if: github.event.pull_request.draft == false
+runs-on: ubuntu-latest
+permissions:
+  pull-requests: write
+steps:
+  - name: Generate Token
+id: generate-token
+uses: actions/create-github-app-token@v1
+with:
+  app-id: ${{ secrets.TIANOCORE_ASSIGN_REVIEWERS_APPLICATION_ID }}
+  private-key: ${{ 
secrets.TIANOCORE_ASSIGN_REVIEWERS_APPLICATION_PRIVATE_KEY }}
+  - name: Checkout Pull Request Target
+uses: actions/checkout@v2
+  - name: Assign Reviewers
+uses: mdkinney/github-action-assign-reviewers@main
+with:
+  token: ${{ steps.generate-token.outputs.token }}
\ No newline at end of file
diff --git a/CODEOWNERS b/CODEOWNERS
new file mode 100644
index 000..f171244
--- /dev/null
+++ b/CODEOWNERS
@@ -0,0 +1,21 @@
+## @file
+# This file contains the code owners of edk2-libc repo
+#
+# Copyright (c) 2024, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+# This file contains the list of maintainers (i.e. people who own the
+# areas and can commit changes) for various parts of edk2-libc.
+
+# EDK II Platforms maintainers
+# 
+
+* @mdkinney @bcran @jpshivakavi
+
+# AppPkg owners
+AppPkg/** @mdkinney @jpshivakavi @bcran
+
+# StdLib and StdLibPrivateInternalFiles owners
+StdLib/** @mdkinney @jpshivakavi @bcran
+StdLibPrivateInternalFiles/** @mdkinney @jpshivakavi @bcran
diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
new file mode 100644
index 000..5173898
--- /dev/null
+++ b/CONTRIBUTORS.md
@@ -0,0 +1,13 @@
+EDK II Libc Maintainers and Reviewers
+==
+
+This file is intended to provide an easy way to look up people's names and 
email addresses given their GitHub usernames from the CODEOWNERS / REVIEWERS 
files.
+
+Since it's an extra file to remember to update when changing maintainers or 
reviewers, it will likely become out-of-sync with CODEOWNERS and/or REVIEWERS 
over time and need an occasional refresh.
+
+
+| Name | e-mail address| Githubusername
  |
+|--|---|-|
+| Rebecca Cran | rebe...@bsdio.com | 
[@bcran](https://github.com/bcran)  |
+| Michael D Kinney | michael.d.kin...@intel.com| 
[@mdkinney](https://github.com/mdkinney)|
+| Jayaprakash Nevara   | n.jayaprak...@intel.com   | 
[@jpshivakavi](https://github.com/jpshivakavi)  |
diff --git a/Maintainers.txt b/Maintainers.txt
deleted file mode 100644
index 8a98987..000
--- a/Maintainers.txt
+++ /dev/null
@@ -1,71 +0,0 @@
-EDK II LIBC Project Maintainers
-===
-
-This file provides information about the primary maintainers for the EDK II 
LIBC
-Project.  This repository was exported from the edk2 repository and depends
-on the edk2 repository.  The following are the links to the edk2 repository:
-
-https://github.com/tianocore/edk2
-https://github.com/tianocore/edk2/blob/master/Readme.md
-https://github.com/tianocore

[edk2-devel] SbsaQemu: SIP_SVC_GET_CPU_TOPOLOGY call failed (need updated binaries in edk2-non-osi?)

2024-09-03 Thread Rebecca Cran via groups.io

Do the TF-A binaries in edk2-non-osi need updated for SbsaQemu?

With the binaries in Platform/Qemu/Sbsa I get the following error during 
boot:


GetCpuCount: We have 4 cpus.
GetMpidr: MPIDR for CPU0: = 0
GetMpidr: MPIDR for CPU1: = 1
GetMpidr: MPIDR for CPU2: = 2
GetMpidr: MPIDR for CPU3: = 3
GetCpuCount: We have 4 cpus.
ERROR:   sbsa_sip_smc_handler: unhandled SMC (0xc2ca) (function id: 202)
GetCpuTopology: SIP_SVC_GET_CPU_TOPOLOGY call failed. We have no cpu 
topology information.



If I update the binaries with a fresh set built from TF-A commit 
8e9bdc5b1d65f49546afbe97943d263a1332e67e it successfully boots to the 
UEFI Shell.



--

Rebecca



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




Re: [edk2-devel] [RFC PATCH 1/1] MdePkg/IndustryStandard: add definitions for ACPI 6.4 CEDT

2024-09-03 Thread Rebecca Cran via groups.io
Also, leading underscores are supposed to be reserved for compiler 
implementations (and there only needs to be a single trailing 
underscore) so it should really be:



__CXL_Early_Discovery_TABLE_H__ -> CXL_EARLY_DISCOVERY_TABLE_H_

--
Rebecca


On 8/30/2024 12:06 PM, Michael D Kinney via groups.io wrote:

For this MdePkg change to add an ACPI table type, do you mind opening a PR?

There are some minor code style issues that need to be addressed.

Structure type names and define names should be all upper case.

__CXL_Early_Discovery_TABLE_H__ -> __CXL_EARLY_DISCOVERY_TABLE_H__

File names should be camel case.

CXLEarlyDiscoveryTable.h -> CxlEarlyDiscoveryTable.h

Also, please provide links to the supporting public specifications in
the include file headers.

Thanks,

Mike


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Jonathan
Cameron via groups.io
Sent: Friday, August 30, 2024 4:17 AM
To: Yuquan Wang 
Cc: marcin.juszkiew...@linaro.org; gaolim...@byosoft.com.cn; Kinney,
Michael D ; ardb+tianoc...@kernel.org;
chenba...@phytium.com.cn; wangyinf...@phytium.com.cn;
shuy...@phytium.com.cn; qemu-de...@nongnu.org; devel@edk2.groups.io
Subject: Re: [edk2-devel] [RFC PATCH 1/1] MdePkg/IndustryStandard: add
definitions for ACPI 6.4 CEDT

On Fri, 30 Aug 2024 10:11:17 +0800
Yuquan Wang  wrote:
One request - when cross posting to multiple lists, it is useful to
add something to the patch title to make it clear what is EDK2, what
is QEMU etc.

[RFC EDK2 PATCH 1/1]

It might irritate the EDK2 folk but it is useful for everyone else
to figure out what they are looking at.


This adds #defines and struct typedefs for the various structure
types in the ACPI 6.4 CXL Early Discovery Table (CEDT).

It's in the CXL spec, not ACPI spec so call out in this
patch description that all that was added in ACPI 6.4 was
the reservation of the ID.  The rest needs to refer to appropriate
CXL specifications.

For naming I have no idea on the EDK2 Convention for
structures in specifications other than ACPI that are for
ACPI structures.  The current one is definitely missleading
however.



Signed-off-by: Yuquan Wang 
---
  MdePkg/Include/IndustryStandard/Acpi64.h  |  5 ++
  .../IndustryStandard/CXLEarlyDiscoveryTable.h | 69

+++

  2 files changed, 74 insertions(+)
  create mode 100644

MdePkg/Include/IndustryStandard/CXLEarlyDiscoveryTable.h

diff --git a/MdePkg/Include/IndustryStandard/CXLEarlyDiscoveryTable.h

b/MdePkg/Include/IndustryStandard/CXLEarlyDiscoveryTable.h

new file mode 100644
index 00..84f88dc737
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/CXLEarlyDiscoveryTable.h
@@ -0,0 +1,69 @@
+/** @file
+  ACPI CXL Early Discovery Table (CEDT) definitions.
+
+  Copyright (c) 2024, Phytium Technology Co Ltd. All rights reserved.
+
+**/
+
+#ifndef __CXL_Early_Discovery_TABLE_H__
+#define __CXL_Early_Discovery_TABLE_H__
+
+#include 
+#include 
+
+#define EFI_ACPI_CXL_Early_Discovery_TABLE_REVISION_01  0x1

//CXL2.0

+#define EFI_ACPI_CXL_Early_Discovery_TABLE_REVISION_02  0x2

//CXL3.1

+
+#define EFI_ACPI_CEDT_TYPE_CHBS 0x0
+#define EFI_ACPI_CEDT_TYPE_CFMWS0x1

Sensible to add all defines from the start?
So CXIMS, RDPAS and CSDS
(only that last one was added in 3.1 / revision 2.0)



+} EFI_ACPI_6_4_CEDT_Structure;
+
+///
+/// Definition for CXL Host Bridge Structure
+///
+typedef struct {
+  EFI_ACPI_6_4_CEDT_Structureheader;
+  UINT32 UID;
+  UINT32 CXLVersion;
+  UINT32 Reserved;
+  UINT64 Base;
+  UINT64 Length;
+} EFI_ACPI_6_4_CXL_Host_Bridge_Structure;

Should this naming reflect where it's actually defined?
EFI_ACPI_CXL_3_1_CXL_Host_Bridge_Structure etc


+
+///
+/// Definition for CXL Fixed Memory Window Structure
+///
+typedef struct {
+  EFI_ACPI_6_4_CEDT_Structureheader;
+  UINT32 Reserved;
+  UINT64 BaseHPA;
+  UINT64 WindowSize;
+  UINT8  InterleaveMembers;
+  UINT8  InterleaveArithmetic;
+  UINT16 Reserved1;
+  UINT32 Granularity;
+  UINT16 Restrictions;
+  UINT16 QtgId;
+  UINT32 FirstTarget;

Is this common for an EDK2 definition?  If it were kernel we'd
be using a [] to indicate this has variable number of elements.
I'm too lazy to check for EDK2 equivalents ;)


+} EFI_ACPI_6_4_CXL_Fixed_Memory_Window_Structure;
+
+#pragma pack()
+
+#endif














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

Re: [edk2-devel] [PATCH 1/2] AmpereSiliconPkg: Implement BMC Configuration screen

2024-09-03 Thread Rebecca Cran via groups.io

On 8/13/2024 9:18 PM, Nhi Pham wrote:

+/**
+  This function updates the BMC information.
+
+  @param[in] VOID
+
+  @retval EFI_SUCCESS   The entry point is executed successfully.
+  @retval Other Some error occurs when executing this entry point.
+
+**/
+EFI_STATUS
+UpdateBmcConfigForm (
+  VOID
+  )
+{


VOID isn't a parameter so shouldn't be documented via @param.

--

Rebecca



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




[edk2-devel] [PATCH edk2-platforms v2 1/1] Maintainers.txt: Use my personal email address

2024-08-21 Thread Rebecca Cran via groups.io
Work with upstream/community is done on my own time, so update my entry
to contain my personal email address.

Signed-off-by: Rebecca Cran 
---
 Maintainers.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 824838486072..27f4056a6f49 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -123,7 +123,7 @@ F: Silicon/Ampere
 M: Nhi Pham 
 R: Chuong Tran 
 R: Leif Lindholm 
-R: Rebecca Cran 
+R: Rebecca Cran 
 
 ARM
 F: Platform/ARM/
-- 
2.34.1



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




[edk2-devel] [PATCH] Maintainers.txt: Use my personal email address

2024-08-21 Thread Rebecca Cran via groups.io
Work with upstream/community is done on my own time, so update my entry
to contain my personal email address.

Signed-off-by: Rebecca Cran 
---
 Maintainers.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 8248384860..27f4056a6f 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -123,7 +123,7 @@ F: Silicon/Ampere
 M: Nhi Pham 

 R: Chuong Tran 

 R: Leif Lindholm 

-R: Rebecca Cran 

+R: Rebecca Cran 

 

 ARM

 F: Platform/ARM/

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120389): https://edk2.groups.io/g/devel/message/120389
Mute This Topic: https://groups.io/mt/108023202/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 1/1] JadePkg: Add ACPI SPMI table

2024-08-14 Thread Rebecca Cran via groups.io

On 8/13/2024 9:33 PM, Nhi Pham via groups.io wrote:
This could be a static function.

+EFI_STATUS
+EFIAPI
+AcpiInstallSpmiTable (
+  VOID
+  );
+
  #endif /* ACPI_PLATFORM_H_ */
diff --git a/Platform/Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.c 
b/Platform/Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.c
index 28c422dff166..a82a93d23fa2 100644
--- a/Platform/Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.c
+++ b/Platform/Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.c


I believe the project now prefers the use of the C keyword "static".


+// BCD Format
+SpmiTable->SpecificationRevision  = DeviceId.SpecificationVersion & 0xF0;
+SpmiTable->SpecificationRevision |= (DeviceId.SpecificationVersion & 0x0F) 
<< 8;


It took a second to understand this. Maybe combine it into a single 
line, or change the '0x0F' to just '0xF'?


--

Rebecca



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120338): https://edk2.groups.io/g/devel/message/120338
Mute This Topic: https://groups.io/mt/107889268/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 1/1] AmpereAltraPkg/FlashFvbDxe: Sync up NVRAM FV with NVRAM cached

2024-08-09 Thread Rebecca Cran via groups.io

Reviewed-by: Rebecca Cran 


On 8/8/2024 9:12 PM, Nhi Pham wrote:

From: Tam Chi Nguyen 

Currently, the NVRAM FV region is only updated once at FlashPei that makes
the data in NVRAM FV outdated with data in NVRAM region in SPI-NOR.
It causes the duplication of the valid NV variables when the Variable
Reclaim process performs. Consequently, after rebooting, system goes to
an infinite loop at GetNextVariableName.

It requires the data in NVRAM FV to be synced up with NVRAM cache in memory
that is managed by VariableDxe.

Signed-off-by: Nhi Pham 
---
  Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf | 1 +
  Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c   | 3 +++
  2 files changed, 4 insertions(+)

diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf 
b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
index 008fd2315ffe..5f537cf7df27 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
@@ -25,6 +25,7 @@ [Packages]
Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
  
  [LibraryClasses]

+  BaseMemoryLib
DebugLib
FlashLib
PcdLib
diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c 
b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c
index 009694703ddd..853c458e375f 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c
@@ -6,6 +6,7 @@
  
  **/
  
+#include 

  #include 
  #include 
  #include 
@@ -361,6 +362,8 @@ FlashFvbDxeWrite (
  return EFI_DEVICE_ERROR;
}
  
+  CopyMem ((UINT8 *)(UINTN)mNvStorageBase + Lba * mFlashBlockSize + Offset, Buffer, *NumBytes);

+
return Status;
  }
  



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




Re: [edk2-devel] [PATCH RFC edk2-platforms 0/5] Phase out MPCore SEC drivers

2024-07-30 Thread Rebecca Cran via groups.io

For the series:
Reviewed-by: Rebecca Cran 

I see this is marked as an RFC, but I think it's a change that should be 
committed.


--
Rebecca

On 7/29/24 06:22, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

The original EDK2 port to 32-bit ARM supported multi-core but on today's
ARM systems, only a single CPU enters the non-secure firmware and the
MPCore drivers are obsolete.

Stop using them in edk2-platforms so we can remove them entirely from
edk2.

Cc: Leif Lindholm 
Cc: Rebecca Cran 
Cc: Nhi Pham 
Cc: Chuong Tran 
Cc: Wenyi Xie 
Cc: Peng Xie 
Cc: Ling Jia 
Cc: Yiqi Shu 

Ard Biesheuvel (5):
   Platform AARCH64: Drop leftover references to deleted timer PCD
   Platform AARCH64: Remove bogus references to MPCore stack
   Platform/Ampere: Switch to unicore SEC implementation
   Platform/Durian: Switch to unicore SEC implementation
   Platform/HiSilicon/D0x: Switch to unicore SEC implementation

  Platform/ARM/Morello/MorelloPlatform.dsc.inc|  2 --
  Platform/ARM/SgiPkg/SgiPlatform.dsc.inc |  2 --
  Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc|  6 +-
  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc   |  1 -
  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc  |  2 --
  Platform/ARM/N1Sdp/N1SdpPlatform.dsc|  2 --
  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc|  2 --
  Platform/Hisilicon/D03/D03.dsc  |  3 +--
  Platform/Hisilicon/D05/D05.dsc  | 10 +-
  Platform/Hisilicon/D06/D06.dsc  |  3 +--
  Platform/LeMaker/CelloBoard/CelloBoard.dsc  |  2 --
  Platform/Phytium/DurianPkg/DurianPkg.dsc|  2 +-
  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc |  2 --
  Platform/Ampere/JadePkg/Jade.fdf|  2 +-
  Platform/Hisilicon/D03/D03.fdf  |  2 +-
  Platform/Hisilicon/D05/D05.fdf  |  2 +-
  Platform/Hisilicon/D06/D06.fdf  |  2 +-
  Platform/Phytium/DurianPkg/DurianPkg.fdf|  2 +-
  18 files changed, 10 insertions(+), 39 deletions(-)

--
2.46.0.rc1.232.g9752f9e123-goog




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




Re: [edk2-devel] [PATCH edk2-platforms 1/1] Prepare for move to GitHub Pull Request workflow

2024-07-26 Thread Rebecca Cran via groups.io

On 7/26/24 11:37, Michael D Kinney via groups.io wrote:


Members with more than one email address is confusing.  Can we
limit to single email address for TianoCore activities?

Also, should Maintainer.txt be removed yet?  There is a GitHub
Action active in edk2 now that assigns reviews from Maintainer.txt.


Sure, I can have all my edk2-platforms work under my 
os.amperecomputing.com email.


I'd like to remove Maintainers.txt now to avoid _another_ file that can 
get out of sync. I know edk2 has chosen to use the Github Action for 
now, but I'd prefer edk2-platforms to move directly to using CODEOWNERS 
(and in a few days, REVIEWERS when I write the script to handle it).


--
Rebecca


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




Re: [edk2-devel] GitHub PR Code Review process now active

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

On 6/5/2024 4:21 PM, Michael D Kinney via groups.io wrote:

* Some PRs have been merged using the "Rebase and Merge" button in the
   PR after all required reviews completed and all CI checks pass. Instead,
   the "push" label should continue to be used. There does not appear to be
   any unexpected side effects from the "Rebase and Merge" button, but that
   option is not available if the PR needs to be rebased. This is what
   Mergify handles through a merge queue, so the easiest way to merge right
   now is the "push" label.

   If the most recent commit was not performed by an EDK II Maintainers, then
   Mergify attempt to rebase may fail.

 Mitigation #1: EDK II Maintainer perform a rebase
 Mitigation #2: Update Mergify to use a bot account with write permission
to perform rebase operations.

   There was feedback earlier in the year that the git commit history does
   not indicate which maintainer was the committer.  Instead it always shows
   Mergify.

   The use of GitHub Merge Queues will be evaluated to see if it can be used
   instead of Mergify and remove the need for the "push" label and allow the
   "Rebase and Merge" button to be used and avoid the Mergify permission issues.


So it sounds like using the "Merge" button is fine as long as the user 
understands they may need to rebase, wait for CI to finish and then merge?


Also, is there a timeline on enabling PRs for the other repos? I'd 
really like to switch to them for edk2-platforms, even if it means 
having to update settings in multiple repos as we find issues with the 
process.



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119489): https://edk2.groups.io/g/devel/message/119489
Mute This Topic: https://groups.io/mt/106355103/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 1/1] Ampere/JadePkg: Add secure boot default keys initialization

2024-06-04 Thread Rebecca Cran via groups.io

Reviewed-by: Rebecca Cran 

--
Rebecca Cran

On 6/4/2024 6:57 PM, Nhi Pham wrote:

This allows to initialize secure boot with the default factory keys
embedded in firmware flash image.

For example, to incorporate PK, KEK, and DB default keys, specify the
corresponding key files in the Jade.dsc as follows:

DEFINE DEFAULT_KEYS= TRUE
DEFINE PK_DEFAULT_FILE = path/to/PK.crt
DEFINE KEK_DEFAULT_FILE1   = path/to/KEK.crt
DEFINE DB_DEFAULT_FILE1= path/to/DB1.crt
DEFINE DB_DEFAULT_FILE2= path/to/DB2.crt

Signed-off-by: Nhi Pham 
---
  Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc | 2 ++
  Platform/Ampere/JadePkg/Jade.fdf | 2 ++
  2 files changed, 4 insertions(+)

diff --git a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc 
b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
index 23579497661d..93b4d1d99dcd 100644
--- a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
+++ b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
@@ -590,6 +590,8 @@ [Components.common]
  
  !if $(SECURE_BOOT_ENABLE) == TRUE


SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+  SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
+  
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf
  !endif
MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf

MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
diff --git a/Platform/Ampere/JadePkg/Jade.fdf b/Platform/Ampere/JadePkg/Jade.fdf
index 7795f0e5..1e2df5ba6142 100644
--- a/Platform/Ampere/JadePkg/Jade.fdf
+++ b/Platform/Ampere/JadePkg/Jade.fdf
@@ -219,7 +219,9 @@ [FV.FvMain]
INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
  !if $(SECURE_BOOT_ENABLE) == TRUE
+!include ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
INF 
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+  INF 
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf
  !endif
INF 
MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119465): https://edk2.groups.io/g/devel/message/119465
Mute This Topic: https://groups.io/mt/106495161/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] AArch64 with HeapGuard: page allocations wrongly aligned

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

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

2024-01-19 Thread Rebecca Cran via groups.io
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

2024-01-19 Thread Rebecca Cran via groups.io
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, &gEfiWatchdogTimerArchProtocolGuid);
 
-  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 [

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

2024-01-19 Thread Rebecca Cran via groups.io
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 (
   &gHardwareInterrupt2ProtocolGuid,
@@ -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

2024-01-19 Thread Rebecca Cran via groups.io
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

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

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

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

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

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

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

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

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

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

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

2024-01-18 Thread Rebecca Cran via groups.io
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 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

2024-01-04 Thread Rebecca Cran via groups.io
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

2024-01-04 Thread Rebecca Cran via groups.io
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, &gEfiWatchdogTimerArchProtocolGuid);
 
-  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

2024-01-04 Thread Rebecca Cran via groups.io
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

2024-01-04 Thread Rebecca Cran via groups.io
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

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

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

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

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

2024-01-04 Thread Rebecca Cran via groups.io
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, &gEfiWatchdogTimerArchProtocolGuid);
 
-  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

2024-01-04 Thread Rebecca Cran via groups.io
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

2024-01-04 Thread Rebecca Cran via groups.io
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

2024-01-04 Thread Rebecca Cran via groups.io
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?

2024-01-04 Thread Rebecca Cran via groups.io
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

2024-01-04 Thread Rebecca Cran via groups.io
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

2024-01-03 Thread Rebecca Cran via groups.io
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

2024-01-03 Thread Rebecca Cran via groups.io
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

2024-01-03 Thread Rebecca Cran via groups.io
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

2024-01-03 Thread Rebecca Cran via groups.io
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 0/4] Add DEBUG_MANAGEABILITY to debug level comments

2023-12-08 Thread Rebecca Cran via groups.io

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

2023-12-08 Thread Rebecca Cran via groups.io
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

2023-12-08 Thread Rebecca Cran via groups.io
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

2023-12-08 Thread Rebecca Cran via groups.io
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

2023-12-08 Thread Rebecca Cran via groups.io
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

2023-12-08 Thread Rebecca Cran via groups.io
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] 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 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 v1 0/2] Upgrade edk2-pytools to latest

2023-10-27 Thread Rebecca Cran via groups.io

On 10/27/2023 9:15 AM, Joey Vagedes via groups.io wrote:

Upgrades edk2-pytool-library to v0.19.3 and edk2-pytool-extensions to v0.25.1
and performs all necessary integrations as noted in the individual package
commits.

Cc: Sean Brogan 
Cc: Michael Kubacki 
Cc: Michael D Kinney 
Cc: Liming Gao 

Joey Vagedes (2):
   .pytool: Integration of edk2-pytools
   BaseTools: Plugin: Integration of edk2-pytools

  .pytool/Plugin/HostUnitTestDscCompleteCheck/HostUnitTestDscCompleteCheck.py | 
 7 ---
  .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py   | 
12 ++--
  BaseTools/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.py  | 
10 +-
  pip-requirements.txt| 
 4 ++--
  4 files changed, 17 insertions(+), 16 deletions(-)



For the series:

Reviewed-by: Rebecca Cran 

--
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110218): https://edk2.groups.io/g/devel/message/110218
Mute This Topic: https://groups.io/mt/102223493/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 1/1] AmpereAltraPkg/Ac01PcieLib: drop useless link status register read

2023-10-17 Thread Rebecca Cran via groups.io

On 10/17/2023 4:05 AM, Laszlo Ersek wrote:

Nhi says that reading LINK_CONTROL_LINK_STATUS_REG is redundant; its only
use was debugging (before commit 380b4b40c60d). Thus, we can go farther
than commit 2e27c62ef000, and remove the MmioRead32() call altogether.

Build-tested with "Jade.dsc".

Cc: Chuong Tran 
Cc: Leif Lindholm 
Cc: Nhi Pham 
Cc: Rebecca Cran 
Suggested-by: Nhi Pham 
Signed-off-by: Laszlo Ersek 
---
  Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c 
b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
index fa00c1e36999..dea2e6406dfd 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
@@ -1744,7 +1744,6 @@ Ac01PcieCoreUpdateLink (
)
  {
AC01_PCIE_CONTROLLER  *Pcie;
-  PHYSICAL_ADDRESS  CfgBase;
UINT8 PcieIndex;
UINT32Index;
  
@@ -1761,12 +1760,10 @@ Ac01PcieCoreUpdateLink (

// Loop for all controllers
for (PcieIndex = 0; PcieIndex < RootComplex->MaxPcieController; 
PcieIndex++) {
  Pcie = &RootComplex->Pcie[PcieIndex];
-CfgBase = RootComplex->MmcfgBase + (RootComplex->Pcie[PcieIndex].DevNum << 
DEV_SHIFT);
  
  if (Pcie->Active && !Pcie->LinkUp) {

if (PcieLinkUpCheck (Pcie)) {
  Pcie->LinkUp = TRUE;
-(VOID)MmioRead32 (CfgBase + PCIE_CAPABILITY_BASE + 
LINK_CONTROL_LINK_STATUS_REG);
  
  // Doing link checking and recovery if needed

  Ac01PcieCoreQoSLinkCheckRecovery (RootComplex, PcieIndex);


Reviewed-by: Rebecca Cran 

--
Rebecca Cran


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




Re: [edk2-devel] Maybe it is time to update code style?

2023-09-25 Thread Rebecca Cran via groups.io

On 9/25/2023 3:03 AM, Marcin Juszkiewicz via groups.io wrote:

I feel sick each time I have to edit EDK2 code.

All those INF, DEC, DSC, FDF, XYZ files are something I do not even try
to understand, just got minimal knowledge what goes where by asking Leif
(thanks a lot!) and observing build error messages.

I got used to UINTN and other weird variable types. No idea where from
they came from (MS Windows?)


Given that people at Intel/Microsoft are still adding code with the 
Windows NT style function documentation, I suspect there will be quite a 
resistance to changing things.


For example, the following was added in 2019:


EFI_STATUS
GetSelfTest (
...
  )
/*++

Routine Description:

  Execute the Get Self Test ...

Arguments:

  IpmiInstance- Data structure...

Returns:

  EFI_SUCCESS   - BMC Self test results...

--*/
{
  EFI_STATUS  Status;

Vincent provided the following explanation on Twitter:

"It's from MS, namley the coding style used in Windows NT 
https://computernewb.com/~lily/files/Documents/NTDesignWorkbook/coding.pdf 
. Ken Reneris wrote the original EFI sample at Intel after working 
having worked on various parts of NT, including the HAL, at MS. He 
brought that coding style with him to Intel."


--
Rebecca Cran


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




Re: [edk2-devel] [PATCH edk2-platforms 0/2] some trivial changes

2023-09-20 Thread Rebecca Cran via groups.io

On 9/20/2023 9:55 AM, Marcin Juszkiewicz via groups.io wrote:

Those changes float around whenever I do some changes in code. Time to
send them for merge and stop bothering.

Marcin Juszkiewicz (2):
   Silicon/SbsaQemu: drop duplicated Pcd
   Silicon/SbsaQemu: fix comment to show proper table name

  .../Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf   | 2 --
  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 2 +-
  2 files changed, 1 insertion(+), 3 deletions(-)



Series:

Reviewed-by: Rebecca Cran 

--
Rebecca Cran


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




[edk2-devel] [PATCH edk2-platforms v2 1/1] Platform/Ampere: Update Readme.md

2023-09-18 Thread Rebecca Cran via groups.io
Improve the Readme.md in Platform/Ampere:

Direct people to the top-level Readme.md file for build instructions.
Mention the edk2-ampere-tools repo for additional build tools.
Delete everything else, since it's redundant.

Signed-off-by: Rebecca Cran 
---
 Platform/Ampere/Readme.md | 29 ++--
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/Platform/Ampere/Readme.md b/Platform/Ampere/Readme.md
index 894bad3437b8..b50c80546170 100644
--- a/Platform/Ampere/Readme.md
+++ b/Platform/Ampere/Readme.md
@@ -1,39 +1,14 @@
-# Introduction
-
-This document provides the guideline to build UEFI firmware for Ampere 
Computing's Arm64 reference platforms.
-
-Platform code is located under Platform/Ampere/{Platform Name}Pkg.
-
-Silicon code is located under Silicon/Ampere/Ampere{SoC Name}Pkg.
-
-# Build machines
-
-- x86 Linux host machines running latest Ubuntu or CentOS releases.
-- Arm64 Linux host machines if native compiling. This has been tested on 
Ampere's eMAG and Altra hardware platforms with latest AArch64 CentOS or Ubuntu 
releases.
-
-# How to build (Linux Environment)
+# How to build 
 
 Please follow top-level Readme.md for build instructions.
 
 ## Additional build tools
 
 Ampere provides additional tools and documentation for automating the manual 
process described as described in the top-level README.md,
-and for building a final Tianocore UEFI image that can be flashed onto the 
target system.
+and for building a final TianoCore UEFI image that can be flashed onto the 
target system.
 
 To use these tools, clone the following to the **WORKSPACE** location:
 
 ```bash
 $ git clone https://github.com/AmpereComputing/edk2-ampere-tools.git
 ```
-
-## Notes
-
-If you run into any build issue with the Intel ASL+ Optimizing 
Compiler/Disassembler (IASL) that comes with your Linux distro,
-download and install the IASL compiler from https://acpica.org/. At the time 
of this write-up, we have tested with version 20200110.
-
-```bash
-$ wget https://acpica.org/sites/acpica/files/acpica-unix2-20200110.tar.gz
-$ tar xzf acpica-unix2-20200110.tar.gz
-$ cd acpica-unix2-20200110
-$ make HOST=_CYGWIN && sudo make install
-```
-- 
2.34.1



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




Re: [edk2-devel] Replacing Eth0 with USB Tethering

2023-09-12 Thread Rebecca Cran via groups.io

On 9/12/2023 10:04 AM, CrossedCarpet via groups.io wrote:

Greetings,
As newer laptops start lacking Ethernet ports, I started investigating 
the possibility of using *USB Tethering* as a replacement, to *allow for 
internet connection at boot*.
Current attempts at employing *ifconfig* to configure a Tether 
connection failed to display anything meaningful.
This made me reach the conclusion that UEFI doesn't support this 
protocol and as such one must somehow implement it.
Is this correct? If I were to do so, would you be so kind to give me 
some pointers?
If this is also in the interest of somebody else, a contribution with a 
patch like so would then be welcome?


The UsbNetwork drivers were recently added to edk2 to support this 
functionality.


https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Bus/Usb/UsbNetwork

--
Rebecca Cran


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




Re: [edk2-devel] [PATCH edk2-platforms 1/1] Platform/Ampere: Update Readme.md

2023-09-12 Thread Rebecca Cran via groups.io

On 9/12/2023 3:58 AM, Leif Lindholm via groups.io wrote:


So, it's really good that you add "known good" versions so that if
someone runs into a problem around toolchain version compatibility,
they know what to use. But anything in edk2-platform should be kept up
to date so that it builds on current operating systems.


Really, I think we'd want to remove everything from the Readme.md except 
perhaps the "Additional build tools" section.



--
Rebecca Cran


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




[edk2-devel] [PATCH edk2-platforms 1/1] Platform/Ampere: Update Readme.md

2023-09-11 Thread Rebecca Cran via groups.io
Improve the Readme.md in Platform/Ampere:

- At this point eMAG is irrelevant, and most people are likely
  using Altra systems. Drop mention of it.
- Instead of mentioning the 'latest' versions of CentOS and Ubuntu
  (which will by definition change over time), specify Ubuntu 22.04
  and CentOS 7.
- Fix the link to the acpica download, since content has been moved from
  acpica.org to intel.com.
- Assuming the build is being done on Linux, acpica shouldn't be built
  with a CYGWIN definition.
- To avoid making people wait ages for acpica to build, add `-j8` as an
  example of building in parallel.

Signed-off-by: Rebecca Cran 
---
 Platform/Ampere/Readme.md | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Platform/Ampere/Readme.md b/Platform/Ampere/Readme.md
index 894bad3437b8..066876dab4b5 100644
--- a/Platform/Ampere/Readme.md
+++ b/Platform/Ampere/Readme.md
@@ -8,8 +8,8 @@ Silicon code is located under Silicon/Ampere/Ampere{SoC 
Name}Pkg.
 
 # Build machines
 
-- x86 Linux host machines running latest Ubuntu or CentOS releases.
-- Arm64 Linux host machines if native compiling. This has been tested on 
Ampere's eMAG and Altra hardware platforms with latest AArch64 CentOS or Ubuntu 
releases.
+- x86 Linux host machines running Ubuntu 22.04 or CentOS 7 releases.
+- Arm64 Linux host machines if native compiling. This has been tested on 
Ampere's Altra hardware platforms with AArch64 CentOS 7 or Ubuntu 22.04 
releases.
 
 # How to build (Linux Environment)
 
@@ -32,8 +32,8 @@ If you run into any build issue with the Intel ASL+ 
Optimizing Compiler/Disassem
 download and install the IASL compiler from https://acpica.org/. At the time 
of this write-up, we have tested with version 20200110.
 
 ```bash
-$ wget https://acpica.org/sites/acpica/files/acpica-unix2-20200110.tar.gz
+$ wget https://downloadmirror.intel.com/774850/acpica-unix2-20200110.tar.gz
 $ tar xzf acpica-unix2-20200110.tar.gz
 $ cd acpica-unix2-20200110
-$ make HOST=_CYGWIN && sudo make install
+$ make -j8 && sudo make install
 ```
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108494): https://edk2.groups.io/g/devel/message/108494
Mute This Topic: https://groups.io/mt/101298216/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 1/1] Maintainers.txt: Update maintainers for Ampere platforms

2023-09-07 Thread Rebecca Cran via groups.io

On 9/7/2023 9:46 AM, Nhi Pham wrote:

This adds Rebecca Cran as reviewer, updates myself as maintainer, and
Leif as reviewer. Also, removes Vu Nguyen and Thang Nguyen due to no
longer working on EDK2.

Signed-off-by: Nhi Pham 
---
  Maintainers.txt | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index d1d7613ef48d..d77f0dd35ac4 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -99,11 +99,10 @@ M: Leif Lindholm 
  Ampere Computing
  F: Platform/Ampere
  F: Silicon/Ampere
-R: Nhi Pham 
-R: Vu Nguyen 
-R: Thang Nguyen 
+M: Nhi Pham 
  R: Chuong Tran 
-M: Leif Lindholm 
+R: Leif Lindholm 
+R: Rebecca Cran 
  
  ARM

  F: Platform/ARM/


Reviewed-by: Rebecca Cran 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108405): https://edk2.groups.io/g/devel/message/108405
Mute This Topic: https://groups.io/mt/101217294/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] MdePkg/Library/BaseRngLib: Fix include guard

2023-09-06 Thread Rebecca Cran via groups.io

On 9/6/2023 11:29 AM, Michael Kubacki via groups.io wrote:

From: Michael Kubacki 

The include guard is incomplete and does not define the macro.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Rebecca Cran 
Signed-off-by: Michael Kubacki 
---
  MdePkg/Library/BaseRngLib/BaseRngLibInternals.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h 
b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
index a9cbce15302f..d0abcc3452ee 100644
--- a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
+++ b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
@@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  **/
  
  #ifndef BASE_RNGLIB_INTERNALS_H_

+#define BASE_RNGLIB_INTERNALS_H_
  
  /**

Generates a 16-bit random number.


Reviewed-by: Rebecca Cran 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108343): https://edk2.groups.io/g/devel/message/108343
Mute This Topic: https://groups.io/mt/101198135/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: support arm64 as a platform name in addition to aarch64

2019-04-11 Thread Rebecca Cran via Groups.Io

On 2019-04-11 11:52, Laszlo Ersek wrote:

On 04/11/19 16:23, Philippe Mathieu-Daudé wrote:

On 4/11/19 5:16 AM, Rebecca Cran via Groups.Io wrote:

Some systems such as FreeBSD identify the platform as 'arm64'
and not 'aarch64' as Linux does.

Per https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297
this seems to be a FreeBSD bug.

You are correct, but that issue was filed on 2017-06-26. I don't think
we should hold our breaths, as long as the workaround is simple. And, it
does look simple. (We've worked around worse.)

I do suggest an addition to the patch, however: Rebecca, please add the
link discovered by Phil near the "arm64" matches, in a comment.
Something like:

# work around <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220297>




Thanks, I'll do that. I don't think there's anything to fix, because 
"uname -m" returns "arm64" and "uname -p" "aarch64". The EDK2 Makefiles 
use "uname -m" which I don't think anybody has complained about, but 
which return different values on Linux and FreeBSD.



--

Rebecca Cran


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

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



Re: [edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output

2019-04-11 Thread Rebecca Cran via Groups.Io

On 2019-04-11 11:35, Laszlo Ersek wrote:

On 04/11/19 16:32, Philippe Mathieu-Daudé wrote:


Rebecca: do you remember if you modified something in your profile?
How are you sending your patches to the list?

Sorry you became our 'patches via groups.io' guinea pig ;)


Yes, I've changed those fields.



If I remember correctly, this is somehow related to Outlook (i.e., the sender's 
MUA / first MTA), not the mailing list(s), or git.



I'm sending emails on Thunderbird via Postfix, so there's no Outlook 
involved.



--
Rebecca Cran


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

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



Re: [edk2-devel] Building EDK2 code on arm64 (aarch64) - BaseTools fails

2019-04-10 Thread Rebecca Cran via Groups.Io

On 2019-04-10 20:07, Rebecca Cran via Groups.Io wrote:
Thanks. I'm working on fixing it now: it looks like it needs new 
compiler flags in BaseTools/Source/C/Makefiles/header.makefile, as 
well as recognizing arm64 as a platform name in addition to aarch64



It didn't need any compiler flag changes after all: it only needed the 
change to recognize 'arm64' as a platform name, in addition to avoiding 
falling through to the 'arm' case which was causing it to still pull in 
the ARM instead of AARCH64 headers.



--
Rebecca Cran


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

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



[edk2-devel] [PATCH] BaseTools: support arm64 as a platform name in addition to aarch64

2019-04-10 Thread Rebecca Cran via Groups.Io
Some systems such as FreeBSD identify the platform as 'arm64'
and not 'aarch64' as Linux does.

Signed-off-by: Rebecca Cran 
---
 BaseTools/Source/C/GNUmakefile   | 5 +++--
 BaseTools/Source/C/Makefiles/header.makefile | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GNUmakefile b/BaseTools/Source/C/GNUmakefile
index 1d048c4cc6..37bcce519c 100644
--- a/BaseTools/Source/C/GNUmakefile
+++ b/BaseTools/Source/C/GNUmakefile
@@ -21,8 +21,9 @@ ifndef HOST_ARCH
   endif
   ifneq (,$(findstring aarch64,$(uname_m)))
 HOST_ARCH=AARCH64
-  endif
-  ifneq (,$(findstring arm,$(uname_m)))
+  else ifneq (,$(findstring arm64,$(uname_m)))
+HOST_ARCH=AARCH64
+  else ifneq (,$(findstring arm,$(uname_m)))
 HOST_ARCH=ARM
   endif
   ifndef HOST_ARCH
diff --git a/BaseTools/Source/C/Makefiles/header.makefile 
b/BaseTools/Source/C/Makefiles/header.makefile
index 90fb3453ad..d76b8283dd 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -23,8 +23,9 @@ ifndef HOST_ARCH
   endif
   ifneq (,$(findstring aarch64,$(uname_m)))
 HOST_ARCH=AARCH64
-  endif
-  ifneq (,$(findstring arm,$(uname_m)))
+  else ifneq (,$(findstring arm64,$(uname_m)))
+HOST_ARCH=AARCH64
+  else ifneq (,$(findstring arm,$(uname_m)))
 HOST_ARCH=ARM
   endif
   ifndef HOST_ARCH
-- 
2.20.1


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

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



Re: [edk2-devel] Building EDK2 code on arm64 (aarch64) - BaseTools fails

2019-04-10 Thread Rebecca Cran via Groups.Io

On 2019-04-10 18:09, Liming Gao wrote:

Yes. No test on FreeBSD. If possible, can you contribute the patch to fix this 
issue on FreeBSD?




Thanks. I'm working on fixing it now: it looks like it needs new 
compiler flags in BaseTools/Source/C/Makefiles/header.makefile, as well 
as recognizing arm64 as a platform name in addition to aarch64.



--
Rebecca Cran


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

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



Re: [edk2-devel] Building EDK2 code on arm64 (aarch64) - BaseTools fails

2019-04-10 Thread Rebecca Cran via Groups.Io

On 2019-04-10 08:41, Philippe Mathieu-Daudé wrote:


This doesn't look like an architecture problem. It seems your GCC
version isn't checked correctly by BaseTools. This warning is supposed
to be disabled, see:

StdLib/LibC/Stdio/Stdio.inf:  GCC:*_*_*_CC_FLAGS= -fno-builtin
-Wno-pointer-to-int-cast -Wno-int-to-pointer-cast -Wno-format



Thanks! I shouldn't have posted so late, I didn't look closely enough at 
the error messages before sending my email.



--
Rebecca Cran


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

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



[edk2-devel] Building EDK2 code on arm64 (aarch64) - BaseTools fails

2019-04-09 Thread Rebecca Cran via Groups.Io

Is there any expectation that EDK2 will build on non-x86 systems?

I tried building BaseTools (from git master, 
ae2fb9ead47b5abaf2a4e815b5f57c8f4838b221) using GCC 8.2 on a SoftIron 
OverDrive 1000 (running FreeBSD) but there are lots of errors, such as:



gcc  -c  -I .. -I ../Include/Common -I ../Include/ -I 
../Include/IndustryStandard -I ../Common/ -I .. -I . -I ../Include/Arm/  
-MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
-Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict 
-Wno-unused-result -nostdlib -g -O2  BasePeCoff.c -o BasePeCoff.o

BasePeCoff.c: In function 'PeCoffLoaderGetPeHeader':
BasePeCoff.c:120:49: error: cast from pointer to integer of different 
size [-Werror=pointer-to-int-cast]
   *PeHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *) 
((UINTN)ImageContext->Handle + ImageContext->PeCoffHeaderOffset);

 ^
BasePeCoff.c:120:12: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]
   *PeHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *) 
((UINTN)ImageContext->Handle + ImageContext->PeCoffHeaderOffset);

    ^
BasePeCoff.c: In function 'PeCoffLoaderImageAddress':
BasePeCoff.c:551:10: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]

   return (UINT8 *) ((UINTN) ImageContext->ImageAddress + Address);
  ^


--
Rebecca Cran


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

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



[edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output

2019-04-04 Thread Rebecca Cran via Groups.Io
Fix a few typos in LegacyBiosBuildE820, and improve the debug output of
the E820 table to pad with zeros instead of spaces, remove extra hyphens
and display the memory type in decimal.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Rebecca Cran 
---
 .../Csm/LegacyBiosDxe/LegacyBootSupport.c  | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
index a7b8e6a9a0..8c415cdfc6 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
@@ -1711,9 +1711,9 @@ LegacyBiosBuildE820 (
 
   do {
 //
-// Use size returned back plus 1 descriptor for the AllocatePool.
+// Use size returned for the AllocatePool.
 // We don't just multiply by 2 since the "for" loop below terminates on
-// EfiMemoryMapEnd which is dependent upon EfiMemoryMapSize. Otherwize
+// EfiMemoryMapEnd which is dependent upon EfiMemoryMapSize. Otherwise
 // we process bogus entries and create bogus E820 entries.
 //
 EfiMemoryMap = (EFI_MEMORY_DESCRIPTOR *) AllocatePool (EfiMemoryMapSize);
@@ -1801,7 +1801,7 @@ LegacyBiosBuildE820 (
 MemoryBlockLength = (UINT64) (LShiftU64 (EfiEntry->NumberOfPages, 12));
 if ((EfiEntry->PhysicalStart + MemoryBlockLength) < 0x10) {
   //
-  // Skip the memory block is under 1MB
+  // Skip the memory block if under 1MB
   //
 } else {
   if (EfiEntry->PhysicalStart < 0x10) {
@@ -1926,7 +1926,7 @@ LegacyBiosBuildE820 (
   *Size = (UINTN) (Index * sizeof (EFI_E820_ENTRY64));
 
   //
-  // Determine OS usable memory above 1Mb
+  // Determine OS usable memory above 1MB
   //
   Private->IntThunk->EfiToLegacy16BootTable.OsMemoryAbove1Mb = 0x;
   for (TempIndex = Above1MIndex; TempIndex < Index; TempIndex++) {
@@ -1948,7 +1948,7 @@ LegacyBiosBuildE820 (
   // Print DEBUG information
   //
   for (TempIndex = 0; TempIndex < Index; TempIndex++) {
-DEBUG((EFI_D_INFO, "E820[%2d]: 0x%16lx  0x%16lx, Type = 0x%x \n",
+DEBUG((EFI_D_INFO, "E820[%2d]: 0x%016lx - 0x%016lx, Type = %d\n",
   TempIndex,
   E820Table[TempIndex].BaseAddr,
   (E820Table[TempIndex].BaseAddr + E820Table[TempIndex].Length),
-- 
2.21.0


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

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