From: "Ranbir Singh" <rsingh@ventanamicro.com>
To: devel@edk2.groups.io, rsingh@ventanamicro.com
Cc: Hao A Wu <hao.a.wu@intel.com>, Ray Ni <ray.ni@intel.com>
Subject: [edk2-devel] [PATCH v1 5/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix UNUSED_VALUE Coverity issues
Date: Wed, 27 Sep 2023 11:47:01 +0530 [thread overview]
Message-ID: <20230927061701.451202-6-rsingh@ventanamicro.com> (raw)
In-Reply-To: <20230927061701.451202-1-rsingh@ventanamicro.com>
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 Status
gets overridden.
Remove the return value storage in Status or add Status check as
seems appropriate at a particular point.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4239
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 68 +++++++++++---------
MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 42 ++++++++----
MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 8 +++
3 files changed, 72 insertions(+), 46 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index f43f10325f16..ae770d766381 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -544,12 +544,12 @@ DeRegisterPciDevice (
EFI_OPEN_PROTOCOL_TEST_PROTOCOL
);
if (!EFI_ERROR (Status)) {
- Status = gBS->UninstallMultipleProtocolInterfaces (
- Handle,
- &gEfiLoadFile2ProtocolGuid,
- &PciIoDevice->LoadFile2,
- NULL
- );
+ gBS->UninstallMultipleProtocolInterfaces (
+ Handle,
+ &gEfiLoadFile2ProtocolGuid,
+ &PciIoDevice->LoadFile2,
+ NULL
+ );
}
//
@@ -678,19 +678,21 @@ StartPciDevicesOnBridge (
ChildHandleBuffer
);
- PciIoDevice->PciIo.Attributes (
- &(PciIoDevice->PciIo),
- EfiPciIoAttributeOperationSupported,
- 0,
- &Supports
- );
- Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
- PciIoDevice->PciIo.Attributes (
- &(PciIoDevice->PciIo),
- EfiPciIoAttributeOperationEnable,
- Supports,
- NULL
- );
+ if (!EFI_ERROR (Status)) {
+ PciIoDevice->PciIo.Attributes (
+ &(PciIoDevice->PciIo),
+ EfiPciIoAttributeOperationSupported,
+ 0,
+ &Supports
+ );
+ Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
+ PciIoDevice->PciIo.Attributes (
+ &(PciIoDevice->PciIo),
+ EfiPciIoAttributeOperationEnable,
+ Supports,
+ NULL
+ );
+ }
return Status;
} else {
@@ -726,19 +728,21 @@ StartPciDevicesOnBridge (
ChildHandleBuffer
);
- PciIoDevice->PciIo.Attributes (
- &(PciIoDevice->PciIo),
- EfiPciIoAttributeOperationSupported,
- 0,
- &Supports
- );
- Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
- PciIoDevice->PciIo.Attributes (
- &(PciIoDevice->PciIo),
- EfiPciIoAttributeOperationEnable,
- Supports,
- NULL
- );
+ if (!EFI_ERROR (Status)) {
+ PciIoDevice->PciIo.Attributes (
+ &(PciIoDevice->PciIo),
+ EfiPciIoAttributeOperationSupported,
+ 0,
+ &Supports
+ );
+ Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
+ PciIoDevice->PciIo.Attributes (
+ &(PciIoDevice->PciIo),
+ EfiPciIoAttributeOperationEnable,
+ Supports,
+ NULL
+ );
+ }
}
CurrentLink = CurrentLink->ForwardLink;
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index eda97285ee18..636885dd189d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -2796,6 +2796,20 @@ IsPciDeviceRejected (
// Test its high 32-Bit BAR
//
Status = BarExisted (PciIoDevice, BarOffset, &TestValue, &OldValue);
+ if (EFI_ERROR (Status)) {
+ //
+ // Not sure if it is correct to skip the below if (TestValue == OldValue) check in this special scenario.
+ // If correct, then remove these 11 comment lines eventually.
+ // If not correct, then replace "continue;" with blank "; // Nothing to do" and also remove these 11 comment lines eventually
+ // OR
+ // Remove the newly added if (EFI_ERROR (Status)) { ... } block completely and change
+ // Status = BarExisted (PciIoDevice, BarOffset, &TestValue, &OldValue);
+ // =>
+ // BarExisted (PciIoDevice, BarOffset, &TestValue, &OldValue);
+ // i.e., no return value storage in Status
+ //
+ continue;
+ }
if (TestValue == OldValue) {
return TRUE;
}
@@ -2861,13 +2875,13 @@ ResetAllPpbBusNumber (
if (!EFI_ERROR (Status) && (IS_PCI_BRIDGE (&Pci))) {
Register = 0;
Address = EFI_PCI_ADDRESS (StartBusNumber, Device, Func, 0x18);
- Status = PciRootBridgeIo->Pci.Read (
- PciRootBridgeIo,
- EfiPciWidthUint32,
- Address,
- 1,
- &Register
- );
+ PciRootBridgeIo->Pci.Read (
+ PciRootBridgeIo,
+ EfiPciWidthUint32,
+ Address,
+ 1,
+ &Register
+ );
SecondaryBus = (UINT8)(Register >> 8);
if (SecondaryBus != 0) {
@@ -2878,13 +2892,13 @@ ResetAllPpbBusNumber (
// Reset register 18h, 19h, 1Ah on PCI Bridge
//
Register &= 0xFF000000;
- Status = PciRootBridgeIo->Pci.Write (
- PciRootBridgeIo,
- EfiPciWidthUint32,
- Address,
- 1,
- &Register
- );
+ PciRootBridgeIo->Pci.Write (
+ PciRootBridgeIo,
+ EfiPciWidthUint32,
+ Address,
+ 1,
+ &Register
+ );
}
if ((Func == 0) && !IS_PCI_MULTI_FUNC (&Pci)) {
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 71767d3793d4..087fe563c0bc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -1250,6 +1250,10 @@ PciScanBus (
&State
);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "Failed to initialize Hotplug PCI Controller, Status %r\n", Status));
+ }
+
PreprocessController (
PciDevice,
PciDevice->BusNumber,
@@ -1501,6 +1505,10 @@ PciRootBridgeP2CProcess (
if (!IsListEmpty (&Temp->ChildList)) {
Status = PciRootBridgeP2CProcess (Temp);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "Failed to process Option Rom on PCI root bridge %p, Status %r\n", Temp, Status));
+ }
}
CurrentLink = CurrentLink->ForwardLink;
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109112): https://edk2.groups.io/g/devel/message/109112
Mute This Topic: https://groups.io/mt/101612811/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
prev parent reply other threads:[~2023-09-27 6:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-27 6:16 [edk2-devel] [PATCH v1 0/5] BZ 4239: Fix PciBusDxe issues pointed by Ranbir Singh
2023-09-27 6:16 ` [edk2-devel] [PATCH v1 1/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix DEADCODE Coverity issue Ranbir Singh
2023-09-27 6:16 ` [edk2-devel] [PATCH v1 2/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix MISSING_BREAK Coverity issues Ranbir Singh
2023-09-27 6:16 ` [edk2-devel] [PATCH v1 3/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix ARRAY_VS_SINGLETON " Ranbir Singh
2023-09-27 6:17 ` [edk2-devel] [PATCH v1 4/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix NULL_RETURNS Coverity issue Ranbir Singh
2023-09-27 6:17 ` Ranbir Singh [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230927061701.451202-6-rsingh@ventanamicro.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox