The return value after calls to functions
gBS->UninstallMultipleProtocolInterfaces, StartPciDevicesOnBridge,
PciPciDeviceInfoCollector, BarExisted, PciRootBridgeIo->Pci.Write,
gPciHotPlugInit->InitializeRootHpc and PciRootBridgeP2CProcess is
stored in Status, but it is not made of any use and later
From: Ranbir Singh
The function StartPciDevices has a check
ASSERT (RootBridge != NULL);
but this comes into play only in DEBUG mode. In Release mode, there
is no handling if the RootBridge value is NULL and the code proceeds
to unconditionally dereference "RootBridge" which will lead to CR
From: Ranbir Singh
The function PciHostBridgeResourceAllocator is not making use of the
generic approach as is used in one of the other function namely -
DumpResourceMap. As a result, the following warnings can be seen as
reported by Coverity e.g.
(30) Event address_of: Taking address with "&Io
From: Ranbir Singh
The function UpdatePciInfo has switch-case code in which there are fall
through from case 32: to case 64:. While this is seeemingly intentional,
it is not evident to any static analyzer tool. Just adding
// No break; here as this is an intentional fallthrough.
as explicit
From: Ranbir Singh
The function PciHotPlugRequestNotify has two if blocks towards the end
of function both containing return. Due to the parameter checks ensured
at the beginning of the function, one of the two if blocks is bound to
come in execution flow. Hence, the return EFI_SUCCESS; at line 2
Ranbir Singh (5):
MdeModulePkg/Bus/Pci/PciBusDxe: Fix DEADCODE Coverity issue
MdeModulePkg/Bus/Pci/PciBusDxe: Fix MISSING_BREAK Coverity issues
MdeModulePkg/Bus/Pci/PciBusDxe: Fix ARRAY_VS_SINGLETON Coverity issues
MdeModulePkg/Bus/Pci/PciBusDxe: Fix NULL_RETURNS Coverity issue
MdeModuleP
The return value after calls to functions CoreProcessFvImageFile,
CoreStartImage, CoreGetDepexSectionAndPreProccess,
CoreInternalAddMemorySpace, CoreAddIoSpace, CoreAllocateMemorySpace
and CoreCloseProtocol is stored in Status, but it is not made of any
use and later Status gets overridden.
An op
From: Ranbir Singh
"1 << Priority" / "1 << Event->NotifyTpl" are potentially overflowing
expressions with type "int" (32 bits, signed) evaluated using 32-bit
arithmetic, and then used in a context that expects an expression of
type "UINTN" (64 bits, unsigned).
To avoid overflow, cast "1" to type
From: Ranbir Singh
The functions CoreConvertSpace and CoreAllocateSpace in
MdeModulePkg/Core/Dxe/Gcd/Gcd.c has
ASSERT (FALSE); at lines 755 and 1155 which gets hit when
Operation neither include GCD_MEMORY_SPACE_OPERATION nor include
GCD_IO_SPACE_OPERATION but this comes into play only
From: Ranbir Singh
In the function PromoteGuardedFreePages(), the value of AvailablePages
cannot be ZERO at the condition check point
if (AvailablePages != 0) {
as the code can come out of the while loop above only when AvailablePages
is non-ZERO.
Hence, remove the redundant condition check
From: Ranbir Singh
The function CoreIsSchedulable has switch-case code in which
case EFI_DEP_BEFORE:
case EFI_DEP_AFTER:
has a comment that
// For a well-formed Dependency Expression, the code should never get here.
It also has an ASSERT (FALSE); but this is applicable only in D
v1 -> v2:
- Rebased
- Commit message update wrt Fix UNUSED_VALUE Coverity issues
Ranbir Singh (5):
MdeModulePkg/Core/Dxe: Fix FORWARD_NULL Coverity issues
MdeModulePkg/Core/Dxe: Fix MISSING_BREAK Coverity issue
MdeModulePkg/Core/Dxe: Fix DEADCODE Coverity issue
MdeModulePkg/Core/Dxe: F
Nate,
Thanks for the great patch! Minor comments:
1. Regarding the DllEntryPoint, what's the difference between below line in
your patch and the original code that calls GetProcAddress()?
* DllEntryPoint = (VOID *) ((UINTN)Library +
(UINTN)Hdr.Pe32Plus->OptionalHeader.AddressOfEntryPo
Thanks for the reminder/education on the coding style. I knew my habit of
putting variables at the top of functions came from somewhere.
Pushed: https://github.com/tianocore/edk2/commit/ad1c039
-Original Message-
From: Kinney, Michael D
Sent: Tuesday, September 26, 2023 2:43 PM
To: Des
*Reminder: TianoCore Bug Triage - APAC / NAMO*
*When:*
Tuesday, September 26, 2023
6:30pm to 7:30pm
(UTC-07:00) America/Los Angeles
*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e34
Reviewed-by: Hao A Wu
Best Regards,
Hao Wu
> -Original Message-
> From: Cheng, Gao
> Sent: Tuesday, September 26, 2023 4:25 PM
> To: devel@edk2.groups.io
> Cc: Cheng, Gao ; Wu, Hao A ;
> Ni, Ray ; Wang, Jian J ; Gao,
> Liming
> Subject: [PATCH v2] MdeModulePkg/Xhci: Skip size round up
Hi Nate,
Reviewed-by: Michael D Kinney
That makes complete sense now on the use case when building
drivers or apps outside EmulatorPkg DSC file and wanting
to support source level debug. In the past, I have had to
add components to EmulatorPkg DSC file.
Thank you for adding feature to support
Hi Mike,
Yes, we can delete those lines now 😊. I have those lines commented out in my
copy of EmulatorPkg. I found this because I have been trying to use EmulatorPkg
to source level debug drivers that I built separately via one of the
edk2-platform builds.
Thanks,
Nate
-Original Message--
Hi Nate,
Declaring all the local variables at the top of the function is
really a coding style recommendation documented in the EDK II
C Coding Style Specification. End of Section 5.4.1.1.
Declaring local in other scopes is strongly discouraged.
https://tianocore-docs.github.io/edk2-CCodingSta
I have VS Code on Windows working today using the PDB based symbol file.
The following lines in the EmulatorPkg DSC file force 4KB alignment and the DLL
export of the InitializeDriver symbol
https://github.com/tianocore/edk2/blob/bf0bdacdd6f6cdd2e9ac5db14b6daf19a5a5bd57/EmulatorPkg/EmulatorPkg.d
Looks like it compiles fine in VS2015, and we just removed the tools_def
entries for every VC++ version older than that. And C99 has been in gcc for a
very long time and clang forever. So perhaps we can start using C99 style
variable declarations finally?
Thanks,
Nate
-Original Message
Oh, never mind, I see you are suggesting a C99 style variable declaration. Do
we need to worry about old compiler that want all variable declarations at the
top still?
Thanks,
Nate
-Original Message-
From: Desimone, Nathaniel L
Sent: Tuesday, September 26, 2023 1:38 PM
To: Kinney, Mich
Hi Mike,
Unfortunately, that change will generate the following warning on GCC4.6+
warning: variable "Success" set but not used [-Wunused-but-set-variable]
Hence why I wrote it that way. Let me know if you would like me to make a
different change before committing.
Thanks,
Nate
-Original
Hi Mike,
Source level debug with VS Code does indeed work today with gdb or lldb. This
change makes the Visual Studio Windows debugger work as well.
You are correct that if the same DLL is loaded more than once that this method
cannot perform source level debug on the second instance; but this
Hi Nate,
I am able to do source level debug of EmulatorPkg using VS Code today.
What scenarios are broken?
I do know that the DLL based approach would only allow a single
instance of the module to be loaded and debugged. If, for example,
a driver is loaded more than once from the UEFI Shell in
Thanks Nate!
I have noticed this issue for a while.
One comment below. With that update:
Reviewed-by: Michael D Kinney
Mike
> -Original Message-
> From: Desimone, Nathaniel L
> Sent: Tuesday, September 26, 2023 11:36 AM
> To: devel@edk2.groups.io
> Cc: Andrew Fish ; Ni, Ray ; Kinney
From: Michael Kubacki
Adds a workflow to run CodeQL against all packages built in
.pytool/CISettings.py. The following is done:
1. Determine which packages to build against. Those that support
are managed by .pytool/CISettings.py will be selected.
For each package:
2. Determine how to inter
From: Michael Kubacki
Adds the `--codeql` parameter to `stuart_update` and
`stuart_ci_build`.
- `stuart_update --codeql` - Downloads the CodeQL CLI locally. The
command will pull the appropriate binary for the host OS.
- `stuart_ci_build --codeql` - Runs CodeQL during the build resulting
in
From: Michael Kubacki
CodeQL currently runs via the codeql-analysis.yml GitHub workflow
which uses the `github/codeql-action/init@v2` action (pre-build)
and the `github/codeql-action/analyze@v2` action (post-build) to
setup the CodeQL environment and extract results.
This infrastructure is remov
From: Michael Kubacki
Adds a Python module to the CodeQL plugin directory that exports
functions commonly needed for Stuart-based platforms to easily
enable CodeQL in their platform build.
This functionality has already moved to edk2-pytool-extensions
https://github.com/tianocore/edk2-pytool-ext
From: Michael Kubacki
Adds a CodeQL plugin that supports CodeQL in the build system.
1. CodeQlBuildPlugin - Generates a CodeQL database for a given build.
2. CodeQlAnalyzePlugin - Analyzes a CodeQL database and interprets
results.
3. External dependencies - Assist with downloading the CodeQL
From: Michael Kubacki
CodeQL currently runs via the codeql-analysis.yml GitHub workflow
which uses the github/codeql-action/init@v2 action (pre-build)
and the github/codeql-action/analyze@v2 action (post-build) to
setup the CodeQL environment and extract results.
This infrastructure is removed i
After running EmulatorPkg, one will notice that their terminal acts
strangely. This is caused by the EmulatorPkg Host changing the terminal
mode and not restoring the original mode, which is now fixed.
Cc: Andrew Fish
Cc: Ray Ni
Cc: Michael D Kinney
Cc: Chasel Chiu
Signed-off-by: Nate DeSimone
The ARM implementation of of InternalLongJump always returned the value
Value - but it is not supposed to ever return 0. Add the test to prevent
that, and return 1 if Value is 0 - as is already present in AArch64.
Signed-off-by: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
---
MdePkg/Libr
There may be architectures on which there are benefits to
eor r0, r0(, r0)
but ARM was never one of them. Change to more readable
mov r0, #0
instead.
Signed-off-by: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
---
MdePkg/Library/BaseLib/Arm/SetJumpLongJump.S | 2 +-
MdePkg/Library/B
Both in SetJump and in InternalLongJump, 32-bit w register views were
used for the UINTN return value. In SetJump, this did not cause errors;
it was only counterintuitive. But in InternalLongJump, it meant the top
32 bits of Value were stripped off.
Change all of these to use the 64-bit x register
The SetJump comment header states that:
If JumpBuffer is NULL, then ASSERT().
However, this was not currently done.
Add a call to InternalAssertJumpBuffer.
Signed-off-by: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
---
MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S | 8
M
Drop redundant comment about IPF (clearly copied across from now deleted
code).
Also change
"Instead is resumes execution" ->
"Instead it resumes execution"
Signed-off-by: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
---
MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S | 3 +--
MdePkg/
Back in the mists of time (January 2020) I submitted a set to clean up
and fix a few bugs in ARM/AArch64 BaseLib SetJump/LongJump implementations:
https://edk2.groups.io/g/devel/message/65812
Then hubris struck and I meant to refactor the code for all architectures,
and since this was in my first
Hi Sami/Yeo
Do you have any comments on this series?
Regards,
On Thu, Sep 14, 2023 at 4:10 PM Tuan Phan wrote:
> This series makes StandaloneMmCpu platform independent so that
> other platforms besides ARM/AARCH64 can use it without creating
> new driver.
>
> There are two parts in this series:
Reviews on this patch series would be much appreciated :)
On 8/28/23 9:38 AM, Ard Biesheuvel wrote:
I was hoping to get around to this before the end of the week (but I
am not a MdeModulePkg maintainer)
On Mon, 28 Aug 2023 at 18:27, Taylor Beebe wrote:
Can I please get reviews/feedback on th
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4555
Add logic that checks if the code doesn't overflow
ACPI_EXTENDED_HID_DEVICE_PATH node when searching for optional
strings. If the string is not provided in the device path node
default value of "\0" is used.
Cc: Michael D Kinney
Cc: Liming
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4555
Github PR: https://github.com/tianocore/edk2/pull/4865
Fix for buffer overlows that arise in DevPathToTextAcpiEx when device path node
producer doesn't specify all of the optional strings.
Tests:
- Booted the platform and confirmed that pla
Hi everybody,
I have built the Whisky Lake platform and I got the fd file but now I want
to upload it into my motherboard's flash chip but I haven't found the right
way of doing so. it seems I need to translate it into a *.rom file but I
don't know how to do that. I have already built a coreboot fi
After review, there are unnecessary steps for BSP and AP sync for exit.
This patch is to reduce one round BSP and AP sync for exit so as to
improve SMM performance:
WaitForAllAPs <- ReleaseBsp
ReleaseAllAPs -> WaitForBsp
Cc: Eric Dong
Cc: Ray Ni
Cc: Zeng Star
Cc: Rahul Kumar
Cc: Gerd Hoffmann
This patch is to:
1. Define 2 new functions (WaitForBsp & ReleaseBsp) used
for the semaphore sync between BSP & AP.
2. Add ReleaseOneAp(), used for BSP to release one AP.
With the change, BSP & AP Sync flow will be easy understand:
BSP to Release All APs ---> AP to Wait BSP
ReleaseAllAPs ()
The series patches are to optimize semaphore sync between BSP
and AP:
Patch 1: Define 3 functions (WaitForBsp & ReleaseBsp & ReleaseOneAp)
specific for BSP & AP sync, which will make the flow easy to
understand.
Patch 2: Reduce one round BSP and AP sync for exit so as to
improve SMM performance
Ji
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4560
TRB Template is 16 bytes. When boundary checking is 64 bytes for xHCI
device/host memory address, it may exceed xHCI host memory pool and
cause unwanted DXE_ASSERT. Introduce a new input parameter to indicate
whether to enforce 64byte size al
48 matches
Mail list logo
