Re: [edk2-devel] [PATCH] Emulator/X86EmulatorDxe: Replace with MultiArchUefiPkg build
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
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
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?)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
On 1/18/2024 12:26 PM, Oliver Smith-Denny wrote: Does this solve your issue? I have to run to a meeting, but I can write this in actual patch form (and give it a quick test) later. Unfortunately that didn't work: I still get the assert. ... SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table ASSERT_EFI_ERROR (Status = Invalid Parameter) ASSERT [SmbiosDxe] /local-data/src/ampereone/edk2/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c(145): !(((INTN)(RETURN_STATUS)(Status)) < 0) -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114090): https://edk2.groups.io/g/devel/message/114090 Mute This Topic: https://groups.io/mt/103810212/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements
Fixes and improvements to GenericWatchdogDxe. PR: https://github.com/tianocore/edk2/pull/5176 Changes between v3 and v4: - Check Interface Identification Register for architecture revision before setting the high offset register value. - Use @par for reference. - Move setting of EBS flag from patch 2/3 to 3/3. - Disable the watchdog in the case that EBS has been called and the timer period is non-zero. Rebecca Cran (3): ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset ArmPkg: Introduce global mTimerPeriod and remove calculation ArmPkg: Disable watchdog interaction after exiting boot services ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 11 +++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 69 +--- 2 files changed, 54 insertions(+), 26 deletions(-) -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114086): https://edk2.groups.io/g/devel/message/114086 Mute This Topic: https://groups.io/mt/103832317/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation
The calculation of the timer period was broken. Introduce a global mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz is only used in one place, remove the global and make it a local variable. Do the same with mNumTimerTicks. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 35 +--- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 20549aa91d94..8dd247c44e8f 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -28,13 +28,10 @@ in a second */ #define TIME_UNITS_PER_SECOND 1000 -// Tick frequency of the generic timer basis of the generic watchdog. -STATIC UINTN mTimerFrequencyHz = 0; - /* In cases where the compare register was set manually, information about how long the watchdog was asked to wait cannot be retrieved from hardware. It is therefore stored here. 0 means the timer is not running. */ -STATIC UINT64 mNumTimerTicks = 0; +STATIC UINT64 mTimerPeriod = 0; STATIC UINT8 WatchdogRevision; @@ -95,7 +92,7 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mTimerPeriod= 0; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -110,7 +107,6 @@ WatchdogInterruptHandler ( ) { STATIC CONST CHAR16 ResetString[] = L"The generic watchdog timer ran out."; - UINT64 TimerPeriod; WatchdogDisable (); @@ -123,8 +119,7 @@ WatchdogInterruptHandler ( // the timer period plus 1. // if (mWatchdogNotify != NULL) { -TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); -mWatchdogNotify (TimerPeriod + 1); +mWatchdogNotify (mTimerPeriod + 1); } gRT->ResetSystem ( @@ -204,22 +199,27 @@ WatchdogSetTimerPeriod ( IN UINT64TimerPeriod // In 100ns units ) { - UINTN SystemCount; + UINTN SystemCount; + UINT64 TimerFrequencyHz; + UINT64 NumTimerTicks; // if TimerPeriod is 0, this is a request to stop the watchdog. if (TimerPeriod == 0) { -mNumTimerTicks = 0; +mTimerPeriod = 0; WatchdogDisable (); return EFI_SUCCESS; } // Work out how many timer ticks will equate to TimerPeriod - mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; + TimerFrequencyHz = ArmGenericTimerGetTimerFreq (); + ASSERT (TimerFrequencyHz != 0); + mTimerPeriod = TimerPeriod; + NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT48) { + if (NumTimerTicks > MAX_UINT48) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't @@ -227,9 +227,9 @@ WatchdogSetTimerPeriod ( WatchdogWriteOffsetRegister (MAX_UINT48); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); -WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); +WatchdogWriteCompareRegister (SystemCount + NumTimerTicks); } else { -WatchdogWriteOffsetRegister (mNumTimerTicks); +WatchdogWriteOffsetRegister (NumTimerTicks); WatchdogEnable (); } @@ -264,7 +264,7 @@ WatchdogGetTimerPeriod ( return EFI_INVALID_PARAMETER; } - *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); + *TimerPeriod = mTimerPeriod; return EFI_SUCCESS; } @@ -332,9 +332,6 @@ GenericWatchdogEntry ( This will avoid conflicts with the universal watchdog */ ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &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
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
Update GenericWatchdogDxe to disable watchdog interaction after exiting boot services. Also, move the mEfiExitBootServicesEvent event to the top of the file with the other static variables. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 8dd247c44e8f..189194a2b9ee 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -35,10 +35,14 @@ STATIC UINT64 mTimerPeriod = 0; STATIC UINT8 WatchdogRevision; +/* disables watchdog interaction after Exit Boot Services */ +STATIC BOOLEAN mExitedBootServices = FALSE; + #define MAX_UINT48 0xULL STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC EFI_EVENT mEfiExitBootServicesEvent; STATIC VOID @@ -93,6 +97,7 @@ WatchdogExitBootServicesEvent ( { WatchdogDisable (); mTimerPeriod= 0; + mExitedBootServices = TRUE; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -203,7 +208,15 @@ WatchdogSetTimerPeriod ( UINT64 TimerFrequencyHz; UINT64 NumTimerTicks; - // if TimerPeriod is 0, this is a request to stop the watchdog. + // If we've exited Boot Services but TimerPeriod isn't zero, this + // indicates that the caller is doing something wrong. + if (mExitedBootServices && (TimerPeriod != 0)) { +mTimerPeriod = 0; +WatchdogDisable (); +return EFI_DEVICE_ERROR; + } + + // If TimerPeriod is 0 this is a request to stop the watchdog. if (TimerPeriod == 0) { mTimerPeriod = 0; WatchdogDisable (); @@ -307,8 +320,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { WatchdogGetTimerPeriod }; -STATIC EFI_EVENT mEfiExitBootServicesEvent; - EFI_STATUS EFIAPI GenericWatchdogEntry ( -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114089): https://edk2.groups.io/g/devel/message/114089 Mute This Topic: https://groups.io/mt/103832320/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation
On 1/5/2024 1:26 AM, Ard Biesheuvel wrote: @@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mTimerPeriod= 0; + mExitedBootServices = TRUE; Where is this declared/defined? Oh, it's defined in the 3rd patch - which obviously breaks bisect. I'll fix it. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114019): https://edk2.groups.io/g/devel/message/114019 Mute This Topic: https://groups.io/mt/103538116/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 3/3] ArmPkg: Disable watchdog interaction after exiting boot services
On 1/5/2024 4:12 AM, Sami Mujawar wrote: - // if TimerPeriod is 0, this is a request to stop the watchdog. + // If we've exited Boot Services but TimerPeriod isn't zero, this + // indicates that the caller is doing something wrong. + if (mExitedBootServices && (TimerPeriod != 0)) { [SAMI] Thanks for updating the code to return the error code. However, I see you are not stopping the watchdog timer. Is this because you expect the watchdog period to expire and reset the system? I removed the code to stop the watchdog timer because it's an error condition. However, I've updated it so it does also get stopped in this case too. Also, did you see an issue that motivated this patch, or this was just a case of hardening the code? Can you provide more information, please? [/SAMI] This was issues found and improvements made by various people in the last couple of years that we're now upstreaming to contribute improvements and reduce our diffs against upstream. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114018): https://edk2.groups.io/g/devel/message/114018 Mute This Topic: https://groups.io/mt/103538118/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned
On 1/18/2024 12:04 PM, Oliver Smith-Denny wrote: On 1/18/2024 10:45 AM, Rebecca Cran via groups.io wrote: No, I mean SbsaQemu from edk2-platforms: https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu Sure, if you can repro there that is helpful. I've realized it won't repro there or other virtual platforms because it requires that SmbiosDxe run the reallocation code, which I believe is only done on systems with a larger number of SMBIOS tables - for example where RAS etc. is supported. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114007): https://edk2.groups.io/g/devel/message/114007 Mute This Topic: https://groups.io/mt/103810212/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned
On 1/18/2024 11:38 AM, Oliver Smith-Denny wrote: Yeah, if you can get it running there, that would be a good data point. I assume you mean the Project Mu QemuSbsaPkg? If so that is great, but you will need to update the RUNTIME_PAGE_ALLOCATION_GRANULARITY back to 0x1. It was set to 0x1000 for a historical issue that we are working on reconciling with what edk2 has. No, I mean SbsaQemu from edk2-platforms: https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu To clarify, if you turn off pool guard, does the assert go away? Yes. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114004): https://edk2.groups.io/g/devel/message/114004 Mute This Topic: https://groups.io/mt/103810212/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned
On 1/18/2024 9:48 AM, Oliver Smith-Denny via groups.io wrote: Are you including this commit: https://github.com/tianocore/edk2/commit/00b51e0d78a547dd78119ec44fcc74a01b6f79c8? Can you share some more details on where this is failing? I.e. what assert is getting tripped? Presumably without HeapGuard enabled, you aren't seeing the failure? Are you hitting this case: https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Core/Dxe/Mem/Page.c#L1570-L1573? Does this repro on ArmVirtPkg? Yes, I have that commit in my tree. I'm hitting this assert in FreePages: https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Library/DxeCoreMemoryAllocationLib/MemoryAllocationLib.c#L190 It's called by SmbiosCreate64BitTable: https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c#L1342 And yes, that's the case I'm hitting. I'm having trouble getting ArmVirtPkg to run. Would it be useful testing using SbsaQemu instead? -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114002): https://edk2.groups.io/g/devel/message/114002 Mute This Topic: https://groups.io/mt/103810212/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned
I've been debugging an assert failure when using HeapGuard on AArch64. A call to FreePages in SmbiosDxe is failing because the memory is aligned to 0x1000 instead of 0x1 as defined by RUNTIME_PAGE_ALLOCATION_GRANULARITY. I'm enabling HeapGuard by setting the PCDs to the following values: gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC000 gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC000 -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113995): https://edk2.groups.io/g/devel/message/113995 Mute This Topic: https://groups.io/mt/103810212/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements
On 1/5/2024 1:26 AM, Ard Biesheuvel via groups.io wrote: On Fri, 5 Jan 2024 at 06:15, Rebecca Cran wrote: Fixes and improvements to GenericWatchdogDxe. What is the difference between v2 and v3? Sorry, I should have said that. I forgot to build v2 and it had a bug in the exit boot services handler where I'd forgotten to update a line. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113298): https://edk2.groups.io/g/devel/message/113298 Mute This Topic: https://groups.io/mt/103538117/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
The generic watchdog offset register is 48 bits wide, and can be set by performing two 32-bit writes. Add support for writing the high 16 bits of the offset register and update the signature of the WatchdogWriteOffsetRegister function to take a UINT64 value. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 6 +- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h index 9bc3bf47047c..2a0634e7e9f1 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h @@ -1,9 +1,12 @@ /** @file * +* Copyright (c) 2023, Ampere Computing LLC. All rights reserved. * Copyright (c) 2013-2017, ARM Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent * +* See Generic Watchdog specification in Arm Base System Architecture 1.0C: +* https://developer.arm.com/documentation/den0094/c/ **/ #ifndef GENERIC_WATCHDOG_H_ @@ -14,7 +17,8 @@ // Control Frame: #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x000) -#define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x00C) #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x014) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 66c6c37c08b0..f8c39458a53a 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -1,5 +1,6 @@ /** @file * +* Copyright (c) 2023, Ampere Computing LLC. All rights reserved. * Copyright (c) 2013-2018, ARM Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent @@ -35,16 +36,19 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +#define MAX_UINT48 0xULL + STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; STATIC VOID WatchdogWriteOffsetRegister ( - UINT32 Value + UINT64 Value ) { - MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value); + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32); + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT16); } STATIC @@ -211,17 +215,17 @@ WatchdogSetTimerPeriod ( /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT32) { + if (mNumTimerTicks > MAX_UINT48) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't trigger an interrupt, set the offset to max. */ -WatchdogWriteOffsetRegister (MAX_UINT32); +WatchdogWriteOffsetRegister (MAX_UINT48); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); } else { -WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks); +WatchdogWriteOffsetRegister (mNumTimerTicks); WatchdogEnable (); } -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113215): https://edk2.groups.io/g/devel/message/113215 Mute This Topic: https://groups.io/mt/103538115/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation
The calculation of the timer period was broken. Introduce a global mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz is only used in one place, remove the global and make it a local variable. Do the same with mNumTimerTicks. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 36 ++ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index f8c39458a53a..78cee62a19d6 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -28,13 +28,10 @@ in a second */ #define TIME_UNITS_PER_SECOND 1000 -// Tick frequency of the generic timer basis of the generic watchdog. -STATIC UINTN mTimerFrequencyHz = 0; - /* In cases where the compare register was set manually, information about how long the watchdog was asked to wait cannot be retrieved from hardware. It is therefore stored here. 0 means the timer is not running. */ -STATIC UINT64 mNumTimerTicks = 0; +STATIC UINT64 mTimerPeriod = 0; #define MAX_UINT48 0xULL @@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mTimerPeriod= 0; + mExitedBootServices = TRUE; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -106,7 +104,6 @@ WatchdogInterruptHandler ( ) { STATIC CONST CHAR16 ResetString[] = L"The generic watchdog timer ran out."; - UINT64 TimerPeriod; WatchdogDisable (); @@ -119,8 +116,7 @@ WatchdogInterruptHandler ( // the timer period plus 1. // if (mWatchdogNotify != NULL) { -TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); -mWatchdogNotify (TimerPeriod + 1); +mWatchdogNotify (mTimerPeriod + 1); } gRT->ResetSystem ( @@ -200,22 +196,27 @@ WatchdogSetTimerPeriod ( IN UINT64TimerPeriod // In 100ns units ) { - UINTN SystemCount; + UINTN SystemCount; + UINT64 TimerFrequencyHz; + UINT64 NumTimerTicks; // if TimerPeriod is 0, this is a request to stop the watchdog. if (TimerPeriod == 0) { -mNumTimerTicks = 0; +mTimerPeriod = 0; WatchdogDisable (); return EFI_SUCCESS; } // Work out how many timer ticks will equate to TimerPeriod - mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; + TimerFrequencyHz = ArmGenericTimerGetTimerFreq (); + ASSERT (TimerFrequencyHz != 0); + mTimerPeriod = TimerPeriod; + NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT48) { + if (NumTimerTicks > MAX_UINT48) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't @@ -223,9 +224,9 @@ WatchdogSetTimerPeriod ( WatchdogWriteOffsetRegister (MAX_UINT48); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); -WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); +WatchdogWriteCompareRegister (SystemCount + NumTimerTicks); } else { -WatchdogWriteOffsetRegister (mNumTimerTicks); +WatchdogWriteOffsetRegister (NumTimerTicks); WatchdogEnable (); } @@ -260,7 +261,7 @@ WatchdogGetTimerPeriod ( return EFI_INVALID_PARAMETER; } - *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); + *TimerPeriod = mTimerPeriod; return EFI_SUCCESS; } @@ -327,9 +328,6 @@ GenericWatchdogEntry ( This will avoid conflicts with the universal watchdog */ ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &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
Fixes and improvements to GenericWatchdogDxe. PR: https://github.com/tianocore/edk2/pull/5176 Rebecca Cran (3): ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset ArmPkg: Introduce global mTimerPeriod and remove calculation ArmPkg: Disable watchdog interaction after exiting boot services ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 6 ++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 60 +- 2 files changed, 40 insertions(+), 26 deletions(-) -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113217): https://edk2.groups.io/g/devel/message/113217 Mute This Topic: https://groups.io/mt/103538117/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3 3/3] ArmPkg: Disable watchdog interaction after exiting boot services
Update GenericWatchdogDxe to disable watchdog interaction after exiting boot services. Also, move the mEfiExitBootServicesEvent event to the top of the file with the other static variables. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 78cee62a19d6..ddf131660f9d 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -33,10 +33,14 @@ It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mTimerPeriod = 0; +/* disables watchdog interaction after Exit Boot Services */ +STATIC BOOLEAN mExitedBootServices = FALSE; + #define MAX_UINT48 0xULL STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC EFI_EVENT mEfiExitBootServicesEvent; STATIC VOID @@ -200,7 +204,13 @@ WatchdogSetTimerPeriod ( UINT64 TimerFrequencyHz; UINT64 NumTimerTicks; - // if TimerPeriod is 0, this is a request to stop the watchdog. + // If we've exited Boot Services but TimerPeriod isn't zero, this + // indicates that the caller is doing something wrong. + if (mExitedBootServices && (TimerPeriod != 0)) { +return EFI_DEVICE_ERROR; + } + + // If TimerPeriod is 0 this is a request to stop the watchdog. if (TimerPeriod == 0) { mTimerPeriod = 0; WatchdogDisable (); @@ -304,8 +314,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { WatchdogGetTimerPeriod }; -STATIC EFI_EVENT mEfiExitBootServicesEvent; - EFI_STATUS EFIAPI GenericWatchdogEntry ( -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113218): https://edk2.groups.io/g/devel/message/113218 Mute This Topic: https://groups.io/mt/103538118/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/3] ArmPkg: Fix the calculation of the timer period in GenericWatchdogDxe
Thanks, I've incorporated the changes into the v2 patch series. -- Rebecca On 1/3/2024 3:56 PM, Ard Biesheuvel wrote: Hi Rebecca, On Wed, 3 Jan 2024 at 21:44, Rebecca Cran wrote: Fix the calculation of the timer period in GenericWatchdogDxe: we need to multiply before dividing to keep the values as integers. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 05df101d5f4b..8f02f38c64e3 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -119,7 +119,7 @@ WatchdogInterruptHandler ( // the timer period plus 1. // if (mWatchdogNotify != NULL) { -TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); +TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / mTimerFrequencyHz); Could we just store the timer period in a global mTimerPeriod, and get rid of mNumTimerTicks entirely? AFAICT, that would get rid of these calculations as well. mWatchdogNotify (TimerPeriod + 1); } @@ -260,7 +260,7 @@ WatchdogGetTimerPeriod ( return EFI_INVALID_PARAMETER; } - *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); + *TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / mTimerFrequencyHz); return EFI_SUCCESS; } -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113214): https://edk2.groups.io/g/devel/message/113214 Mute This Topic: https://groups.io/mt/103510102/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 3/3] ArmPkg: Disable watchdog interaction after exiting boot services
Thanks, I've incorporated the changes into the v2 patch series. -- Rebecca On 1/4/2024 3:01 AM, Sami Mujawar wrote: Hi Rebecca, Thank you for this patch. I have some minor suggestions marked inline as [SAMI]. Regards, Sami Mujawar On 03/01/2024, 20:44, "Rebecca Cran" mailto:rebe...@os.amperecomputing.com>> wrote: Update GenericWatchdogDxe to disable watchdog interaction after exiting boot services. Also, move the mEfiExitBootServicesEvent event to the top of the file with the other static variables. Signed-off-by: Rebecca Cran mailto:rebe...@os.amperecomputing.com>> --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 8f02f38c64e3..912106eb6ad2 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -36,10 +36,14 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +/* disables watchdog interaction after Exit Boot Services */ +STATIC BOOLEAN mExitedBootServices = FALSE; + #define MAX_UINT48 0xULL STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC EFI_EVENT mEfiExitBootServicesEvent; STATIC VOID @@ -91,7 +95,8 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mNumTimerTicks = 0; + mExitedBootServices = TRUE; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -202,8 +207,9 @@ WatchdogSetTimerPeriod ( { UINTN SystemCount; - // if TimerPeriod is 0, this is a request to stop the watchdog. - if (TimerPeriod == 0) { + // if TimerPeriod is 0 or we've exited boot services, + // this is a request to stop the watchdog. + if (TimerPeriod == 0 || mExitedBootServices) { [SAMI] I think we need extra brackets here. Also, should EFI_DEVICE_ERROR be returned if ExitBootServices is signalled but TimerPeriod != 0? i.e. we stop the watchdog but return an error to the caller so that it knows to fix the broken code. [/SAMI] mNumTimerTicks = 0; WatchdogDisable (); return EFI_SUCCESS; @@ -303,8 +309,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { WatchdogGetTimerPeriod }; -STATIC EFI_EVENT mEfiExitBootServicesEvent; - EFI_STATUS EFIAPI GenericWatchdogEntry ( -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113213): https://edk2.groups.io/g/devel/message/113213 Mute This Topic: https://groups.io/mt/103510105/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 2/3] ArmPkg: Introduce global mTimerPeriod and remove calculation
The calculation of the timer period was broken. Introduce a global mTimerPeriod so the calculation can be removed. Since mTimerFrequencyHz is only used in one place, remove the global and make it a local variable. Do the same with mNumTimerTicks. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 36 ++ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index f8c39458a53a..f74d0d1ced2e 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -28,13 +28,10 @@ in a second */ #define TIME_UNITS_PER_SECOND 1000 -// Tick frequency of the generic timer basis of the generic watchdog. -STATIC UINTN mTimerFrequencyHz = 0; - /* In cases where the compare register was set manually, information about how long the watchdog was asked to wait cannot be retrieved from hardware. It is therefore stored here. 0 means the timer is not running. */ -STATIC UINT64 mNumTimerTicks = 0; +STATIC UINT64 mTimerPeriod = 0; #define MAX_UINT48 0xULL @@ -91,7 +88,8 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mNumTimerTicks = 0; + mExitedBootServices = TRUE; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -106,7 +104,6 @@ WatchdogInterruptHandler ( ) { STATIC CONST CHAR16 ResetString[] = L"The generic watchdog timer ran out."; - UINT64 TimerPeriod; WatchdogDisable (); @@ -119,8 +116,7 @@ WatchdogInterruptHandler ( // the timer period plus 1. // if (mWatchdogNotify != NULL) { -TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); -mWatchdogNotify (TimerPeriod + 1); +mWatchdogNotify (mTimerPeriod + 1); } gRT->ResetSystem ( @@ -200,22 +196,27 @@ WatchdogSetTimerPeriod ( IN UINT64TimerPeriod // In 100ns units ) { - UINTN SystemCount; + UINTN SystemCount; + UINT64 TimerFrequencyHz; + UINT64 NumTimerTicks; // if TimerPeriod is 0, this is a request to stop the watchdog. if (TimerPeriod == 0) { -mNumTimerTicks = 0; +mTimerPeriod = 0; WatchdogDisable (); return EFI_SUCCESS; } // Work out how many timer ticks will equate to TimerPeriod - mNumTimerTicks = (mTimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; + TimerFrequencyHz = ArmGenericTimerGetTimerFreq (); + ASSERT (TimerFrequencyHz != 0); + mTimerPeriod = TimerPeriod; + NumTimerTicks = (TimerFrequencyHz * TimerPeriod) / TIME_UNITS_PER_SECOND; /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT48) { + if (NumTimerTicks > MAX_UINT48) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't @@ -223,9 +224,9 @@ WatchdogSetTimerPeriod ( WatchdogWriteOffsetRegister (MAX_UINT48); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); -WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); +WatchdogWriteCompareRegister (SystemCount + NumTimerTicks); } else { -WatchdogWriteOffsetRegister (mNumTimerTicks); +WatchdogWriteOffsetRegister (NumTimerTicks); WatchdogEnable (); } @@ -260,7 +261,7 @@ WatchdogGetTimerPeriod ( return EFI_INVALID_PARAMETER; } - *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); + *TimerPeriod = mTimerPeriod; return EFI_SUCCESS; } @@ -327,9 +328,6 @@ GenericWatchdogEntry ( This will avoid conflicts with the universal watchdog */ ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &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
Update GenericWatchdogDxe to disable watchdog interaction after exiting boot services. Also, move the mEfiExitBootServicesEvent event to the top of the file with the other static variables. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index f74d0d1ced2e..44d812193031 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -33,10 +33,14 @@ It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mTimerPeriod = 0; +/* disables watchdog interaction after Exit Boot Services */ +STATIC BOOLEAN mExitedBootServices = FALSE; + #define MAX_UINT48 0xULL STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC EFI_EVENT mEfiExitBootServicesEvent; STATIC VOID @@ -200,7 +204,13 @@ WatchdogSetTimerPeriod ( UINT64 TimerFrequencyHz; UINT64 NumTimerTicks; - // if TimerPeriod is 0, this is a request to stop the watchdog. + // If we've exited Boot Services but TimerPeriod isn't zero, this + // indicates that the caller is doing something wrong. + if (mExitedBootServices && (TimerPeriod != 0)) { +return EFI_DEVICE_ERROR; + } + + // If TimerPeriod is 0 this is a request to stop the watchdog. if (TimerPeriod == 0) { mTimerPeriod = 0; WatchdogDisable (); @@ -304,8 +314,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { WatchdogGetTimerPeriod }; -STATIC EFI_EVENT mEfiExitBootServicesEvent; - EFI_STATUS EFIAPI GenericWatchdogEntry ( -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113212): https://edk2.groups.io/g/devel/message/113212 Mute This Topic: https://groups.io/mt/103537820/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements
Fixes and improvements to GenericWatchdogDxe. PR: https://github.com/tianocore/edk2/pull/5176 Rebecca Cran (3): ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset ArmPkg: Introduce global mTimerPeriod and remove calculation ArmPkg: Disable watchdog interaction after exiting boot services ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 6 ++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 60 +- 2 files changed, 40 insertions(+), 26 deletions(-) -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113209): https://edk2.groups.io/g/devel/message/113209 Mute This Topic: https://groups.io/mt/103537817/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
The generic watchdog offset register is 48 bits wide, and can be set by performing two 32-bit writes. Add support for writing the high 16 bits of the offset register and update the signature of the WatchdogWriteOffsetRegister function to take a UINT64 value. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 6 +- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h index 9bc3bf47047c..2a0634e7e9f1 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h @@ -1,9 +1,12 @@ /** @file * +* Copyright (c) 2023, Ampere Computing LLC. All rights reserved. * Copyright (c) 2013-2017, ARM Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent * +* See Generic Watchdog specification in Arm Base System Architecture 1.0C: +* https://developer.arm.com/documentation/den0094/c/ **/ #ifndef GENERIC_WATCHDOG_H_ @@ -14,7 +17,8 @@ // Control Frame: #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x000) -#define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x00C) #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x014) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 66c6c37c08b0..f8c39458a53a 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -1,5 +1,6 @@ /** @file * +* Copyright (c) 2023, Ampere Computing LLC. All rights reserved. * Copyright (c) 2013-2018, ARM Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent @@ -35,16 +36,19 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +#define MAX_UINT48 0xULL + STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; STATIC VOID WatchdogWriteOffsetRegister ( - UINT32 Value + UINT64 Value ) { - MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value); + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32); + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT16); } STATIC @@ -211,17 +215,17 @@ WatchdogSetTimerPeriod ( /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT32) { + if (mNumTimerTicks > MAX_UINT48) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't trigger an interrupt, set the offset to max. */ -WatchdogWriteOffsetRegister (MAX_UINT32); +WatchdogWriteOffsetRegister (MAX_UINT48); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); } else { -WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks); +WatchdogWriteOffsetRegister (mNumTimerTicks); WatchdogEnable (); } -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113210): https://edk2.groups.io/g/devel/message/113210 Mute This Topic: https://groups.io/mt/103537818/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Updates to .mailmap needed for Jeff Brasen, Jake Garver, Joey Vagades and Michael Roth?
I noticed recent commits by Jeff Brasen, Jake Garver, Joey Vagades and Michael Roth have funky Author lines, which I think means .mailmap needs updated? commit 7a5823f85be99b9a92751fcf4141f7982fa5cc80 Author: Jeff Brasen via groups.io Date: Thu Dec 28 12:47:08 2023 -0800 EmbeddedPkg: Add DtPlatformLoaderLib gmock support Add Google Mock Library for DtPlatformLoaderDtbLib Signed-off-by: Jeff Brasen commit 9f0061a03b61d282fbc0ba5be22155d06a5e64a1 Author: Joey Vagedes via groups.io Date: Wed Dec 6 12:27:02 2023 -0800 BaseTools: Resolve regex syntax warnings Switches regex patterns to raw text to resolve python 3.12 syntax warnings in regards to invalid escape sequences, as is suggested by the re (regex) module in python. Cc: Rebecca Cran Cc: Liming Gao Cc: Bob Feng Cc: Yuwei Chen Signed-off-by: Joey Vagedes Reviewed-by: Rebecca Cran Author: Jake Garver via groups.io Date: Tue Oct 3 23:04:54 2023 +0800 MdeModulePkg/RegularExpressinoDxe: Fix clang error Ignore old style declaration warnings in oniguruma/src/st.c. This was already ignored for MSFT, but newer versions of clang complain as well. Signed-off-by: Jake Garver Reviewed-by: Nhi Pham Tested-by: Nhi Pham Reviewed-by: Liming Gao commit f008890ae55929f7f17e7d2f8aff929255007d33 Author: Roth, Michael via groups.io Date: Wed Aug 16 15:11:45 2023 -0500 OvmfPkg/AmdSev: fix BdsPlatform.c assertion failure during boot Booting an SEV guest with AmdSev OVMF package currently triggers the following assertion with QEMU: InstallQemuFwCfgTables: installed 7 tables PcRtc: Write 0x20 to CMOS location 0x32 [Variable]END_OF_DXE is signaled Initialize variable error flag (FF) ASSERT_EFI_ERROR (Status = Not Found) ASSERT [BdsDxe] /home/VT_BUILD/ovmf/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c(1711): !(((INTN)(RETURN_STATUS)(Status)) < 0) This seems to be due to commit 81dc0d8b4c, which switched to using PlatformBootManagerLib instead of PlatformBootManagerLibGrub. That pulls in a dependency on gEfiS3SaveStateProtocolGuid provider being available (which is asserted for in BdsPlatform.c:PlatformBootManagerBeforeConsole()/SaveS3BootScript()), but the libraries that provide it aren't currently included in the build. Add them similarly to what's done for OvmfPkg. Fixes: 81dc0d8b4c ("OvmfPkg/AmdSev: stop using PlatformBootManagerLibGrub") Signed-off-by: Michael Roth Acked-by: Gerd Hoffmann Acked-by: Jiewen Yao -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113201): https://edk2.groups.io/g/devel/message/113201 Mute This Topic: https://groups.io/mt/103534194/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms][PATCH v1 2/7] Platform/Sgi: add no-stack-protector flag for StMM builds
Wouldn't it be better to add the Stack Protector library (MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf) instead of disabling it? -- Rebecca Cran On 1/4/2024 11:49 AM, Prabin CA via groups.io wrote: Add the no-stack-protector compiler flag to allow StandaloneMM builds on both AArch64 and x86 host. Without this flag, the link stage fails with the following errors on multiple files when built with gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0: undefined reference to `__stack_chk_guard' undefined reference to `__stack_chk_fail' Signed-off-by: Vijayenthiran Subramaniam Signed-off-by: Prabin CA --- Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc index ab54b3b25f4c..2a8c678c0816 100644 --- a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc +++ b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc @@ -145,5 +145,5 @@ [Components.AARCH64] # ### [BuildOptions.AARCH64] - GCC:*_*_*_CC_FLAGS = -mstrict-align -march=armv8-a -D DISABLE_NEW_DEPRECATED_INTERFACES + GCC:*_*_*_CC_FLAGS = -mstrict-align -march=armv8-a -fno-stack-protector -D DISABLE_NEW_DEPRECATED_INTERFACES GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113199): https://edk2.groups.io/g/devel/message/113199 Mute This Topic: https://groups.io/mt/103528421/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/3] ArmPkg: Fix the calculation of the timer period in GenericWatchdogDxe
Fix the calculation of the timer period in GenericWatchdogDxe: we need to multiply before dividing to keep the values as integers. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 05df101d5f4b..8f02f38c64e3 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -119,7 +119,7 @@ WatchdogInterruptHandler ( // the timer period plus 1. // if (mWatchdogNotify != NULL) { -TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); +TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / mTimerFrequencyHz); mWatchdogNotify (TimerPeriod + 1); } @@ -260,7 +260,7 @@ WatchdogGetTimerPeriod ( return EFI_INVALID_PARAMETER; } - *TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * mNumTimerTicks); + *TimerPeriod = ((TIME_UNITS_PER_SECOND * mNumTimerTicks) / mTimerFrequencyHz); return EFI_SUCCESS; } -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113105): https://edk2.groups.io/g/devel/message/113105 Mute This Topic: https://groups.io/mt/103510102/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/3] ArmPkg: Disable watchdog interaction after exiting boot services
Update GenericWatchdogDxe to disable watchdog interaction after exiting boot services. Also, move the mEfiExitBootServicesEvent event to the top of the file with the other static variables. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 8f02f38c64e3..912106eb6ad2 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -36,10 +36,14 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +/* disables watchdog interaction after Exit Boot Services */ +STATIC BOOLEAN mExitedBootServices = FALSE; + #define MAX_UINT48 0xULL STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; +STATIC EFI_EVENT mEfiExitBootServicesEvent; STATIC VOID @@ -91,7 +95,8 @@ WatchdogExitBootServicesEvent ( ) { WatchdogDisable (); - mNumTimerTicks = 0; + mNumTimerTicks = 0; + mExitedBootServices = TRUE; } /* This function is called when the watchdog's first signal (WS0) goes high. @@ -202,8 +207,9 @@ WatchdogSetTimerPeriod ( { UINTN SystemCount; - // if TimerPeriod is 0, this is a request to stop the watchdog. - if (TimerPeriod == 0) { + // if TimerPeriod is 0 or we've exited boot services, + // this is a request to stop the watchdog. + if (TimerPeriod == 0 || mExitedBootServices) { mNumTimerTicks = 0; WatchdogDisable (); return EFI_SUCCESS; @@ -303,8 +309,6 @@ STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = { WatchdogGetTimerPeriod }; -STATIC EFI_EVENT mEfiExitBootServicesEvent; - EFI_STATUS EFIAPI GenericWatchdogEntry ( -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113107): https://edk2.groups.io/g/devel/message/113107 Mute This Topic: https://groups.io/mt/103510105/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/3] ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset
The generic watchdog offset register is 48 bits wide, and can be set by performing two 32-bit writes. Add support for writing the high 16 bits of the offset register and update the signature of the WatchdogWriteOffsetRegister function to take a UINT64 value. Signed-off-by: Rebecca Cran --- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 4 +++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 14 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h index 9bc3bf47047c..504bc4cacb33 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h @@ -1,5 +1,6 @@ /** @file * +* Copyright (c) 2023, Ampere Computing LLC. All rights reserved. * Copyright (c) 2013-2017, ARM Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent @@ -14,7 +15,8 @@ // Control Frame: #define GENERIC_WDOG_CONTROL_STATUS_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x000) -#define GENERIC_WDOG_OFFSET_REG ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x008) +#define GENERIC_WDOG_OFFSET_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x00C) #define GENERIC_WDOG_COMPARE_VALUE_REG_LOW ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x010) #define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH ((UINTN)FixedPcdGet64 (PcdGenericWatchdogControlBase) + 0x014) diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c index 66c6c37c08b0..05df101d5f4b 100644 --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c @@ -1,5 +1,6 @@ /** @file * +* Copyright (c) 2023, Ampere Computing LLC. All rights reserved. * Copyright (c) 2013-2018, ARM Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent @@ -35,16 +36,19 @@ STATIC UINTN mTimerFrequencyHz = 0; It is therefore stored here. 0 means the timer is not running. */ STATIC UINT64 mNumTimerTicks = 0; +#define MAX_UINT48 0xULL + STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol; STATIC EFI_WATCHDOG_TIMER_NOTIFY mWatchdogNotify; STATIC VOID WatchdogWriteOffsetRegister ( - UINT32 Value + UINT64 Value ) { - MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value); + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_LOW, Value & MAX_UINT32); + MmioWrite32 (GENERIC_WDOG_OFFSET_REG_HIGH, (Value >> 32) & MAX_UINT32); } STATIC @@ -211,17 +215,17 @@ WatchdogSetTimerPeriod ( /* If the number of required ticks is greater than the max the watchdog's offset register (WOR) can hold, we need to manually compute and set the compare register (WCV) */ - if (mNumTimerTicks > MAX_UINT32) { + if (mNumTimerTicks > MAX_UINT48) { /* We need to enable the watchdog *before* writing to the compare register, because enabling the watchdog causes an "explicit refresh", which clobbers the compare register (WCV). In order to make sure this doesn't trigger an interrupt, set the offset to max. */ -WatchdogWriteOffsetRegister (MAX_UINT32); +WatchdogWriteOffsetRegister (MAX_UINT48); WatchdogEnable (); SystemCount = ArmGenericTimerGetSystemCount (); WatchdogWriteCompareRegister (SystemCount + mNumTimerTicks); } else { -WatchdogWriteOffsetRegister ((UINT32)mNumTimerTicks); +WatchdogWriteOffsetRegister (mNumTimerTicks); WatchdogEnable (); } -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113106): https://edk2.groups.io/g/devel/message/113106 Mute This Topic: https://groups.io/mt/103510103/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/3] ArmPkg: GenericWatchdogDxe fixes and improvements
Fixes and improvements to GenericWatchdogDxe. PR: https://github.com/tianocore/edk2/pull/5176 Rebecca Cran (3): ArmPkg: Update GenericWatchdogDxe to allow setting full 48-bit offset ArmPkg: Fix the calculation of the timer period in GenericWatchdogDxe ArmPkg: Disable watchdog interaction after exiting boot services ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 4 +++- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 32 +++--- 2 files changed, 23 insertions(+), 13 deletions(-) -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113104): https://edk2.groups.io/g/devel/message/113104 Mute This Topic: https://groups.io/mt/103510101/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 0/4] Add DEBUG_MANAGEABILITY to debug level comments
On 12/8/2023 5:19 PM, Rebecca Cran wrote: Add the new DEBUG_MANAGEABILITY debug level to MdePkg.dec and MdePkg.uni. Improve the wording of the description of the DEBUG_MANAGEABILITY level in DebugLib.h. Update the comment block in ArmVirtPkg.dsc.inc with the new list and updated formatting. PR: https://github.com/tianocore/edk2/pull/5127 -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112246): https://edk2.groups.io/g/devel/message/112246 Mute This Topic: https://groups.io/mt/103066384/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 4/4] ArmVirtPkg: Sync debug level comments in ArmVirt.dsc.inc
Update the debug level comments in ArmVirt.dsc.inc to sync with MdePkg/Include/Library/DebugLib.h. Signed-off-by: Rebecca Cran --- ArmVirtPkg/ArmVirt.dsc.inc | 42 ++-- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index 6bc72f1decbd..9b23ef97ec5f 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -315,26 +315,28 @@ [PcdsFixedAtBuild.common] gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f !endif - # DEBUG_INIT 0x0001 // Initialization - # DEBUG_WARN 0x0002 // Warnings - # DEBUG_LOAD 0x0004 // Load events - # DEBUG_FS0x0008 // EFI File system - # DEBUG_POOL 0x0010 // Alloc & Free (pool) - # DEBUG_PAGE 0x0020 // Alloc & Free (page) - # DEBUG_INFO 0x0040 // Informational debug messages - # DEBUG_DISPATCH 0x0080 // PEI/DXE/SMM Dispatchers - # DEBUG_VARIABLE 0x0100 // Variable - # DEBUG_BM0x0400 // Boot Manager - # DEBUG_BLKIO 0x1000 // BlkIo Driver - # DEBUG_NET 0x4000 // SNP Driver - # DEBUG_UNDI 0x0001 // UNDI Driver - # DEBUG_LOADFILE 0x0002 // LoadFile - # DEBUG_EVENT 0x0008 // Event messages - # DEBUG_GCD 0x0010 // Global Coherency Database changes - # DEBUG_CACHE 0x0020 // Memory range cachability changes - # DEBUG_VERBOSE 0x0040 // Detailed debug messages that may - # // significantly impact boot performance - # DEBUG_ERROR 0x8000 // Error + # DEBUG_INIT 0x0001 // Initialization + # DEBUG_WARN 0x0002 // Warnings + # DEBUG_LOAD 0x0004 // Load events + # DEBUG_FS 0x0008 // EFI File system + # DEBUG_POOL 0x0010 // Alloc & Free (pool) + # DEBUG_PAGE 0x0020 // Alloc & Free (page) + # DEBUG_INFO 0x0040 // Informational debug messages + # DEBUG_DISPATCH 0x0080 // PEI/DXE/SMM Dispatchers + # DEBUG_VARIABLE 0x0100 // Variable + # DEBUG_BM 0x0400 // Boot Manager + # DEBUG_BLKIO 0x1000 // BlkIo Driver + # DEBUG_NET0x4000 // Network Io Driver + # DEBUG_UNDI 0x0001 // UNDI Driver + # DEBUG_LOADFILE 0x0002 // LoadFile + # DEBUG_EVENT 0x0008 // Event messages + # DEBUG_GCD0x0010 // Global Coherency Database changes + # DEBUG_CACHE 0x0020 // Memory range cachability changes + # DEBUG_VERBOSE0x0040 // Detailed debug messages that may + # // significantly impact boot performance + # DEBUG_MANAGEABILITY 0x0080 // Detailed debug and payload manageability messages + # // related to modules such as Redfish, IPMI, MCTP etc. + # DEBUG_ERROR 0x8000 // Error !if $(TARGET) != RELEASE gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|$(DEBUG_PRINT_ERROR_LEVEL) !endif -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112245): https://edk2.groups.io/g/devel/message/112245 Mute This Topic: https://groups.io/mt/103066386/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/4] MdePkg: Add manageability debug level to PcdFixedDebugPrintErrorLevel
Update MdePkg.dec to add the manageability debug level to PcdFixedDebugPrintErrorLevel. Signed-off-by: Rebecca Cran --- MdePkg/MdePkg.dec | 1 + 1 file changed, 1 insertion(+) diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index ac54338089e8..bec63f5416e2 100644 --- a/MdePkg/MdePkg.dec +++ b/MdePkg/MdePkg.dec @@ -2213,6 +2213,7 @@ [PcdsFixedAtBuild] # BIT20 - Global Coherency Database changes message. # BIT21 - Memory range cachability changes message. # BIT22 - Detailed debug message. + # BIT23 - Manageability debug message. # BIT31 - Error message. # @Prompt Fixed Debug Message Print Level. gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel|0x|UINT32|0x30001016 -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112242): https://edk2.groups.io/g/devel/message/112242 Mute This Topic: https://groups.io/mt/103066383/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/4] Add DEBUG_MANAGEABILITY to debug level comments
Add the new DEBUG_MANAGEABILITY debug level to MdePkg.dec and MdePkg.uni. Improve the wording of the description of the DEBUG_MANAGEABILITY level in DebugLib.h. Update the comment block in ArmVirtPkg.dsc.inc with the new list and updated formatting. Rebecca Cran (4): MdePkg: Improve wording of manageability debug level comment MdePkg: Add manageability debug level to PcdFixedDebugPrintErrorLevel MdePkg: Update MdePkg.uni with manageability debug level ArmVirtPkg: Sync debug level comments in ArmVirt.dsc.inc MdePkg/MdePkg.dec | 1 + ArmVirtPkg/ArmVirt.dsc.inc| 42 ++-- MdePkg/Include/Library/DebugLib.h | 4 +- MdePkg/MdePkg.uni | 2 + 4 files changed, 27 insertions(+), 22 deletions(-) -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112243): https://edk2.groups.io/g/devel/message/112243 Mute This Topic: https://groups.io/mt/103066384/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/4] MdePkg: Update MdePkg.uni with manageability debug level
Update MdePkg.uni with the manageability debug level. Signed-off-by: Rebecca Cran --- MdePkg/MdePkg.uni | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni index 5c1fa24065c7..75dbc6ffe3c0 100644 --- a/MdePkg/MdePkg.uni +++ b/MdePkg/MdePkg.uni @@ -214,6 +214,7 @@ "BIT20 - Global Coherency Database changes message.\n" "BIT21 - Memory range cacheability changes message.\n" "BIT22 - Detailed debug message.\n" + "BIT23 - Manageability debug message.\n" "BIT31 - Error message." #string STR_gEfiMdePkgTokenSpaceGuid_PcdFixedDebugPrintErrorLevel_PROMPT #language en-US "Fixed Debug Message Print Level" @@ -237,6 +238,7 @@ "BIT20 - Global Coherency Database changes message.\n" "BIT21 - Memory range cacheability changes message.\n" "BIT22 - Detailed debug message.\n" + "BIT23 - Manageability debug message.\n" "BIT31 - Error message." #string STR_gEfiMdePkgTokenSpaceGuid_PcdReportStatusCodePropertyMask_PROMPT #language en-US "Report Status Code Property" -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112244): https://edk2.groups.io/g/devel/message/112244 Mute This Topic: https://groups.io/mt/103066385/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/4] MdePkg: Improve wording of manageability debug level comment
Improve the wording of the comment explaining the DEBUG_MANAGEABILITY debug level. Signed-off-by: Rebecca Cran --- MdePkg/Include/Library/DebugLib.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h index f0c9f6448794..40772f2e0f7b 100644 --- a/MdePkg/Include/Library/DebugLib.h +++ b/MdePkg/Include/Library/DebugLib.h @@ -48,8 +48,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define DEBUG_CACHE 0x0020 // Memory range cachability changes #define DEBUG_VERBOSE 0x0040 // Detailed debug messages that may // significantly impact boot performance -#define DEBUG_MANAGEABILITY 0x0080 // Detailed debug and payload message of manageability - // related modules, such Redfish, IPMI, MCTP and etc. +#define DEBUG_MANAGEABILITY 0x0080 // Detailed debug and payload manageability messages + // related to modules such as Redfish, IPMI, MCTP etc. #define DEBUG_ERROR 0x8000 // Error // -- 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112241): https://edk2.groups.io/g/devel/message/112241 Mute This Topic: https://groups.io/mt/103066382/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] edk2 uncrustify update (73.0.8)?
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)?
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
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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] -=-=-=-=-=-=-=-=-=-=-=-